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

Python executable path #105

Merged
merged 3 commits into from Mar 23, 2018
Merged

Python executable path #105

merged 3 commits into from Mar 23, 2018

Conversation

weaverba137
Copy link
Member

This PR attempts to fix #104 at least for future installs by checking that during the bootstrap process, desiInstall is installed with an explicit desiconda version, not "current".

In theory, it would be possible to reject any attempt to use desiInstall if, internally, it cannot find an explicit desiconda version, but I have not implemented this. I can if that is desired. If we go down this road though, we may want to consider simply eliminating the 'current' symlink entirely, so that it is impossible not to choose an explicit version.

In addition, the desiutil.module file is added to the internal structure of the desiutil Python package so that it is available even if the etc/ directory is not installed, which it won't be, because we no longer copy the full package contents on install.

Finally, there is some clean-up of the package imports in desiutil.install.

@weaverba137 weaverba137 self-assigned this Mar 23, 2018
Copy link
Contributor

@sbailey sbailey left a comment

Choose a reason for hiding this comment

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

Changes look fine, but I'm not sure how to really test it without impacting our current installations, until we have a new desiconda and can try bootstrapping desiutil into it. OK with me to merge and then make other updates as needed when we are bootstrapping a new desiconda.

@weaverba137
Copy link
Member Author

On edison, I hand-edited desiInstall, and installed desidatamodel 1.2.0. The executable scripts in desidatamodel then had the correct Python paths. So I think we'll be OK with just hand-editing desiInstall on cori & edison for the current desiconda version, then double-checking after the next bootstrap.

@weaverba137 weaverba137 merged commit 89175b3 into master Mar 23, 2018
@weaverba137 weaverba137 deleted the python-executable-path branch March 23, 2018 21:35
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.

installed code shouldn't reference desiconda/current
2 participants