Skip to content

depends: Improve id string robustness #20629

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

Merged
merged 3 commits into from
Feb 15, 2021

Conversation

dongcarl
Copy link
Contributor

Environment variables and search paths can drastically effect the
operation of build tools.

Include these in our id string to mitigate against false cache hits.

Note to builders: This will invalidate all depends output caches in BASE_CACHE

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 11, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

@laanwj
Copy link
Member

laanwj commented Dec 14, 2020

Concept ACK. If I understand it correctly, this includes only specific targeted environment variables and not the entire env?

@luke-jr
Copy link
Member

luke-jr commented Dec 14, 2020

gcc -v -E - seems to lose COLLECT_LTO_WRAPPER...

@dongcarl
Copy link
Contributor Author

Concept ACK. If I understand it correctly, this includes only specific targeted environment variables and not the entire env?

Yes! It automatically includes any env var that starts with the name of the tool (e.g. STRIP_* for strip), and if there are particular ones we know about (e.g. ZERO_AR_DATE) we can just append it like so: 6ee5ee4 (#20629)

@dongcarl
Copy link
Contributor Author

gcc -v -E - seems to lose COLLECT_LTO_WRAPPER...

Right, perhaps someone knows a better invocation here? I'm guessing COLLECT_LTO_WRAPPER is omitted because the linker is not going to run?

@dongcarl dongcarl force-pushed the 2020-12-improve-depends-id-string branch 2 times, most recently from 4b9aaca to f792069 Compare December 16, 2020 19:25
@dongcarl
Copy link
Contributor Author

Pushed 4b9aaca -> f792069

  1. Added an invocation of CC/CXX with just "-v" to pick up COLLECT_LTO_WRAPPER
  2. Added comment about COLLECT_LTO_WRAPPER and the weird fd indirection trick

@DrahtBot
Copy link
Contributor

Guix builds

File commit 8452f92
(master)
commit bcf4efe
(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz 47fe896930c0d771... 4f389e051495a470...
*-aarch64-linux-gnu.tar.gz e09c8c469d3844de... d1644b8f32f58e0f...
*-arm-linux-gnueabihf-debug.tar.gz e167617ff0806e89... 70159bbc83ab5357...
*-arm-linux-gnueabihf.tar.gz 6328582ec8f6a043... 96cea72f7b75e077...
*-riscv64-linux-gnu-debug.tar.gz cdc1795bca5d99ad... f930151ff6a3b114...
*-riscv64-linux-gnu.tar.gz 6af595f2cfc8f2d1... c376f2f162571024...
*-win-unsigned.tar.gz 40811556f4d465fb... d44c161c5528ab26...
*-win64-debug.zip e7f88111b53f15c3... a79a8366828ec6a7...
*-win64-setup-unsigned.exe 2afa8ac9334944ba... 47ce1c26ea8e2a24...
*-win64.zip 5a69a9f481267790... 6e3be0d25c9292a5...
*-x86_64-linux-gnu-debug.tar.gz 279d5ed7554ac6d2... 0ae3bdefa4302546...
*-x86_64-linux-gnu.tar.gz b3281357f0ba632a... 9e09f09629cf3413...
*.tar.gz 6e9d2dc19aa6fb92... 0198417c9dd7136d...
guix_build.log 0193e388f34b83ef... 4403833a99495890...
guix_build.log.diff c952affa422fa987...

@laanwj
Copy link
Member

laanwj commented Dec 18, 2020

Code review ACK f792069

@DrahtBot
Copy link
Contributor

Gitian builds

File commit 816314e
(master)
commit a130add
(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz 92c094a57f28b2fe... 1548d056613d4282...
*-aarch64-linux-gnu.tar.gz 6406c70036869155... 35279dc260fdb7d0...
*-arm-linux-gnueabihf-debug.tar.gz fa1dcb7738417481... ca60e75f26aec626...
*-arm-linux-gnueabihf.tar.gz ba0552af3463d066... 75f4dbc9379560a9...
*-osx-unsigned.dmg b38904a51c9c12d5... daff62c8b99e1c3f...
*-osx64.tar.gz 81c94a20c6fe25d9... 91278b789e63ee3f...
*-riscv64-linux-gnu-debug.tar.gz 5199aceb15600fb9... 96a09768b7d1d42f...
*-riscv64-linux-gnu.tar.gz 79881c3a59eecd00... e3d5f74e06f90228...
*-win64-debug.zip 9f6dc265cf93e377... 1edd6655f5b1714b...
*-win64-setup-unsigned.exe 4f64508a4629cde1... 37a5366b9495a77b...
*-win64.zip 85b0b7372d64928b... 42f64b3e2ceb78a2...
*-x86_64-linux-gnu-debug.tar.gz db8b3d3f3967dbb4... de7b27b3711a7318...
*-x86_64-linux-gnu.tar.gz 10c06c92f10e4d8d... 9b4bdc05eda43091...
*.tar.gz b47efbff1aff7eeb... 0a9228076efb94af...
bitcoin-core-linux-22-res.yml 6b70a61e243f8979... 51e0aace21f10ff7...
bitcoin-core-osx-22-res.yml c492c16c3445ba68... 7d32f7a68583a9ce...
bitcoin-core-win-22-res.yml a4afeb86eac5227b... e52f2cf7e4e4b43b...
linux-build.log 6e3648941ec583a8... 92e7f663c31a113b...
osx-build.log 951cd893b8181a69... 5d243fadaa77c6d9...
win-build.log 09e44a321a721c70... 3bcbf90cdf01d0df...
bitcoin-core-linux-22-res.yml.diff 1bae578e73833145...
bitcoin-core-osx-22-res.yml.diff 7ae483317f86820a...
bitcoin-core-win-22-res.yml.diff 4991a0017bd60e5a...
linux-build.log.diff 419e5a5bf80f4867...
osx-build.log.diff f19ccbc6242d8138...
win-build.log.diff 311f56737cd53c51...

Previously, if the value contained syntax that was meaningful to make,
the printing would fail. Quoting properly avoids this.
Environment variables and search paths can drastically effect the
operation of build tools.

Include these in our id string to mitigate against false cache hits.
@dongcarl dongcarl force-pushed the 2020-12-improve-depends-id-string branch from f792069 to 5200929 Compare February 5, 2021 19:41
@laanwj laanwj changed the title depends: Improve id string robustness depends: Improve id string robustness Feb 9, 2021
@laanwj
Copy link
Member

laanwj commented Feb 15, 2021

re-ACK 5200929

@laanwj laanwj merged commit 51397c0 into bitcoin:master Feb 15, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 15, 2021
5200929 depends: Include GUIX_ENVIRONMENT in id string (Carl Dong)
4c7d418 depends: Improve id string robustness (Carl Dong)
b3bdff4 build: Proper quoting for var printing targets (Carl Dong)

Pull request description:

  ```
  Environment variables and search paths can drastically effect the
  operation of build tools.

  Include these in our id string to mitigate against false cache hits.
  ```

  Note to builders: This will invalidate all depends output caches in `BASE_CACHE`

ACKs for top commit:
  laanwj:
    re-ACK 5200929

Tree-SHA512: e70c98da89cde90dc54bc3be89b925787cf94bbf246e27cc9345816b312073d78a02215448f731f21d8cf033c455234a2377ff1d66c00e1f3db69c9c9687d027
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants