Make Python detection optional and more portable #8731
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
behlendorf
added
Status: Code Review Needed
12 tasks
|
This should fix #8809. |
behlendorf
requested changes
May 28, 2019
behlendorf
requested changes
May 30, 2019
behlendorf
approved these changes
May 30, 2019
rlaager
suggested changes
Jun 1, 2019
rlaager
approved these changes
Jun 1, 2019
behlendorf
added
Status: Accepted
12 tasks
|
@behlendorf If it's alright, I'd like to push a revision inspired by @eli-schwartz's suggestion. |
|
@freqlabs absolutely, please go ahead update the PR. |
behlendorf
added
Status: Code Review Needed
eli-schwartz
reviewed
Jun 4, 2019
Previously, --without-python would cause ./configure to fail. Now it is able to proceed, and the Python scripts will not be built. Use portable parameter expansion matching instead of nonstandard substring matching to detect the Python version. This test is duplicated in several places, so define a function for it. Don't assume the full path to binaries, since different platforms do install things in different places. Use AC_CHECK_PROGS instead. When building without Python, also build without pyzfs. Sponsored by: iXsystems, Inc. Signed-off-by: Ryan Moeller <ryan@freqlabs.com>
Codecov Report
@@ Coverage Diff @@
## master #8731 +/- ##
==========================================
+ Coverage 78.67% 78.67% +<.01%
==========================================
Files 382 382
Lines 117771 117780 +9
==========================================
+ Hits 92651 92659 +8
- Misses 25120 25121 +1
Continue to review full report at Codecov.
|
behlendorf
approved these changes
Jun 4, 2019
behlendorf
added
Status: Accepted
eli-schwartz
reviewed
Jun 5, 2019
| AC_MSG_ERROR([Cannot find $PYTHON in your system path]) | ||
| AS_IF([test $PYTHON != :], [ | ||
| AS_IF([$PYTHON --version >/dev/null 2>&1], | ||
| [AM_PATH_PYTHON([2.6], [], [:])], |
There was a problem hiding this comment.
This check is actually redundant, since AM_PATH_PYTHON will already try to run $PYTHON -c some-inline-python-script-which-consults-sys.hexversion.
There was a problem hiding this comment.
Okay, a bit late, hehe.
There was a problem hiding this comment.
Indeed, well to remove the redundancy let's open a new PR.
behlendorf
pushed a commit
that referenced
this pull request
Jun 7, 2019
Previously, --without-python would cause ./configure to fail. Now it is able to proceed, and the Python scripts will not be built. Use portable parameter expansion matching instead of nonstandard substring matching to detect the Python version. This test is duplicated in several places, so define a function for it. Don't assume the full path to binaries, since different platforms do install things in different places. Use AC_CHECK_PROGS instead. When building without Python, also build without pyzfs. Sponsored by: iXsystems, Inc. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Richard Laager <rlaager@wiktel.com> Reviewed-by: Eli Schwartz <eschwartz93@gmail.com> Signed-off-by: Ryan Moeller <ryan@freqlabs.com> Closes #8809 Closes #8731
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When detecting the Python version, don't assume the path to binaries.
Make sure --without-python works.
Motivation and Context
These changes make it easier to build the zfsonlinux code on platforms like FreeBSD, where
trueis/usr/bin/trueandpython3is/usr/local/bin/python3or might not exist at all (i.e. in the base system).Description
Previously, --without-python would cause ./configure to fail. Now it is able to proceed, and the Python scripts will not be built.
Use portable regular expression matching instead of nonstandard substring expansion to detect the Python version. This test is duplicated in several places, so define a function for it.
Don't hard-code paths to binaries such as
trueandpython3.The
cmd/subdirs have been split up to only build the Python subdirs when configured to use Python. Coincidentally, Linux-specific subdirs are also split out from the same line, reducing the diff from ZoF.How Has This Been Tested?
These changes are based on work in progress in porting zol to FreeBSD. I have built ZFS on FreeBSD with these changes.
Types of changes
Checklist:
Signed-off-by.