Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dev setup script #15

Merged
merged 4 commits into from
Oct 25, 2015
Merged

Fix dev setup script #15

merged 4 commits into from
Oct 25, 2015

Conversation

llvasconcellos
Copy link
Contributor

Added creation of files and folders and setting permissions to proper functioning of Profile Manager on developer machines.

@zestyping
Copy link
Contributor

Yes, it's a great idea to make sure the profiles directory exists. Can we do this without sudo though? At the moment any developer can get going without having root access, and now that you've made the profiles directory configurable it seems like the perfect opportunity to switch away from using a directory that only root can access. Perhaps something under $server_dir since that's already being set up here?

# in the profile_apply script there is a hardcoded reference to utils.sh. sed will replace this a mac friendly url.
# TODO: review this after system wide path related changes.
if [[ "$OS" == 'Darwin' ]]; then
sed 's/share/local\/opt/g' tools/profile_apply > "$bin_dir/buendia-profile-apply"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks scary... if the word share appears anywhere in the script it will get changed. How about doing this logic in Python in profile_apply, rather than using sed to edit the Python code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. I fixed it. Now it looks for /usr/share and replace for /usr/local/opt.

@llvasconcellos
Copy link
Contributor Author

I added a TODO and created an issue #17 to remove the sudo request.

# In the profile_apply script there is a hardcoded reference to utils.sh. sed will replace this a mac friendly url.
# TODO: review this after system wide path related changes.
if [[ "$OS" == 'Darwin' ]]; then
sed 's/\/usr\/share/\/usr\/local\/opt/g' tools/profile_apply > "$bin_dir/buendia-profile-apply"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This execution is throwing tools/openmrs_setup: line 93: /usr/local/bin/buendia-profile-apply: Permission denied

Refactoring to sed 's/\/usr\/share/\/usr\/local\/opt/g' tools/profile_apply > sudo "$bin_dir/buendia-profile-apply"worked fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Done.

@viniciusboson
Copy link
Contributor

After this fixing this permission denied, the script worked fine.

screen shot 2015-10-23 at 7 09 48 pm

@llvasconcellos
Copy link
Contributor Author

Thank you @viniciusboson. All suggestions accepted and the script is updated.

llvasconcellos added a commit that referenced this pull request Oct 25, 2015
@llvasconcellos llvasconcellos merged commit 3fb4957 into projectbuendia:dev Oct 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants