Previously, we did not persist the $PATH env var between bitcoin's
configure-phase and its make-phase.
This meant that when configuring with $CONFIG_SITE set to one generated
by depends, we would be configuring with $depends_prefix/native/bin
prepended to $PATH, as described in depends/config.site.in.
However, when make-ing, the $PATH would be set to whatever the caller's
environment is, which didn't include $depends_prefix/native/bin.
To work around this, we:
1. Prepended $depends_prefix/native/bin to our CC, CXX, AR, RANLIB,
NM, and STRIP in order to invoke them with a full path as argv[0].
2. Manually prepended $depends_prefix/native/bin to $PATH in our
Gitian/Guix builds.
These workarounds are not ideal, and there is a better way. Essentially
we want to make sure that what we feature-test at configure-time _is_
what we will run at make-time.
-----
Using AC_ARG_VAR we ensure the following properties:
0. We no longer need the 2 workarounds described above. (They are
dropped in later commits)
1. By default and without any overrides, whatever $PATH is at
configure-time, is whatever $PATH is at make-time.
2. When configuring with depends' CONFIG_SITE, the
$depends_prefix/native/bin prepend to $PATH stays there unless
overridden manually by a ./configure PATH=... or make PATH=...
I believe that this makes using our depends system and our build system
much more consistent, and still allows overrides to be easily made by
those who need it.depends: Properly pass $PATH to configure and pin #20019
pull dongcarl wants to merge 8 commits into bitcoin:master from dongcarl:2020-09-resolve-PATH-prepending-madness changing 9 files +111 −34-
dongcarl commented at 7:52 pm on September 25, 2020: member
-
dongcarl added the label Build system on Sep 25, 2020
-
dongcarl added the label Needs gitian build on Sep 25, 2020
-
dongcarl added the label Needs Guix build on Sep 25, 2020
-
DrahtBot commented at 7:19 pm on September 26, 2020: member
Guix builds
-
DrahtBot removed the label Needs Guix build on Sep 26, 2020
-
DrahtBot commented at 4:23 pm on September 27, 2020: member
Gitian builds
-
DrahtBot removed the label Needs gitian build on Sep 27, 2020
-
MarcoFalke added the label Needs gitian build on Sep 28, 2020
-
dongcarl marked this as a draft on Sep 28, 2020
-
DrahtBot commented at 10:03 pm on September 28, 2020: member
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #20419 (build: set minimum supported macOS to 10.14 by fanquake)
- #18902 (Bugfix: Only use git for build info if the repository is actually the right one by luke-jr)
- #18818 (Fix release tarball generated by gitian by luke-jr)
- #14066 (gitian-linux: Build binaries for 64-bit POWER by luke-jr)
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.
-
DrahtBot commented at 2:56 am on September 29, 2020: member
Gitian builds
-
DrahtBot removed the label Needs gitian build on Sep 29, 2020
-
fanquake commented at 7:15 am on September 29, 2020: memberConcept ACK thing
-
dongcarl commented at 9:24 pm on September 29, 2020: memberHehe, I’ve found that this issue is a little more complicated than what I’ve described in the original description, so keeping it as draft until I figure out a clean solution.
-
6c7e8f067d
depends: Allow relative CONFIG_SITE path env var
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 -
618cbd2c1a
lint: Also lint files with shellcheck directive
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.
-
46756a6987
depends: Fix PYTHONPATH setting in config.site.in
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
-
dongcarl force-pushed on Nov 12, 2020
-
dongcarl force-pushed on Nov 13, 2020
-
dongcarl force-pushed on Nov 13, 2020
-
dongcarl force-pushed on Nov 13, 2020
-
1439448be5
depends: Properly pass $PATH to configure and pin
Previously, we did not persist the $PATH env var between bitcoin's configure-phase and its make-phase. This meant that when configuring with $CONFIG_SITE set to one generated by depends, we would be configuring with $depends_prefix/native/bin prepended to $PATH, as described in depends/config.site.in. However, when make-ing, the $PATH would be set to whatever the caller's environment is, which didn't include $depends_prefix/native/bin. To work around this, we: 1. Prepended $depends_prefix/native/bin to our CC, CXX, AR, RANLIB, NM, and STRIP in order to invoke them with a full path as argv[0]. 2. Manually prepended $depends_prefix/native/bin to $PATH in our Gitian/Guix builds. These workarounds are not ideal, and there is a better way. Essentially we want to make sure that what we feature-test at configure-time _is_ what we will run at make-time. ----- In order to fix this, we introduce a new, non-precious ./configure variable called $PATH_TEMPLATE, which is a shell snippet to be expanded at ./configure-time to form $PATH. Using PATH_TEMPLATE means that: 0. We no longer need the 2 workarounds described above. (They are dropped in later commits) 1. By default and without any overrides, whatever $PATH is used at configure-time, is whatever $PATH is at make-time. 2. When configuring with depends' CONFIG_SITE, the $depends_prefix/native/bin prepend to $PATH stays there unless overridden manually by a ./configure PATH_TEMPLATE=... or make PATH_TEMPLATE=... 3. We have the flexibility to reference and compose configure variables, which can be used for making our WRAP_DIR facilities work properly in gitian. For example, PATH_TEMPLATE="$WRAP_DIR"':${depends_path_fragment}:${PATH}' NOTE: I would recommend reviewers to look at the subsequent commits in this patchset, which demonstrate the simplifications that this change brings about. ----- Example Scenarios: Example 1: Plain configure (the default) Invoking: $ ./configure Means that $PATH_TEMPLATE gets a default value of literally: '${PATH}' which is then expanded to form the PATH to be used for the rest of ./configure and make Example 2: Configure with depends CONFIG_SITE Invoking: $ env CONFIG_SITE=depends/.../share/config.site ./configure Means that $PATH_TEMPLATE gets an alternate default value of literally: '${depends_path_fragment}${PATH_SEPARATOR}${PATH}' which is then expanded to form the PATH to be used for the rest of ./configure and make Example 3: Configure with custom PATH_TEMPLATE Invoking: $ WRAP_DIR=/home/satoshi/wrapped $ env CONFIG_SITE=depends/.../share/config.site \ ./configure PATH_TEMPLATE="$WRAP_DIR"':${depends_path_fragment}:${PATH}' Means that both $depends_path_fragment and $PATH are left to be expanded at configure-time since they are single-quoted while $WRAP_DIR is expanded by the shell at invocation time. Therefore $PATH_TEMPLATE gets a value of literally: '/home/satoshi/wrapped:${depends_path_fragment}:${PATH}' which is then expanded to form the PATH to be used for the rest of ./configure and make -
465afb65d3
depends: Drop argv[0] prepend workaround for $PATH
See previous commit for a full description.
-
479aca4fc4
releases: Drop unnecessary depends PATH workarounds
Since pointing to a CONFIG_SITE generated by our depends system will make ./configure prepend the appropriate depends $PATH fragment, we no longer need these now-redundant workarounds which tries to do the same prepending.
-
cedfba360a
gitian: Use the new PATH_TEMPLATE configure variable
Use the new PATH_TEMPLATE configure variable to properly setup our $PATH, such that the search order at BOTH configure-time and build-time looks like: 1. The value of WRAP_DIR 2. The value of depends_path_fragment rest. The value of PATH This is ultimately what we wanted in the first case, but previously this was not achievable and led to many workarounds that can now be removed (like the GENISOIMAGE= configure variable specification removed in this commit).
-
abfa6574ac
builds: Run libdmg-hfsplus's DMG tool in make deploy
It would seem that previously, because of the $PATH-related problems described in a previous commit in this patchset, the compression of the .iso file to a .dmg file was done outside of `make deploy' in order to use the faketime-wrapped version of libdmg-hfsplus's DMG tool. This is no longer a necessary workaround now that the underlying $PATH-related issues are solved.
-
dongcarl force-pushed on Nov 13, 2020
-
dongcarl commented at 9:32 pm on November 23, 2020: memberAfter another rethink, I was able to solve my original problems without this, so I’m going to close this PR for now.
-
dongcarl closed this on Nov 23, 2020
-
DrahtBot locked this on Feb 15, 2022
Labels
Build system
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 00:12 UTC
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 00:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me
More mirrored repositories can be found on mirror.b10c.me