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

Create a python app for dependencies instead of venv #16694

Merged
merged 11 commits into from Nov 12, 2020

Conversation

mppf
Copy link
Member

@mppf mppf commented Nov 12, 2020

Follow-up to PR #16663 #16644 and #16560

We run into problems when including a venv in the installed code for
Chapel (e.g. in a Chapel RPM) because the venv encodes the path to the
python interpreter (both as symbolic links and also in pyvenv.cfg). We
tried to work around this in PRs #3571 #16663 and #16680.

This PR instead changes the strategy away from trying to create a
long-lasting venv in the third-party/chpl-venv. Instead, it creates a
venv during installation (so we can have pip even on systems that don't
include it by default) and then uses that venv to install the
dependencies into chpldeps which is a Python application. Now we
install the dependencies with python3 -m pip --target path/to/chpldeps.

Additionally we no longer include getpip support in the Makefiles as it
should no longer be used and we stop passing --cache-dir to the pip
commands (so that the cache can be used night-to-night in nightly
testing).

Originally I had hoped to use zipapp to store chpldeps as a zipfile
but that causes problems with Sphinx (when it looks for theme files). So,
here we just have chpldeps as a directory (which Python considers a
kind of application) just as we would before zipping it with zipapp.
zipapp does not do much other than storing the files into a .zip and so
this should have similar portability.

To keep it convenient, Python programs needing chpldeps can be run as
python3 path/to/chpldeps some/python/program.py or for certain included
tools, python3 path/to/chpldeps sphinx-build. This avoids the need for
util/config/run-in-venv.bash in many cases.

Lastly, I wasn't sure what to do with chplspell for this PR since
chplspell is a developer-only tool. For now, it uses the same venv that
we install pip into.

Reviewed by @ben-albrecht and @lydia-duncan - thanks!

  • full local testing
  • test_rpm_module.bash

This started in PR chapel-lang#9137 but the reason is lost to time.
Leaving this flag out will allow pip to use its local cache
and reduce reliance on the network.

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
it is included in python3
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
Started to do this as a zipapp but sphinx doesn't like
to exist within a zipfile (we get errors when it tries to
open a theme). So now we just install the dependencies
and extend the python path to include them.

For convenience, the 'chpldeps' Python app will run
included dependencies, e.g.
  chpldeps sphinx-build arg...

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
Can't have a regular target depend on a phony one.

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
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.

What is the behavior:

  • on a Mac after a clobber?
  • on Linux after a clobber?
  • when calling make chpldoc/make test-venv when nothing has changed in chpl-venv since the last time it was run?
  • with a make docs call when chpldoc is not built?
  • with a make docs call when chpldoc has been built?

third-party/chpl-venv/Makefile.include Show resolved Hide resolved
@mppf
Copy link
Member Author

mppf commented Nov 12, 2020

on a Mac after a clobber?

make docs downloads the dependencies. a second make docs does not download. both succeed.

on Linux after a clobber?

ditto

when calling make chpldoc/make test-venv when nothing has changed in chpl-venv since the last time it was run?

nothing is downloaded

with a make docs call when chpldoc is not built?

it builds chpldoc. it does not rebuild the chpldeps if they have not changed.

with a make docs call when chpldoc has been built?

nothing is downloaded because make chpldoc gathers the dependencies and constructs chpldeps.

For now, this uses the third-party/chpl-venv/build-venv because I'm not
so sure it makes sense to include the chplspell dependency in what gets
installed since it is a developer-only tool.

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
@mppf mppf merged commit 5fad770 into chapel-lang:master Nov 12, 2020
@mppf mppf deleted the venv-zipapp branch November 12, 2020 22:36
mppf added a commit to mppf/chapel that referenced this pull request Nov 12, 2020
I meant to remove this in PR chapel-lang#16694 which makes it no longer used.

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
@mppf mppf mentioned this pull request Nov 12, 2020
mppf added a commit that referenced this pull request Nov 12, 2020
Remove python3-wrapper

I meant to remove this in PR #16694 which makes it no longer used.

Trivial and not reviewed.
mppf added a commit that referenced this pull request Nov 13, 2020
Tidy up Makefile error handling for chpl-venv

Follow-up to PR #16694

* Removes check for a working pip since `python3 -m venv` creates a venv
  that is required to have pip.
* Adjusts the Makefile targets running pip to better handle errors.
  Before, it was creating the build/chpl-venv or install/chpldeps
  directories even if the `pip` command failed. Stop doing that.
*  Adjust run-in{-test}-venv.bash to check for `__main__.py` - To avoid
   problems if the directory exists but nothing within it does, which we
   observed in nightly testing when the `pip` command failed due to
   network errors.

Reviewed by @lydia-duncan - thanks!

- [x] verified that the `chpldeps` directory is not added when causing
  pip to fail by setting `PIP_INDEX_URL` to something bogus
- [x] verified that `run-in-test-venv.bash` reports an error when I
  `mkdir` the `chpldeps` directory
- [x] `make docs` works on Ubuntu 16.04 with Python 3.5
- [x] `make docs` works on a Cray XC
- [x] `make docs` works on Mac OS X
- [x] full local testing
mppf added a commit that referenced this pull request Nov 17, 2020
 Fix chpl_home_utils.py --test-venv calls take 2

Follow-up to PR #16694 and #16560

Replaces #16722.

The --test-venv option was removed in #16694, so adjust scripts that were
using it.  To support chpl-scripts-completion.bash, added
`register-python-argcomplete` as a subcommand for the chpldeps
application. While there, improved the usage checking for chpldeps.

Reviewed by @ronawho - thanks!
daviditen pushed a commit to daviditen/chapel that referenced this pull request Dec 3, 2020
I meant to remove this in PR chapel-lang#16694 which makes it no longer used.

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
Signed-off-by: David Iten <daviditen@users.noreply.github.com>
CaptainSharf pushed a commit to CaptainSharf/chapel that referenced this pull request Jan 16, 2021
I meant to remove this in PR chapel-lang#16694 which makes it no longer used.

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Signed-off-by: CaptainSharf <mohdsharf7407@gail.com>
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