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

Adjust env vars instead of using venv activate #16669

Merged
merged 2 commits into from Nov 5, 2020

Conversation

mppf
Copy link
Member

@mppf mppf commented Nov 5, 2020

Follow-up to PR #16644 and #16560

This PR (along with PR #16663) is intended to resolve warnings in nightly
testing like this

[Warning: could not import filelock]

The venv activate script stores the absolute path but that causes
problems in case it changes (e.g. if CHPL_HOME is renamed or in the
case of module builds).

So, this PR changes run-in-venv.bash and run-in-test-venv.bash to
adjust environment variables directly rather that using the activate
script. It also adjusts c2chapel to use run-in-venv.bash instead of the
activate script.

Reviewed by @lydia-duncan - thanks!

  • test/c2chapel and test/chpldoc pass after building chpldoc and c2chapel
  • full local testing

Copy link
Member

@lydia-duncan lydia-duncan left a comment

Choose a reason for hiding this comment

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

I think this makes sense, thanks!

Add comments, use && in some commands where that was intended

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
The venv activate script stores the absolute path
but that causes problems in case it changes
(e.g. if CHPL_HOME is renamed or in the case of
 module builds)

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
@mppf mppf merged commit 13c9a7e into chapel-lang:master Nov 5, 2020
@mppf mppf deleted the dont-use-activate branch November 5, 2020 22:57
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

2 participants