-
Notifications
You must be signed in to change notification settings - Fork 35.6k
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
depends: Various config.site.in improvements and linting #20359
Conversation
Previously, if ./configure was invoked with: ``` $ env CONFIG_SITE=depends/x86_64-pc-linux-gnu/share/config.site ./configure ``` Where $CONFIG_SITE was a relative path, ./configure would fail with the following misleading output: ``` checking for boostlib >= 1.58.0 (105800)... yes checking whether the Boost::System library is available... yes configure: error: Could not find a version of the Boost::System library! ``` Fully resolving depends_prefix in config.site.in fixes this. To make sure that there are no other side effects I ran a diff on the config.status generated by: 1. The scripts prior to this change with CONFIG_SITE set to a full path: env CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure 2. The scripts after this change with CONFIG_SITE set to a relative path: env CONFIG_SITE=depends/x86_64-pc-linux-gnu/share/config.site ./configure And it looks good! Diff: https://paste.sr.ht/~dongcarl/95b469fbc555c128046e85723d87a9082a754f6b
Files like config.site.in are not referenced by any other script in our tree, so we need to mark it manually with a "shellcheck shell=" directive and make sure that shellcheck is run on them.
Previously, when running ./configure: 1. With CONFIG_SITE pointed to our depends config.site.in, and 2. PYTHONPATH was not set either in the environment or by the user The configure would output something like: PYTHONPATH='depends/x86_64-pc-linux-gnu/share/../native/lib/python3/dist-packages:' When we really mean: PYTHONPATH='depends/x86_64-pc-linux-gnu/share/../native/lib/python3/dist-packages' ...without the colon This change makes sure that: 1. There's no trailing colon, and 2. We use the $PATH_SEPARATOR variable instead of a colon
Concept ACK Nice to see those files being modernized and tidied up. Even nicer having a guarantee they will stay that way thanks to linting! |
Gitian builds
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
ACK 46756a6 These are obvious improvements to the shell script. |
…linting - PYTHONPATH change breaks package detection for some reason
…linting - PYTHONPATH change breaks package detection for some reason
…linting - PYTHONPATH change breaks package detection for some reason
This changeset:
CONFIG_SITE
env var to be a relative path rather than requiring an absolute oneconfig.site.in
file withshellcheck
in our linting scriptsPYTHONPATH
var sensibly inconfig.site.in
Please see commit messages for more details