[WIP] stdenv: inherit src.meta to allow moving most of meta into src.meta
#35075
Conversation
oxij
force-pushed
the
stdenv/src-meta
branch
from
8b3d6dc
to
c15ccb9
Compare
|
Not really, as its very git specific, and I have a more general feature
in mind (see the commit message of
97494769c61966426ab75890662ab75d213b11f5) and hope nixpkgs will
eventually move all `meta.license`s to `src`.
The build failure I will now investigate.
|
|
Strictly speaking its still WIP as many other packages that do |
|
Btw
is still correct. The manual changes a bit because I added some documentation. Other changes seen by ofborg are not actual changes. The contents of |
|
I like most of the changes, but I think the |
oxij
changed the title
src.metasrc.meta to allow moving most of meta into src.meta
|
Agreed. Will do after a couple of other TODO items. |
|
I probably like the |
|
I just introduced it in #33057, so
Actually, that patch is from another related branch where it makes even more sense, but I thought it wouldn't hurt if it cuts in line here instead (as that other branch will almost surely generate copious amounts bikeshedding). |
|
I agree no-grace-period is OK, I would just list it in the description below ‘‘Nothing really changes’’, as it is a change. |
7c6f434c
changed the title
src.meta to allow moving most of meta into src.metasrc.meta to allow moving most of meta into src.meta
|
Added a WIP tag because you explicitly say that there are TODO items you plan to implement here anyway (you wouldn't be able to drop a label, but you can undo my change to the title). |
oxij
force-pushed
the
stdenv/src-meta
branch
from
37dedf6
to
fd2b3cb
Compare
|
Ok. |
|
I think |
|
Maybe, I'm yet unsure myself. The more I think about it the less I like the "lazy `fetchurl`" patch. Maybe I'll completely change the approach there. Will see in the next couple of days.
Would be nice if somebody comfortable with networking looked at #27688. Wink, wink :)
I would love it to be merged before 18.03.
|
nixos/doc/manual/default.nix
Outdated
| @@ -36,7 +36,7 @@ let | |||
| package = args.package or (lib.attrByPath path (throw "Invalid package attribute path `${toString path}'") pkgs); | |||
| in "<listitem>" | |||
| + "<para><literal>pkgs.${name} (${package.meta.name})</literal>" | |||
| + lib.optionalString (!package.meta.evaluates) " <emphasis>[UNAVAILABLE]</emphasis>" | |||
| + lib.optionalString (!package.meta.available) " <emphasis>[UNAVAILABLE]</emphasis>" | |||
There was a problem hiding this comment.
could the broken attribute be re-used there?
There was a problem hiding this comment.
I think broken has a more narrow meaning (this stopped working where it worked before), and the check also includes platforms and license?
| license = licenses.gpl2; | ||
| platforms = [ "i686-linux" "x86_64-linux" "x86_64-darwin" ]; | ||
| }; | ||
| url = "unsupported on ${stdenv.system}"; |
There was a problem hiding this comment.
how is that error displayed? it might be better to add a throw keyword to make it more explicit.
There was a problem hiding this comment.
I don't like this part too. I would prefer for check-meta to give that error. Hence lazy fetchurl patch. But its ugly.
|
> @@ -36,7 +36,7 @@ let
package = args.package or (lib.attrByPath path (throw "Invalid package attribute path `${toString path}'") pkgs);
in "<listitem>"
+ "<para><literal>pkgs.${name} (${package.meta.name})</literal>"
- + lib.optionalString (!package.meta.evaluates) " <emphasis>[UNAVAILABLE]</emphasis>"
+ + lib.optionalString (!package.meta.available) " <emphasis>[UNAVAILABLE]</emphasis>"
> could the `broken` attribute be re-used there?
I think `broken` has a more narrow meaning (this stopped working where it worked before), and the check also includes `platforms` and `license`?
Right. `broken` is set by expression authors, `evaluates`/`available` is a cheap (because `tryEval` on derivation `.outPath` is not cheap, as it computes all the hashes) approximation of the result of trying to actually evaluate the expression (hence the old name).
With current implementation of check-meta `broken` implies `!available`, unless custom check-meta error handler decides its fine for a select package to be broken.
|
|
Michael Raskin <notifications@github.com> writes:
@oxij re: platform-specific downloads: so, what is your current opinion? `builtins.getAttr` and platform-indexed attrset of source information?
Generally, yes. But now I'm syntactically more inclined to something like
```
src = fetchWith fetchurl {
license = licenses.whatever;
... other meta goes here ...
} {
url = ...;
hash = ...;
}
# and
src = fetchWithSystem fetchurl {
... meta goes here ...
} {
x86_64-linux = {
url = ...
hash = ...
};
i686-linux = x86_64-linux;
... etc ...
}
# or, more modular,
src = fetchWith fetchurl {
... meta goes here ...
} (selectSystem rec {
x86_64-linux = {
url = ...
hash = ...
};
i686-linux = x86_64-linux;
... etc ...
})
# in the last example `selectSystem` then has to generate another `meta` with `platforms`
```
I also want to either merge it or make it play nice with source selection from SLNOS (because its awesome), so I have not finalized the design yet.
Btw, I also want to fix `fetchgit` to produce the same hash as `fetchFromGitHub` most of the time by using `git-archive` (not by default at first, obviously).
|
|
Let's postpone the discussion around |
|
You see, I just wanted the first two commits. But |
Immediate effect: no need to do `meta.homepage = src.meta.homepage` anymore. See the next patch. Less immediate effect: you can now move `meta.license` or any other such thing to the `src` attribute of your derivation and by doing so ban the accidental downloading of the sources of softwares you blacklisted with check-meta. In my opinion, moving most of `meta` into `src` makes sense. For instance, `homepage` is an attribute of a source, not an attribute of a derivation.
oxij
force-pushed
the
stdenv/src-meta
branch
from
fd2b3cb
to
724b3c4
Compare
oxij
force-pushed
the
stdenv/src-meta
branch
from
724b3c4
to
993f5f8
Compare
|
Rebased to remove already merged commits. Still WIP and is going to be WIP for a while. |
| # } // optionalAttrs (stdenv.system == "x86_64-linux") { | ||
| # url = ...; | ||
| # sha256 = ...; | ||
| # }) |
There was a problem hiding this comment.
I don't think this pattern is necessarily a good thing. A similar result could be achieved with fetchSource as proposed in the other PR:
let
meta = { ... };
in
fetchSource {
x86_64-linux = fetchurl { url = ...; sha256 = ...; inherit meta; };
...
} stdenv.system
|
```nix
let
meta = { ... };
in
fetchSource {
x86_64-linux = fetchurl { url = ...; sha256 = ...; inherit meta; };
...
} stdenv.system;
```
Firstly, that `fetchSource` is just `getAttr`.
Secondly, I don't like the idea of separating `meta`s and repeating `fetchurl`s. Too much copy-paste.
Also, I would like to have `meta.platforms` to be autogenerated and make the whole thing sufficiently lazy so that accessing `package.src.outPath` would naturally fail with "unsupported on this platform" but looking up the fields would still work. Your example can't trivially do that but my example from several messages above copy-pasted below can
```nix
src = fetchWith fetchurl {
... meta goes here ...
} (selectSystem rec {
x86_64-linux = {
url = ...
hash = ...
};
i686-linux = x86_64-linux;
... etc ...
})
```
|
|
Thank you for your contributions.
|
stale
bot
added
the
2.status: stale
stale
bot
removed
the
2.status: stale
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
Motivation for this change
You can now move
meta.licenseor any other such thing to thesrcattribute of your derivation and by doing so ban the accidental downloading of the sources of softwares you blacklisted with check-meta.In my opinion, moving most of
metaintosrcmakes sense. For instance,homepageis an attribute of a source, not an attribute of a derivation.See commit messages for more info.
Things done
meta.evaluates->meta.available).