depends: Use CC_FOR_BUILD for config.guess #29963

1. 
   hebasto commented at 7:01 pm on April 25, 2024: member

   On the master branch @ 3c88eac28e8984893746caebb313dc3b2fca90db, consider the following commands in the depends subdirectory:

   0$ make print-build HOST=i686-pc-linux-gnu CC="clang -m32"
   1build=x86_64-pc-linux-gnu
   2$ make print-host HOST=i686-pc-linux-gnu CC="clang -m32"
   3host=i686-pc-linux-gnu
   

   The printed variable values are expected.

   However, switching the CC variable context from Makefile to the shell environment breaks expectations:

   0$ CC="clang -m32" make print-build HOST=i686-pc-linux-gnu
   1build=i686-pc-linux-gnu
   2$ CC="clang -m32" make print-host HOST=i686-pc-linux-gnu
   3host=i686-pc-linux-gnu
   

   This PR fixes this issue.

2. hebasto added the label Build system on Apr 25, 2024
3. 
   DrahtBot commented at 7:01 pm on April 25, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

4. hebasto referenced this in commit 370c4e9161 on Apr 25, 2024
5. 
   theuni commented at 7:20 pm on April 25, 2024: member

   Where/why is this an issue?
6. hebasto referenced this in commit 727cc73bcc on Apr 25, 2024
7. 
   hebasto commented at 7:27 pm on April 25, 2024: member

   Where/why is this an issue?

   I faced this issue during my work on the CMake staging branch when the CC environment variable was defined by the CI – https://github.com/hebasto/bitcoin/actions/runs/8822538616/job/24220973382.

   The log shows no cross-compiling, but it is cross-compiling to i686-pc-linux-gnu.

8. 
   theuni commented at 9:01 pm on April 25, 2024: member

   Surely this isn’t the only place env vars like this would cause issues? Feels like a cat-and-mouse game.
9. 
   maflcko commented at 8:11 am on April 26, 2024: member

   The log shows no cross-compiling, but it is cross-compiling to i686-pc-linux-gnu.

   Can you link to the exact lines in the log that show “no cross-compiling”.

10. 
    hebasto commented at 9:29 am on April 26, 2024: member

    The log shows no cross-compiling, but it is cross-compiling to i686-pc-linux-gnu.

    Can you link to the exact lines in the log that show “no cross-compiling”.

    https://github.com/hebasto/bitcoin/actions/runs/8822538616/job/24220973382#step:14:245:

    0Cross compiling ....................... FALSE
    

    Please note that that was a PR branch from the CMake migration project. That branch detects cross-compiling basing on host and build values when building with depends.

11. 
    fanquake commented at 9:51 am on April 26, 2024: member

    Surely this isn’t the only place env vars like this would cause issues? Feels like a cat-and-mouse game.

    Yea, as-is this doesn’t seem like a great fix, and may just break other things?

    A better diff might be something like:

     0diff --git a/depends/Makefile b/depends/Makefile
     1index 005d9696fb..091511758d 100644
     2--- a/depends/Makefile
     3+++ b/depends/Makefile
     4@@ -51,7 +51,7 @@ FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources
     5 C_STANDARD ?= c11
     6 CXX_STANDARD ?= c++20
     7 
     8-BUILD = $(shell ./config.guess)
     9+BUILD = $(shell CC_FOR_BUILD=$CC ./config.guess)
    10 HOST ?= $(BUILD)
    11 PATCHES_PATH = $(BASEDIR)/patches
    12 BASEDIR = $(CURDIR)
    

    which would at least be using the option that is meant to be used for this.

12. depends: Use `CC_FOR_BUILD` for `config.guess`

    Co-authored-by: Michael Ford <fanquake@gmail.com>

    2f664151a0
13. hebasto force-pushed on Apr 26, 2024
14. 
    hebasto commented at 10:05 am on April 26, 2024: member

    A better diff might be something like: … which would at least be using the option that is meant to be used for this.

    I agree. Implemented.

    Thanks!

15. hebasto renamed this:
    depends: Do not consider `CC` environment variable for detecting system
    depends: Use `CC_FOR_BUILD` for `config.guess `
    on Apr 26, 2024
16. 
    theuni commented at 12:47 pm on April 26, 2024: member

    However, switching the CC variable context from Makefile to the shell environment breaks expectations

    Arguably that’s because this wasn’t intended to be supported :)

    I suppose this fix is reasonable, though supporting env vars like this seems quite brittle.

17. hebasto referenced this in commit 21bee57cb6 on Apr 30, 2024
18. hebasto referenced this in commit 516ecf1c8e on May 2, 2024
19. hebasto referenced this in commit 729632c54f on May 2, 2024
20. 
    fanquake commented at 3:37 pm on May 31, 2024: member

    Is this still an issue given recent CMake changes?
21. 
    hebasto commented at 10:10 am on June 3, 2024: member

    Is this still an issue given recent CMake changes?

    Yes. Tested with the master branch @ 80bdd4b6beb878c95478b5623c9f9ff0b948ad57.

    And this PR still fixes it.

22. DrahtBot added the label CI failed on Jun 18, 2024
23. DrahtBot removed the label CI failed on Jun 18, 2024

Contributors
hebasto DrahtBot theuni maflcko fanquake

Labels
Build system