Revert obsolete aarch64 musl hacks #283825
Conversation
This reverts commit 45584b3. These work fine now that we have an up-to-date bootstrap tarball for this platform.
This reverts commit 661dfd8. This problem has gone away now that aarch64-unknown-linux-musl has an up-to-date bootstrap.
This reverts commit e22d0b4. No longer needed now that we have an up-to-date aarch64-unknown-linux-musl bootstrap. Since the original commit, patchelf_0_14 was renamed to patchelfStable. There's no longer any need for it to have a name other than "patchelf", but I've kept "patchelfStable" around as an alias in case anybody's using it.
This reverts commit 3838a0a. This was previously used on aarch64-unknown-linux-musl, but now that that platform has an up-to-date bootstrap and can build current patchelf, there's no longer any need to keep this around.
alyssais
requested review from
Ericson2314,
Mindavi,
sternenseemann,
vcunat and
yu-re-ka
Mindavi
left a comment
Mindavi
left a comment
There was a problem hiding this comment.
Good to see that this happened and the hacks can go again ✨
ofborg
bot
added
8.has: clean-up
| patchelf_0_13 = callPackage ../development/tools/misc/patchelf/0.13.nix { | ||
| patchelf = patchelfStable; | ||
| }; |
There was a problem hiding this comment.
Attribute is being removed without a throwing alias
There was a problem hiding this comment.
Is that really mandatory? Isn't it pretty obvious what's happened if a versioned attribute you were using disappears? (To the extent it's likely that anybody was explicitly using this version introduced just to be the default on aarch64-musl in the first place…)
There was a problem hiding this comment.
While the API surface of Nixpkgs isn't explicitly declared somewhere yet, it's pretty much accepted that top-level attributes are effectively part of that. It's also not declared explicitly what stability guarantees Nixpkgs gives about such an API, but I think it's also fairly accepted that attributes should continue existing, that's why we have aliases.
Ideally I'd favor a deprecation message for one release, but in the absence of that we should at least have throwing aliases. Just removing attributes entirely is a bit too much.
And we should assume that any part of the API ends up getting used. Even if we don't hear from them, there likely is some user (or more) out there getting annoyed over attributes randomly disappearing. We can improve those users experience by being more diligent with aliases, I think we should go for that :)
There was a problem hiding this comment.
Don't bother with this if you don't want to (though I'd merge your PR if you did). But just for the future I think it would be good to have aliases for all removed attributes.
There was a problem hiding this comment.
And we should assume that any part of the API ends up getting used. Even if we don't hear from them, there likely is some user (or more) out there getting annoyed over attributes randomly disappearing.
We change little-used APIs all the time. There's no way it's possible to maintain our ability to iterate if we have to keep every reachable attribute working and unchanging for a two-cycle deprecation period.
There was a problem hiding this comment.
if we have to keep every reachable attribute working and unchanging for a two-cycle deprecation period
Yeah, but that's not the case for a throwing alias, which costs effectively nothing
3 participants
Description of changes
— @sternenseemann, 2022-04-22
I started working on getting aarch64-unknown-linux-musl into shape in April 2022. At that time, the stdenv didn't even build, and it hadn't built for years, due to woefully outdated bootstrap binaries. To get newer bootstrap binaries, we needed to get far enough with the old bootstrap binaries to be able to build new ones, getting Hydra to build them, and then getting those new binaries uploaded to tarballs.nixos.org. Getting that far with the old bootstrap binaries adding some hacks (like downgrading patchelf). Getting Hydra to build them required a complicated refactoring, as the previous scheme was designed around system doubles so there was no way to express a alternative choice of libc. This refactoring took several attempts.
But now, as of #283492, my two-year-long journey has finally reached its end. We have up-to-date bootstrap binaries, and we can revert the hacks we needed along the way to bring us to this point. So, as was always intended, this is a revert of #169764, as well as one extra hack added in #258564.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.