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

Allow conda activate to set its own PATH. #109

Merged
merged 3 commits into from Feb 8, 2020
Merged

Allow conda activate to set its own PATH. #109

merged 3 commits into from Feb 8, 2020

Conversation

ktlim
Copy link
Contributor

@ktlim ktlim commented Feb 7, 2020

No description provided.

Copy link
Member

@jhoblitt jhoblitt left a comment

Choose a reason for hiding this comment

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

This looks reasonable but it looks like it may somehow break the templating of loadLSST.bash?

export LSST_CONDA_ENV_NAME=\${LSST_CONDA_ENV_NAME:-${LSST_CONDA_ENV_NAME}}
# shellcheck disable=SC1091
source activate "\$LSST_CONDA_ENV_NAME"
source "${miniconda_path}/bin/activate" "\$LSST_CONDA_ENV_NAME"
Copy link
Member

Choose a reason for hiding this comment

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

At some point aren't we supposed to be switching to conda activate ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave that modernization to someone else.

conda activate should do this for us.
@ktlim
Copy link
Contributor Author

ktlim commented Feb 7, 2020

@jhoblitt Indeed the point of this is to change the template output. I believe I have removed the test that no longer applies.

@ktlim
Copy link
Contributor Author

ktlim commented Feb 7, 2020

@jhoblitt Some help with figuring out why the bash compile fails would be appreciated, though. The failure seems to be caused by the makefile looking for newversion.h after it has explicitly moved it aside. Perhaps a parallel make race condition? Nothing seems new for this step.

@ktlim
Copy link
Contributor Author

ktlim commented Feb 8, 2020

Looks like making the make non-parallel solved this.

@ktlim ktlim merged commit 4a81834 into master Feb 8, 2020
@ktlim ktlim deleted the tickets/DM-23393 branch February 8, 2020 01:12
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.

None yet

3 participants