Conversation
nixpkgs-ci
bot
added
2.status: merge conflict
|
I think we can change comments from dates to releases, like "Added in 25.05" and enforce it so it will be easier to migrate later |
|
Yeah, that would be better, but with #442066 on the horizon I’m not personally invested in making significant changes here – just don’t want to have to migrate thousands of ancient aliases to |
wolfgangwalther
force-pushed
the
push-lpkmnukxqspk
branch
from
b7405c4 to
94ba119
Compare
|
Rebased this after merging #427017, solving a merge conflict that had already appeared along the way. |
nixpkgs-ci
bot
removed
the
2.status: merge conflict
wolfgangwalther
left a comment
wolfgangwalther
left a comment
There was a problem hiding this comment.
Obviously, I did not check every single change - but I went through every commit and I approve the intent / approach.
nixpkgs-ci
bot
added
the
12.approvals: 1
emilazy
force-pushed
the
push-lpkmnukxqspk
branch
from
94ba119 to
baba070
Compare
|
I’ve dropped the commit to add warnings to aliases that are new in 25.11, since that has the usual awkwardness relating to the overlapping support periods of unstable, stable, and oldstable. The only remaining decision is whether we want to go through a warning period for the long history of aliases from 25.05 and prior, or whether we’re okay making them throw immediately. I favour going straight to throws, because the replacements are already available in all supported versions, and it will result in much less churn when migrating things to structured aliases. We do already often add similar renames as throws from the get go. With structured aliases we’ll get an automatic transition through silent, warning, throw, and removal, so we won’t build up as big a backlog over time. |
emilazy
force-pushed
the
push-lpkmnukxqspk
branch
2 times, most recently
from
1f7ae96 to
4530faf
Compare
|
I'm all in favor of making these old aliases throws immediately: It's great that we have ideas for a fully automated workflow, it's great that we have scripts to theoretically do these transition periods right now as well - but in practice we didn't have that, so far. Just because we have an idea of "a warning should be better before a throw" doesn't mean we actually established that in practice. Thus, it's entirely fine to get rid of all the cruft now to get to a rather clean slate from which we can do better in the future. |
|
I am worried that drop of so many aliases without throw or warning will immediately affect many users and there is no easy way to find what is replacement for each alias, only looking at this pr diff. With warning replacement is clear and gives users time |
|
This PR does not drop any aliases that were not already throws in the 25.05 release tag. |
wolfgangwalther
force-pushed
the
push-lpkmnukxqspk
branch
from
4530faf to
b716213
Compare
|
Rebased to resolve merge conflict (simple addition of another throwing alias) |
|
Thank you, great work! |
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
This previously merged in `self`, so the flattened attribute set was not quite correct, but it’s ancient anyway.
Renamed [0]. [0] NixOS/nixpkgs#456065
Renamed [0]. [0] NixOS/nixpkgs#456065
Adapt the module to the changes introduced in NixOS#456065
`wrapGAppsHook` was renamed to `wrapGAppsHook3` in NixOS/nixpkgs#307077 (available since NixOS 24.05), and since NixOS/nixpkgs#456065 the old name no longer works. Update the old virt-manager package accordingly.
| vaapiVdpau = libva-vdpau-driver; # Added 2024-06-05 | ||
| validphys2 = throw "validphys2 has been removed, since it has a broken dependency that was removed"; # Added 2024-08-21 | ||
| util-linuxCurses = throw "'util-linuxCurses' has been renamed to/replaced by 'util-linux'"; # Converted to throw 2025-10-27 | ||
| utillinux = throw "'utillinux' has been renamed to/replaced by 'util-linux'"; # Converted to throw 2025-10-27 |
There was a problem hiding this comment.
The commit message on cede056 asserts that “This was fixed upstream in node2nix in 2020”, but it’s wrong: node2nix incorrectly prefers utillinux over util-linux, and therefore hits this throw.
There was a problem hiding this comment.
Right… Sorry about this. It has been reverted to a warning in #456527.
FWIW I would recommend moving off node2nix as it is unmaintained and we have better alternatives these days; we are steadily removing things from our nodePackages set that used it. I expect it will simply break when this becomes a throw next release.
|
The removal of the nixpkgs/pkgs/top-level/packages-config.nix Lines 18 to 20 in 829cffc |
Right, PR in #457521. |
Almost all these expressions already depend on depot. This is most notably the case for all expressions that are exported to github.com/wpcarro, so the current implementation of the package builds does not actually allow Nix builds independently from depot. Currently, they somewhat clumsily replicate the API of buildEmacsPackage so they can be used with depot. This unfortunately introduces a hard dependency on the emacsPackages alias in Nixpkgs which has been removed recently (see [also]). To deal with this, we may want to change the internal API of buildEmacsPackage or implement a shim. This is much easier to do when it only needs to happen in a single, central place. While I was at it, I moved all test suites into installCheckPhase which just requires -f package-activate-all instead of building a separate version of the package and a emacsWithPackages derivation. We can also change passthru.meta to meta without any problems. [also]: NixOS/nixpkgs#456065 (comment) Change-Id: Ic91766ee8f88f1786679f8d0bedc4bf7456f299c Reviewed-on: https://cl.tvl.fyi/c/depot/+/13814 Autosubmit: sterni <[email protected]> Tested-by: BuildkiteCI Reviewed-by: wpcarro <[email protected]> Reviewed-by: sterni <[email protected]>
Almost all these expressions already depend on depot. This is most notably the case for all expressions that are exported to github.com/wpcarro, so the current implementation of the package builds does not actually allow Nix builds independently from depot. Currently, they somewhat clumsily replicate the API of buildEmacsPackage so they can be used with depot. This unfortunately introduces a hard dependency on the emacsPackages alias in Nixpkgs which has been removed recently (see [also]). To deal with this, we may want to change the internal API of buildEmacsPackage or implement a shim. This is much easier to do when it only needs to happen in a single, central place. While I was at it, I moved all test suites into installCheckPhase which just requires -f package-activate-all instead of building a separate version of the package and a emacsWithPackages derivation. We can also change passthru.meta to meta without any problems. [also]: NixOS/nixpkgs#456065 (comment) Change-Id: Ic91766ee8f88f1786679f8d0bedc4bf7456f299c Reviewed-on: https://cl.tvl.fyi/c/depot/+/13814 Autosubmit: sterni <[email protected]> Tested-by: BuildkiteCI Reviewed-by: wpcarro <[email protected]> Reviewed-by: sterni <[email protected]>
Almost all these expressions already depend on depot. This is most notably the case for all expressions that are exported to github.com/wpcarro, so the current implementation of the package builds does not actually allow Nix builds independently from depot. Currently, they somewhat clumsily replicate the API of buildEmacsPackage so they can be used with depot. This unfortunately introduces a hard dependency on the emacsPackages alias in Nixpkgs which has been removed recently (see [also]). To deal with this, we may want to change the internal API of buildEmacsPackage or implement a shim. This is much easier to do when it only needs to happen in a single, central place. While I was at it, I moved all test suites into installCheckPhase which just requires -f package-activate-all instead of building a separate version of the package and a emacsWithPackages derivation. We can also change passthru.meta to meta without any problems. [also]: NixOS/nixpkgs#456065 (comment) Change-Id: Ic91766ee8f88f1786679f8d0bedc4bf7456f299c Reviewed-on: https://cl.tvl.fyi/c/depot/+/13814 Autosubmit: sterni <[email protected]> Tested-by: BuildkiteCI Reviewed-by: wpcarro <[email protected]> Reviewed-by: sterni <[email protected]>
|
Whyyy??? Who does this help? |
Cleaning up old cruft helps simplify the codebase. The alternative is to keep aliases forever, which usually isn't what we want. That said, I came here because several aliases I was using went straight from silent aliases to throws. I expected an intermediate stage, where aliases would warn (or warnOnInstantiate) for a release (or so) before eventually switching to throw. Am I wrong to assume that deprecated aliases should usually warn for a while before they start throwing? Am I wrong to assume that aliases that don't warn1 are implicitly not deprecated (yet)? Footnotes
|
This is an idea that has, unfortunately, never been fully implemented and lived in practice. We want this to happen in the future, in a structured way, that codifies those expectations. A very early draft is in #442066. Unfortunately, because nobody ever ran the related scripts over years, this didn't happen.
They'd already not be available for users running with Note that this only affects aliases that can be changed to their replacement in all supported NixOS versions, so even for downstream projects, the adjustment should be very simple. |
Hm, I'd always considered that To me, gating something behind I've always treated If I wanted to add a second name for something, I would use
Thanks, I'll take a look when I have time ❤️ |
The former is in fact an alias of `runCommand`. Many of these aliases are being phased out and dropped in a future release of nixpkgs. See: <NixOS/nixpkgs#456065> Aliases are going to be replaced by a more automated/structural approach, but that is orthogonal to the deprecation.
The former is in fact an alias of `runCommand`. Many of these aliases are being phased out and dropped in a future release of nixpkgs. See: <NixOS/nixpkgs#456065> Aliases are going to be replaced by a more automated/structural approach, but that is orthogonal to the deprecation.
10 participants
Based on @pbsds and @mdaniels5757’s work on the script in #427017, with some manual work on top as described in #427017 (review). Hopefully this is the last time we have to do this before structured aliases are a thing!
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.