Avoid using aliases, disallow aliases in release #43692
Merged
Conversation
This file contains 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
jtojnar
reviewed
Jul 17, 2018
| @@ -1,4 +1,4 @@ | |||
| { stdenv, fetchFromGitHub, gettext, poppler_qt5, qt5 , pkgconfig }: | |||
| { stdenv, fetchFromGitHub, gettext, poppler, qt5 , pkgconfig }: | |||
There was a problem hiding this comment.
poppler is not the same as poppler_qt5.
There was a problem hiding this comment.
It shold be the qt5 poppler if you do libsForQt5.callPackage. I forgot to do it here though.
There was a problem hiding this comment.
But Edit: Oh, you mean poppler artribute should be the glib variant, not the qt one.libsForQt5.callPackage ../applications/graphics/ktikz { }
|
Why, though? |
|
Nice! |
peti
approved these changes
Jul 18, 2018
FRidh
reviewed
Jul 18, 2018
pkgs/top-level/aliases.nix
Outdated
| @@ -75,8 +75,6 @@ mapAliases ({ | |||
| devicemapper = lvm2; # added 2018-04-25 | |||
| digikam5 = digikam; # added 2017-02-18 | |||
| dmtx = dmtx-utils; # added 2018-04-25 | |||
| docbook5_xsl = docbook_xsl_ns; # added 2018-04-25 | |||
There was a problem hiding this comment.
Aren't these aliases also intended for outside Nixpkgs?
There was a problem hiding this comment.
Yes mostly. But a few are used so extensively it didn't seem worth it to rewrite them all here (lzma = xz, udev = systemd). Others were a little bit questionable aliases in the first place (mysql = mariadb,kerberos = libkrb5).
GrahamcOfBorg
added
6.topic: GNOME
matthewbauer
changed the title
matthewbauer
force-pushed
the
disallow-aliases-in-release
branch
2 times, most recently
from
103dcda
to
870cb99
Compare
|
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: lollypop-portal, qtikz Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: lollypop-portal, qtikz Partial log (click to expand)
|
|
@GrahamcOfBorg build nixnote2 flacon texstudio dmraid lollypop-portal |
|
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: nixnote2, flacon, texstudio, dmraid, lollypop-portal Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: nixnote2, flacon, texstudio, dmraid, lollypop-portal Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: lollypop-portal, qtikz Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: nixnote2, flacon, dmraid, lollypop-portal The following builds were skipped because they don't evaluate on aarch64-linux: texstudio Partial log (click to expand)
|
otherwise we get this error on evaluation: derivation 'lollypop-portal' has invalid meta attribute 'override' derivation 'lollypop-portal' has invalid meta attribute 'overrideDerivation'
This makes the command ‘nix-env -qa -f. --arg config '{skipAliases =
true;}'’ work in Nixpkgs.
Misc...
- qtikz: use libsForQt5.callPackage
This ensures we get the right poppler.
- rewrites:
docbook5_xsl -> docbook_xsl_ns
docbook_xml_xslt -> docbook_xsl
diffpdf: fixup
This will make hydra & ofborg ignore aliases when evluating
matthewbauer
force-pushed
the
disallow-aliases-in-release
branch
from
041d9f5
to
aab3182
Compare
|
|
|
Success on x86_64-linux (full log) Attempted: lollypop-portal Partial log (click to expand)
|
|
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: lollypop-portal Partial log (click to expand)
|
76999cc changed some hashes resulting in this PR being technically a mass rebuild. To avoid this, I am restoring some of the hashes (even though it seems silly). My main goal is to get this PR merged quickly as treewide changes like this get out-of-date quickly. This commit should be reverted on the next mass rebuild.
|
Success on aarch64-linux (full log) Attempted: lollypop-portal Partial log (click to expand)
|
|
Sorry for the rush but these PRs can fall out of date fast. I will plan to merge this PR once we can get the number of rebuilds under 100. |
|
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: lollypop-portal Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: lollypop-portal Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: lollypop-portal Partial log (click to expand)
|
|
But in a change like this which affects just names on the Nix level, every rebuild is a unintended change thus a bug, no? |
|
Yeah, I wonder if we should revert this. No hashes should change as a result of this PR, @matthewbauer. |
jtojnar
reviewed
Jul 19, 2018
| @@ -1,5 +1,5 @@ | |||
| { stdenv, fetchzip, atk, cairo, dmd, gdk_pixbuf, gnome3, gst_all_1, librsvg | |||
| , pango, pkgconfig, substituteAll, which }: | |||
| , pango, pkgconfig, substituteAll, which, gst_plugins_base }: | |||
There was a problem hiding this comment.
This is wrong, the derivation is using gst-plugins-base from gst_all_1. You pass in a legacy gst-plugins-base (thought it will not be used).
|
Comparing the changed derivations with #!/usr/bin/env nix-shell
#!nix-shell -i bash --pure -p curl -p git -p nix -p indent
paths=$(curl -L https://gist.github.com/GrahamcOfBorg/0d8b33ede1f4e2442d148feaffb43528/raw/77b80187e2c8e4b78cffc367d14dff298234310c/Changed%2520Paths | cut -f2 | sort | uniq)
for path in $paths; do
echo '==========' $path '=========='
git checkout d7d31fea7e7eef8ff4495e75be5dcbb37fb215d0 -q
before=$(nix-instantiate -A $path 2> /dev/null)
# beforebld=$(nix-build -A $path --no-out-link)
git checkout 4ed7a4b993d3e03e16eb5dbe0f1ea3061d2eb475 -q
after=$(nix-instantiate -A $path 2> /dev/null)
# afterbld=$(nix-build -A $path --no-out-link)
# nix-diff $beforebld $afterbld
if [ "$after" != "$before" ]; then diff -u <(cat "${before%!*}" | indent) <(cat "${after%!*}" | indent); fi
doneI see some re-orderings: Weirdly, some attributes just produce the same |
|
Just reverted d683225 because this was giving evaluation errors. It is very weird because I though OfBorg would cover everything. It looks like both Haskell & Perl may still be using some broken aliases.
I hope we don't have to do that. All of the hash changes were either related to removing unwanted whitespace or removing duplicate listed inputs. It appears very common for packages to do something weird like Here is the full list of what has changed:
All of the others in https://gist.github.com/GrahamcOfBorg/0d8b33ede1f4e2442d148feaffb43528/raw/77b80187e2c8e4b78cffc367d14dff298234310c/Changed%2520Paths should just be moved aliases. |
Do such things in separate commits, it becomes much, much easier to review. |
|
I think having this as a hard error was a bad idea, it makes it way too easy for the tarball job to be broken accidentally. For example https://nix-cache.s3.amazonaws.com/log/i8221imxr8cimkycd7vglyi62an1qirv-nixpkgs-tarball-18.09pre146984.f99f42c4736.drv and https://git.io/fN4XI. A warning (say, inside the tarball job) would be better. |
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
This will make release.nix error if a package is using one of the aliases in aliases.nix. For this to work initially, it required treewide changes. Some old aliases that are used very extensively were also move back to all-packages.nix
/cc @volth
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)