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

Use python3/python/python2 instead of just python #16560

Merged
merged 3 commits into from Oct 21, 2020

Conversation

mppf
Copy link
Member

@mppf mppf commented Oct 5, 2020

This PR adjusts core parts of Chapel to work with whatever python it can find - looking for python3, then python, then python2. This should enable the compiler and runtime to build and operate in a variety of systems.

Features that need maximum portability are adapted to use a new sh script util/config/find-python.sh which returns the version of python to run. It chooses (in this order) python3 python python2. In each case the thing calling the python script in normal usage computes the name of the interpreter to run.

  • Makefiles set CHPL_MAKE_PYTHON from util/config/find-python.sh and then run Python support scripts like $(CHPL_MAKE_PYTHON) some-script.py (which translates to e.g. python3 some-script.py)
  • updated checkChplInstall similarly (called from make check)
  • printchplenv already included an sh wrapper so updated it to run util/config/find-python.sh
  • added an sh wrapper for compileline that uses util/config/find-python.sh
  • setchplenv scripts now call util/config/find-python.sh to invoke helpers
  • mason test also uses find-python.sh

This PR then adjusts many other uses of python to now require python3. Things in this category are:

  • make docs, chpldoc and related support
  • start_test and related support
  • python scripts in test/ e.g. .prediff files
  • tests that run python scripts by naming the interpreter

Future work -- revisit PR #3571 if necessary

  • Useful trick for finding issues here is make SHELL='sh -x'

@mppf mppf mentioned this pull request Oct 5, 2020
3 tasks
@mppf mppf force-pushed the use-python3-python-python2 branch from 5419e5c to af2d91c Compare October 5, 2020 21:17
@mppf mppf force-pushed the use-python3-python-python2 branch 6 times, most recently from 30dfa80 to 490d17f Compare October 6, 2020 14:23
@mppf mppf force-pushed the use-python3-python-python2 branch from 490d17f to 773a11e Compare October 15, 2020 11:23
@mppf mppf marked this pull request as ready for review October 15, 2020 16:40
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
@mppf mppf force-pushed the use-python3-python-python2 branch from e64ed4f to edf104e Compare October 16, 2020 13:33
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
@mppf mppf changed the title Adjust core scripts to use python3/python/python2 Use python3/python/python2 instead of just python Oct 16, 2020
@mppf mppf removed the post-release label Oct 16, 2020
@lydia-duncan lydia-duncan merged commit 49b7834 into chapel-lang:master Oct 21, 2020
mppf added a commit that referenced this pull request Oct 22, 2020
Use surrogateescape in computePerfStats

Follow-up to PR #16560

The `computePerfStats` program opens the output file looking for keys to 
find perf information (such as timing printouts). However, we have some
programs (e.g. mandelbrot) that output binary data. That caused a problem
after PR #16560 because the python3 default I/O is assuming UTF-8. This
PR adjusts the file opened to read the perf information to tolerate UTF-8 
encoding errors by using the `surrogateescape` error handling strategy.
This allows the rest of the processing to assume UTF-8 and should have 
minimal impact since we expect the matched strings (e.g. `TIME=140.0`)
will be UTF-8.

- [x] mandelbrot tests pass with `start_test -performance`

Reviewed by @ben-albrecht - thanks!
mppf added a commit that referenced this pull request Oct 22, 2020
Fix llvmDebug test for python3

Follow-up to PR #16560

Fix several problems with python script in test/llvm/llvmDebug.
While there, got the test working with CHPL_LLVM=system.

Test change only - not reviewed.

- [x] test/llvm/llvmDebug passes with CHPL_LLVM=system
- [x] test/llvm/llvmDebug passes with CHPL_LLVM=llvm
mppf added a commit to mppf/chapel that referenced this pull request Oct 23, 2020
Follow-up to PR chapel-lang#16560 and chapel-lang#16608

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
mppf added a commit that referenced this pull request Nov 3, 2020
fix man/Makefile

Follow-up to PR #16644 and #16560

* Adjust run-in-venv.bash to error messages to stderr
* Put back changes from PR #5579
* Invoke rst2man.py with the current python interpreter to avoid problems with the shebang being too long in some settings

Mostly trivial but by @ronawho - thanks!
mppf added a commit that referenced this pull request Nov 3, 2020
Include util/run-in-venv.bash in install/tarball

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

To resolve smoke test failures.

Reviewed by @ronawho - thanks!

- [x] verified that `util/buildRelease/gen_release` and untarring that and running `make check-chpldoc` in it works
mppf added a commit that referenced this pull request Nov 4, 2020
Move util/run-in-venv.bash to util/config/run-in-venv.bash

Follow-up to PRs #16649, #16644, and #16560.

Because it need not be in PATH and the setchplenv.bash scripts add util/ to PATH.

Reviewed by @ben-albrecht - thanks!

- [x] full local testing
mppf added a commit that referenced this pull request Nov 4, 2020
Update c2chapel tests to work if c2chapel not in path

Follow-up to PR #16644 and #16560.

PR #16644 stopped leaving anything in `CHPL_HOME` in `PATH` when running 
tests (because PATH was updated inconsistently in the past and because we
need to remove the venv path).

However that caused problems for test/c2chapel because it assumed 
c2chapel was in the PATH. This PR fixes those tests to find c2chapel in
the bin directory.

Reviewed by @ben-albrecht - thanks!

- [x] full local testing
mppf added a commit that referenced this pull request Nov 4, 2020
…ion-works

Improve find-python.sh to check python --version

Follow-up to PR #16560

Does not check the output of python --version. It just
tries to run that and checks for return code 0.

This addresses problems with pyenv when python3 is installed
but not selected as the current python and when the system
does not include a python3 installation.

Helps with Cray/chapel-private#1407

