Merged
Conversation
In practice, this is a strictly stronger condition than target != build as we never have build = target != host. Really, the attribute should be removed altogether, but for now we make it work for plain libraries, which do not care about the target platform. In the few cases where the compilers use this and actually care about the target platform, I'll manually change them to use `targetPlatform` instead.
The previous commit redefines `stdenv.cross` for the sake of normal libaries, the most common use-case of that attribute. Some compilers however relied on the old definition so we have them use `targetPlatform` instead. This special casing is fine because we eventually want to remove `stdenv.cross` and use either `hostPlatform` or `targetPlatform` instead.
Before all overrides were also pruned in the previous stage, now only gcc and binutils are, because they alone care about about the target platform. The rest of the overrides don't, so it's better to preserve them in order to avoid spurious rebuilds.
|
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @peti, @edolstra, @vizanto, @LnL7 and @copumpkin to be potential reviewers. |
7 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation for this change
stdenv.crossis a silly attribute that needs to go leaving the well-definedhostPlatformandtargetPlatform. This PR doesn't remove it, but changes its definition: before it tracked the target platform which is sometimes more useful for compilers, and now it tracks the host platform which is more useful for everything else. Most usages are libraries, falling in the "everything else" category, so changing the definition makes sense to appease the majority. The few compiler (gcc in particular) uses that exist I remove to usetargetPlatform--- preserving correctness and becoming more explicit in the process.I would also update the documentation aside mentioning
stdenv.crossas deprecated, but the definition given actually erroneously assumes this PR is already merged!This is split off from #25047
Things done
I commandeered and old jobset of mine to create http://hydra.nixos.org/jobset/nixpkgs/pr-25194 to test, because I couldn't figure out how to run a hydra job locally. http://hydra.nixos.org/eval/1353037?compare=cross-trunk shows no regressions. Yay!