AndersonTorres
commented
4 years ago I can do it too :)
Open nh2 opened 4 years ago
AndersonTorres
commented
4 years ago I can do it too :)
vikanezrimaya
commented
4 years ago Automatically generated by rg "[^p]name = " pkgs/ --files-with-matches | sed -e 's/^/ - [ ] /'
vikanezrimaya
commented
4 years ago This list might contain some errors, since I used a fairly naive regex to make it. I already marked three items for which @nh2 filed a PR.
siraben
commented
4 years ago @kisik21 please try to collapse the comment with
<details>
list goes here
</details>
xaverdh
commented
4 years ago I grepped for package explicitly that explicitly set name = ${pname}-${version} and tried to fix them.
The following question arose:
Is this just about making pname and name discoverable, or also about making them easily overridable?
Its a common pattern for packages to e.g. bind pname + version with a let, then define name in terms of that (also in the let binding), propagate name to mkDerivation, but also use it elsewhere e.g. in the src part.
Even if the package passes a recursive set to mkDerivation (there is no way to access the name if its not bound "statically" in the recursive set, right?), the only way I can see to properly make this overridable is by manually writing down a fixpoint.
So something like
let res = mkDerivation {
pname = [...];
version = [...];
src = [...]${res.name}[...]
} in resWould be nice if recursive sets had a kind of self attribute..
edit: I guess one can just pass name in addition to pname and version to mkDerivation. Is that acceptable?
vikanezrimaya
commented
4 years ago @siraben done, but the markdown is not processed inside of HTML as per Markdown spec, so I wrapped it in <pre /> too, so it'll look a little bit less messy.
siraben
commented
4 years ago I just checked for some packages that I maintain, such as miranda. This is correct, right?
pname = "miranda";
version = "2.066";
nh2
commented
4 years ago @kisik21 the markdown works for me if I simply remove the <pre> tags in your message:
nh2
commented
4 years ago I just checked for some packages that I maintain, such as
miranda. This is correct, right?
@siraben Yes, correct.
nh2
commented
4 years ago Is this just about making
pnameandnamediscoverable, or also about making them easily overridable?
@xaverdh This is just about making the technical change to use pname and stop using name. For more sophisticated changes, a separate issue would be better.
Its a common pattern for packages to e.g. bind pname + version with a
let, then definenamein terms of that
This pattern still counts as the old (bad) pattern, for the new (good) pattern one has to give pname and version attributes directly to mkDerivation. I'll write that into the issue description.
I guess one can just pass
namein addition topnameandversiontomkDerivation. Is that acceptable?
I don't thinks so, or at least I haven't seen that done.
I believe the most common pattern to re-use versions e.g. in src is
mkDerivation rec {
pname = "mypackage";
version = "1.2.3";
src = {
... expression using `version` variable here ...
};
}@nh2 done, the markdown indeed works. Weird, it didn't work the first time!
siraben
commented
4 years ago I tried a search with the following command. Turned up quite a lot of results. This list packages that have a version number in their name.
$ ag "[^p]name.*=.*-[0-9]+.*;" pkgs/ -G default.nix | grep -v patch > report.txt
$ wc -l report.txt
1622 report.txtAny ideas on how to automate this process? Take pngcrush for example,
name = "pngcrush-1.8.13";
src = fetchurl {
url = "mirror://sourceforge/pmt/${name}-nolib.tar.xz";
sha256 = "0l43c59d6v9l0g07z3q3ywhb8xb3vz74llv3mna0izk9bj6aqkiv";
};Seems like we would need some more complex meta-programming to be able to do this change en masse.
edolstra
commented
4 years ago A few comments:
Migrating to pname/version is a bit premature (and not super useful) given that Nix doesn't actually support these attributes.
It creates a large amount of code churn, in particular making cherry-picking to the release branches harder.
vikanezrimaya
commented
4 years ago Maybe we should coordinate this with 21.03 release then?
worldofpeace
commented
4 years ago and not super useful
I can't remember if there's some thing with repology only using pname and version?
nh2
commented
4 years ago
- Migrating to pname/version is a bit premature (and not super useful) given that Nix doesn't actually support these attributes.
@edolstra I don't understand, 13000 packages already use pname + version.
Are we talking about something else?
Or do you mean that we don't gain additional features yet?
AndersonTorres
commented
4 years ago Are we talking about something else?
"Migrating" here has the sense of a full migration. In the sense of parsing etc. the Nix language doesn't care much about it - it's just cosmetical.
edolstra
commented
4 years ago @edolstra I don't understand, 13000 packages already use pname + version.
Yes, and it doesn't actually do anything since Nix doesn't know about those attributes. For instance, nix-env -u doesn't look at the version attribute.
xaverdh
commented
4 years ago Yes, and it doesn't actually do anything since Nix doesn't know about those attributes. For instance,
nix-env -udoesn't look at the version attribute.
This change is not neccessarily just about nix itself. I guess it also simplifies automation and such in nixpkgs itself, because the interface is better. E.g. extracting versions of all packages etc..
siraben
commented
4 years ago I guess it also simplifies automation and such in nixpkgs itself, because the interface is better. E.g. extracting versions of all packages etc..
@xaverdh is it known if this would have a positive impact on automation such as @ryantm's nix-update bot?
jtojnar
commented
4 years ago I can't remember if there's some thing with repology only using
pnameandversion?
Repology just uses pname and version from packages.json, which are not the derivation attributes of the same name – the JSON fields are created by parsing name :crying_cat_face: That is what is meant by Nix not knowing about pname and version attributes.
siraben
commented
4 years ago What happened to https://github.com/NixOS/nix/blob/master/src/libexpr/names.cc ? Where is the parser now?
jtojnar
commented
4 years ago https://github.com/NixOS/nix/blob/HEAD@%7B2019-09-11T15:42:25Z%7D/src/libexpr/names.cc#L19 → https://github.com/NixOS/nix/blob/ae3191666f1828d9694fb615a721d409fb8582bd/src/libstore/names.cc#L19
Refined GitHub extension allows you to view comment link targets at the time they were linked.
FRidh
commented
4 years ago As mentioned Nix does not utilize this information. Thus, changing this only makes sense if there are any other good reasons for it.
Support for pname and version was originally merged in efca8b4b97cab7653cc369973fb6267d160c494a without any consensus on the topic (https://github.com/NixOS/rfcs/pull/35). The separation between pname and version already existed in the Python builder, however, as I commented before
I'm of the opinion version should not actually be an attribute of the derivation but instead of the source.
AndersonTorres
commented
4 years ago The "good reasons" are uniformity. The old name=<pkgname>-${version}; version = <release version>; is redundant and confusing sometimes, while "pname & version" is cleaner.
nh2
commented
4 years ago Linking some treewide PRs done for this in the past for people interested in automation (CC @volth):
AndersonTorres
commented
4 years ago 
nixos-discourse
commented
3 years ago This issue has been mentioned on NixOS Discourse. There might be relevant details there:
ShamrockLee
commented
3 years ago Not sure if it's related, but appimageTools.extractType2 and appimageTools.wrapType2 still accept only name instead of pname and version.
AndersonTorres
commented
3 years ago Not sure if it's related, but
appimageTools.extractType2andappimageTools.wrapType2still accept onlynameinstead ofpnameandversion.
Sometimes many appimages has no info about version. It is a bit hardcore to update.
Also name is a bit more, ehr, hardcoded in appimageTools.
ShamrockLee
commented
3 years ago Not sure if it's related, but
appimageTools.extractType2andappimageTools.wrapType2still accept onlynameinstead ofpnameandversion.Sometimes many appimages has no info about version. It is a bit hardcore to update.
Also
nameis a bit more, ehr, hardcoded in appimageTools.
Got it. Thank you.
samuela
commented
3 years ago I tried to make a dent in this with https://github.com/NixOS/nixpkgs/pull/123438. Is there some point where we can flag name in mkDerivations with a deprecation warning or error?
Artturin
commented
3 years ago after #128843 there's no more meta.versions in nixpkgs;
davidak
commented
3 years ago The last open PR was merged.
Now we can wait until it is merged from staging to master (status) and then check if anything is left.
Then Nix can get changed to actually use this values, e.g. for the packages.json which is used by Repology.
Thanks to everyone involved!
Stunkymonkey
commented
2 years ago @nh2
Somebody confirm that meta.tag should indeed be removed as well
I am unable to find a package that uses meta.tag.
I asked @7c6f434c about advantages of using the update-walker.sh. Maybe someone could tell how to proceed here: https://github.com/7c6f434c/nixpkgs-email/issues/1
What about files called default.upstream? are they not needed if meta.updateWalker is not set?
same about meta.downloadRegExpr?
7c6f434c
commented
2 years ago The other way round, if updateWalker is set then parsing default.nix works even without default.upstream.
But yeah, I stopped actively monitoring for updates to packages that work well anyway, so whatever, you can drop the (pre-updateScript) updaters
wackbyte
commented
2 years ago oops, sorry about the spam
erdnaxe
commented
2 years ago Some Ruby packages seem to break when splitting "name" to "pname"+"version": https://github.com/NixOS/nixpkgs/issues/70171#issuecomment-559007480 The associated issue is closed, but I am worried the underlying issue is still not fixed.
erdnaxe
commented
2 years ago Is there a specific reason for not using pname+version in font packages?
pkgs/data/fonts/cooper-hewitt/default.nix:4: name = "cooper-hewitt-2014-06-09";
pkgs/data/fonts/nanum-gothic-coding/default.nix:5: fullName = "NanumGothicCoding-2.5";
pkgs/data/fonts/powerline-fonts/default.nix:4: name = "powerline-fonts-2018-11-11";
pkgs/data/fonts/ipaexfont/default.nix:4: name = "ipaexfont-004.01";
pkgs/data/fonts/proggyfonts/default.nix:4: name = "proggyfonts-0.1";
pkgs/data/fonts/wqy-microhei/default.nix:4: name = "wqy-microhei-0.2.0-beta";
pkgs/data/fonts/vdrsymbols/default.nix:4: name = "vdrsymbols-20100612";
pkgs/data/fonts/libre-bodoni/default.nix:4: name = "libre-bodoni-2.000";
pkgs/data/fonts/oxygenfonts/default.nix:4: name = "oxygenfonts-20160824";
pkgs/data/fonts/lato/default.nix:4: name = "lato-2.0";
pkgs/data/fonts/baekmuk-ttf/default.nix:4: name = "baekmuk-ttf-2.2";
pkgs/data/fonts/myrica/default.nix:4: name = "myrica-2.011.20160403";
pkgs/data/fonts/libre-franklin/default.nix:4: name = "libre-franklin-1.014";
pkgs/data/fonts/tai-languages/default.nix:5: name = "tai-ahom-2015-07-06";
pkgs/data/fonts/freefont-ttf/default.nix:4: name = "freefont-ttf-20120503";
pkgs/data/fonts/aurulent-sans/default.nix:4: name = "aurulent-sans-0.1";
pkgs/data/fonts/sampradaya/default.nix:4: name = "sampradaya-2015-05-26";
pkgs/data/fonts/encode-sans/default.nix:4: name = "encode-sans-1.002";
pkgs/data/fonts/mro-unicode/default.nix:4: name = "mro-unicode-2013-05-25";
pkgs/data/fonts/dosis/default.nix:4: name = "dosis-1.007";
pkgs/data/fonts/undefined-medium/default.nix:4: name = "undefined-medium-1.0";
pkgs/data/fonts/libre-baskerville/default.nix:4: name = "libre-baskerville-1.000";
pkgs/data/fonts/libertine/default.nix:4: name = "linux-libertine-5.3.0";
pkgs/data/fonts/lmodern/default.nix:4: name = "lmodern-2.005";
pkgs/data/fonts/ubuntu-font-family/default.nix:4: name = "ubuntu-font-family-0.83";
pkgs/data/fonts/kawkab-mono/default.nix:4: name = "kawkab-mono-20151015";
pkgs/data/fonts/mph-2b-damase/default.nix:4: name = "MPH-2B-Damase-2";
pkgs/data/fonts/yanone-kaffeesatz/default.nix:4: name = "yanone-kaffeesatz-2004";
pkgs/data/fonts/cabin/default.nix:4: name = "cabin-1.005";
pkgs/data/fonts/ipafont/default.nix:4: name = "ipafont-003.03";Should I open pull requests to split the "name" of all these packages?
AndersonTorres
commented
2 years ago Many of those packages are generated without using mkDerivation, since they can be installed by merely unpacking the source tarball and moving the files to its desired locations.
Here is a typical example:
fetchzip {
name = "cooper-hewitt-2014-06-09";
url = "https://www.cooperhewitt.org/wp-content/uploads/fonts/CooperHewitt-OTF-public.zip";
postFetch = ''
mkdir -p $out/share/fonts/opentype
unzip -j $downloadedFile \*.otf -d $out/share/fonts/opentype/
'';
####
}I don't believe this would be useful, after all pname,version are not recognized by the fetchers (yet).
Maybe a
let
pname = ...
version = ...
name = pname + "-" + version
in
....
inherit name;would be useful?
Artturin
commented
2 years ago fetch* are just fetchers not package creators so the things that use them for that should be converted to use mkDerivation
SuperSandro2000
commented
2 years ago fetch* are just fetchers not package creators so the things that use them for that should be converted to use mkDerivation
Fonts are mostly just files which get downloaded and copied to the nix store. We don't need to layer an extra derivation which copies $src into a directory into $out.
06kellyjac
commented
2 years ago As a note all these PRs are making the fonts dependent on the compilers since it's using stdenv rather than stdenvNoCC
This won't lead to constant rebuilds because of the fixed output but it's still a concern as it bloats the build c̶l̶o̶j̶u̶r̶e̶ closure.
A true solution IMO would be having fetchers support pname & version or just leaving them alone, rather than using stdenvNoCC.mkDerivation
winterqt
commented
2 years ago fetch* are just fetchers not package creators so the things that use them for that should be converted to use mkDerivation
If this is the case, shouldn't we remove being able to pass meta to fetchers? Since that functionality doesn't align with that statement.
06kellyjac
commented
2 years ago No because I disagree with that statement.
If you're just fetching data and maybe moving or deleting some files there is no benefit IMO for using in mkDerivation.
winterqt
commented
2 years ago I agree. What I meant is that if that statement was the general consensus, then that would have to happen, which it hasn't. Which leaves us at a crossroads.
It seems that the move to mkDerivation for fonts happened without much discussion, so let's have that discussion now, reach a consensus, and make the required changes (potentially reverting the mkDerivation migrations).
AndersonTorres
commented
2 years ago Is there any advantage in using a pure fetchurl solution instead of stdenvNoCC.mkDerivation? Is it faster somehow?
For me mkDerivation reduces the mental load.
SuperSandro2000
commented
2 years ago A true solution IMO would be having fetchers support pname & version or just leaving them alone, rather than using
stdenvNoCC.mkDerivation
https://github.com/NixOS/nixpkgs/pull/194425
Is there any advantage in using a pure
fetchurlsolution instead ofstdenvNoCC.mkDerivation? Is it faster somehow?
We are only using one fixed output derivation instead of two where the second just copies files around. Every derivation has a small overhead which we can save here. Also it makes it easier to rebuild FODs to see if the extra script they are using still works.
winterqt
commented
2 years ago We are only using one fixed output derivation instead of two where the second just copies files around.
Don't you mean one FOD and one non-FOD, in the case of using mkDerivation?
It seems like there isn't a tracking issue about this long-term effort yet (e.g. to mention in commit messages and to make it sprintable).
Nixpkgs moved to an explicit versioning scheme, with the plan to change all packages like so:
See the sprint checkbox list for the remaining packages: https://github.com/NixOS/nixpkgs/issues/103997#issuecomment-728752185
This obviates
meta.versionandmeta.tag; they should be removed (as discussed in #12156).Tangentially related: Where to put
unstableversions / package names (see #68518).Details on what's right and what isn't
Note that the following pattern is not good because it still uses
name:In the new approach,
pnameandversionhave to be given directly tomkDerivation.You can use
mkDerivation recto be able to re-use e.g.versioninsrcor other fields:Automation
You can link big treewide efforts/PRs here (tick merged ones):
Tasks
meta.tagshould indeed be removed as wellupdate-walker.shscript? Ask @7c6f434c: #151363updateWalkerbecause it's the last reference toupdate-walker.sh: #151363