Reviewed by @lydia-duncan - thanks!

- [x] full local testing
@mppf mppf mentioned this pull request Nov 4, 2020
2 tasks
mppf added a commit that referenced this pull request Nov 4, 2020
Add genGraphs wrapper

Follow-up to PRs #16644 and #16560.

Since genGraphs is run as a subprocess (rather than imported), it needs 
to take steps to run in the venv so that `import yaml` will work.

This is intended to resolve warnings like this in some nightly testing 
configurations:

```
[Warning: “annotate” import failed, no annotations will be generated]
``` 

Reviewed by @ronawho - thanks!

- [x] spot-checked a performance test
- [x] full local testing
mppf added a commit that referenced this pull request Nov 4, 2020
Replace symbolic link python3 in venv with wrapper

Follow-up to PRs #16644 and #16560.

This PR is intended to solve problems where this warning is reported in some nightly testing configurations:
```
[Warning: could not import filelock]
```

The venv normally includes in some-venv/bin/python3 a symbolic link to
the real python3 interpreter used.

This has the drawback that when distributing the venv, the relevant
python executable might be in a different place.

So, this commit adds a bash script that runs python while telling it to
think it is being launched from that location (that is what the exec -a
here does). The wrapper uses whichever python3 is available in the
system. This allows the python3 being run to change (e.g. if the venv is
used on a system with a different path to the python3).

This approach replaces the approach from PR #3571.

Reviewed by @ronawho - thanks!

- [x] checked basic module build on an XC
- [x] full local testing
mppf added a commit that referenced this pull request Nov 5, 2020
Fix make && make man when CHPL_HOME is not set

Follow-up to PR #16644 and #16560

Some nightly testing configurations run `make` and then `make man`
without having `CHPL_HOME` set. However, `make man` was recently
changed to use `run-in-venv.bash` but this script required `CHPL_HOME`
be set.

This PR adjusts `run-in-venv.bash` to determine `CHPL_HOME` if it is
not set and in that event to set `CHPL_HOME` for the subcommand. This
allows `make && make man` as well as `make && make docs`
to function when `CHPL_HOME` is not set.

While testing this PR, I noticed additionally that `make clobber` does
not remove the generated files in `man/man1`, so this PR adjusts
`Makefile` and `man1/Makefile` to do so.

Reviewed by @ronawho - thanks!

- [x] make && make man && make docs works
- [x] test/chpldoc/ passes
mppf added a commit that referenced this pull request Nov 5, 2020
Improve error message for start_test with no venv

Follow-up to PRs #16644 and #16560.

Improve error message for `start_test` with no venv or for a venv that is
built without testing support (say, if one runs `make clobber && make
docs` and then tries to use `start_test`).

Additionally recent changes had broken workflows using
`CHPL_TEST_VENV_DIR=none` / `CHPL_TEST_VENV_DIR=<dir> `.

This PR improves the situation by adding a script
`util/test/run-in-test-venv.bash` that is run from `paratest.server` and
from the `start_test`, `sub_test`, and `genGraphs` wrappers. The new
script includes the logic previously present in
`util/test/activate_chpl_test_venv.py` to work with
`CHPL_TEST_VENV_DIR=none` / `CHPL_TEST_VENV_DIR=<dir> `.

Reviewed by @ronawho - thanks!

- [x] verified expected error from `start_test`
- [x] verified expected error from `paratest.server`
mppf added a commit that referenced this pull request Nov 5, 2020
 Adjust env vars instead of using venv activate

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!

- [x] test/c2chapel and test/chpldoc pass after building chpldoc and c2chapel
- [x] full local testing
mppf added a commit that referenced this pull request Nov 5, 2020
Adjust llvmDebug_test.py to handle system llvm

Follow up to PR #16613 and PR #16560

System LLVM installations use versioned commands
like instead of llvm-dwarfdump we have llvm-dwarfdump-10.

This commit adjusts the test to handle that.

Test change only - not reviewed.
@mppf mppf mentioned this pull request Nov 9, 2020
3 tasks
mppf added a commit that referenced this pull request Nov 9, 2020
Fix mac os x make docs

Follow-up to PR #16644 and #16560

This PR is intended to solve problems with `make docs` on Mac OS X.

* Don't replace python3 symlinks on mac os x
  * Our python3-wrapper exists to tolerate a different python3 path (e.g.
    in the case of installation) but it causes problems with the Python
    wrapper used for Mac OS X frameworks (in cpython,
    Mac/Tools/pythonw.c). I suspect there is a bug with this framework
    wrapper however I don't have a small reproducer. Setting the
    `PYTHONEXECUTABLE` variable did not help. However, calling e.g.
    `/usr/local/Cellar/python@3.9/3.9.0_1/Frameworks/Python.framework/Versions/3.9/Resources/Python.app/Contents/MacOS/Python`
    from our python3-wrapper allowed it to work (since this bypasses the
    pythonw.c wrapper).

* Update Makefiles to avoid `touch sphinx-build` which confused the issue

Reviewed by @ronawho - thanks!

- [x] make docs on mac os x with brew python3
- [x] make docs on Ubuntu 20.04
- [x] make docs on SLES12
mppf added a commit that referenced this pull request Nov 12, 2020
Create a python app for dependencies instead of venv

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!

- [x] full local testing
- [x] `test_rpm_module.bash`
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!
CaptainSharf pushed a commit to CaptainSharf/chapel that referenced this pull request Jan 16, 2021
Follow-up to PR chapel-lang#16560 and chapel-lang#16608

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

Signed-off-by: CaptainSharf <mohdsharf7407@gail.com>
CaptainSharf pushed a commit to CaptainSharf/chapel that referenced this pull request Jan 16, 2021
Follow-up to chapel-lang#16629 and chapel-lang#16560

---
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

2 participants