build: Propagate user-defined tools to native packages #23571
Closed
Conversation
This file contains hidden or 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
|
Concept ACK, perhaps we can use variable names like |
It makes sense within a build system which is based on Autotools. Is there any benefit in pure |
|
The benefit is just that the user can specify a different CC for build vs target, e.g.: $ make -C depends CC_FOR_BUILD=clang CC=x86_64-w64-mingw32-gcc |
Should new variables be limited to compilers only, i.e, |
Up to you! It seems like CC and CXX would be useful enough, so whatever's easy to do I guess |
shaavan
reviewed
Nov 23, 2021
There was a problem hiding this comment.
Concept ACK
This PR allows users to provide their values for CC and CXX flags. If the user does not give those values, the earlier method of assigning the build_$(build_os)$1, and build$(build_arch)$(build_os)$1.
Otherwise, these variables are assigned the value the user has provided. If that is empty, the earlier set value of these variables is used. If that's also empty, the default value is used.
As discussed in the PR description, this change will prevent unnecessary problems where the default tools are not available (by the same name) in the system.
P.s.: (As it's my first time reviewing PR involving makefiles, if some of my observations are erroneous, please do correct me.)
557d4d5 to
0ac8f23
Compare
|
Updated 557d4d5 -> 0ac8f23 (pr23571.01 -> pr23571.02). Added a commit which addresses @dongcarl's comment:
|
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
dongcarl
reviewed
Nov 30, 2021
|
Concept ACK, I've run into this problem in the past too. |
Guix builds: |
0ac8f23 to
2872533
Compare
|
Updated 0ac8f23 -> 2872533 (pr23571.02 -> pr23571.03). |
Guix builds: |
dongcarl
reviewed
Dec 21, 2021
2872533 to
bcfe469
Compare
|
Updated 2872533 -> bcfe469 (pr23571.03 -> pr23571.04). Addressed the recent @dongcarl's comments. |
|
Code Review ACK bcfe469 🚀 |
Guix builds: |
ccdf380 to
5f7b004
Compare
|
Rebased bcfe469 -> 5f7b004 (pr23571.04 -> pr23571.05). |
|
The PR description has been updated. |
|
Light code-review ACK 5f7b004 |
|
Rebased 5f7b004 -> 76d4d3b (pr23571.05 -> pr23571.06) due to the conflict with #19952. |
Guix builds on
|
This was referenced Apr 17, 2022
|
Rebased 76d4d3b -> ef802d0 (pr23571.06 -> pr23571.07). |
|
Are you still working on this? |
I am. This PR fixes a bug in our depends build system, and it's ready for further reviewing. |
|
Can probably be rebased for #23619. I'm not super convinced at adding all the |
I think it would be better if this was scoped to just the actual bug fix. |
|
Closing. Up for grabs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
On master (66636ca) build tools for native packages are set to defaults, even when such a tool is not available in the system by its default name:
On system without GCC installed, this causes an error:
This PR fixes this issue with introducing support of
{CC,CXX}_FOR_BUILDvariables.With this PR: