[WIP] stdenv: inherit `src.meta` to allow moving most of `meta` into `src.meta` by oxij · Pull Request #35075 · NixOS/nixpkgs

oxij · 2018-02-17T15:36:38Z

Motivation for this change

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.

See commit messages for more info.

Things done

Nothing really changes (except meta.evaluates -> meta.available).
Fits CONTRIBUTING.md.

oxij · 2018-02-17T18:06:27Z

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.

oxij · 2018-02-17T20:58:04Z

Strictly speaking its still WIP as many other packages that do throw in src will be broken by check-meta. But I don't want to touch them before we decide what to do with #34755 as it proposes a better syntax than 37dedf64536f366df730584b5fb285b0469d2bea.

oxij · 2018-02-17T21:17:16Z

Btw

Nothing really changes.

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 .drvs doesn't change, their hashes change, but since they are fixed-output it wouldn't rebuild anything.

7c6f434c · 2018-02-17T21:19:20Z

I like most of the changes, but I think the allowBinary should be a separate PR, because there are a lot of things to discuss about it. Technically, bootstrap tools for stdenv (a transitive dependency of everything…) are a binary blob. Then there are Rust, GHC, SBCL, FPC… that we bootstrap from upstream binaries.

oxij · 2018-02-17T21:26:14Z

Agreed. Will do after a couple of other TODO items.

7c6f434c · 2018-02-17T21:31:26Z

I probably like the meta.available name better, but renaming meta.evaluates is an externally visible change.

oxij · 2018-02-17T22:33:27Z

I just introduced it in #33057, so

I don't think anyone outside nixpkgs uses it yet.
There were no stable releases with this feature so I feel like its fine to change it without any grace period. Its master, after all.

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).

7c6f434c · 2018-02-17T22:37:37Z

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 · 2018-02-17T22:41:03Z

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 · 2018-02-17T22:48:59Z

Ok. allowBinary stashed away. The only TODO left here is #35075 (comment). I feel like stealing a patch from #34755 and fixing all the things this breaks using it here. Lets discuss the thing there.

7c6f434c · 2018-02-17T23:31:11Z

I think getAttrByName you proposed there is called builtins.getAttr, but maybe you want better error reporting.

oxij · 2018-02-18T00:49:19Z

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.

zimbatm · 2018-02-18T21:14:03Z

nixos/doc/manual/default.nix

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?

zimbatm · 2018-02-18T21:16:22Z

pkgs/development/mobile/androidenv/platform-tools.nix

how is that error displayed? it might be better to add a throw keyword to make it more explicit.

I don't like this part too. I would prefer for check-meta to give that error. Hence lazy fetchurl patch. But its ugly.

zimbatm · 2018-02-19T15:27:36Z

Let's postpone the discussion around pkgs/development/mobile/androidenv/platform-tools.nix and fetchurl. The rest of the code makes sense and could be merged already. I would also like to have a multi-fetcher but it has to be fleshed out a bit more.

oxij · 2018-02-19T15:34:49Z

You see, I just wanted the first two commits. But manual build then fails because src.meta fails for the unlazy fetchurl which breaks meta of platformTools which is used to generate documentation. So I started fixing that and got where I'm now. Of course, I can just hack around it, but I prefer to waste some time now experimenting, that waste some time later hacking around the hacks.

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 · 2018-02-19T15:43:21Z

Rebased to remove already merged commits. Still WIP and is going to be WIP for a while.

zimbatm · 2018-02-20T14:23:05Z

pkgs/build-support/fetchurl/default.nix

+  #   } // optionalAttrs (stdenv.system == "x86_64-linux") {
+  #     url = ...;
+  #     sha256 = ...;
+  #   })


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

oxij · 2018-02-20T15:35:11Z

```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 ... }) ```

stale · 2020-06-01T23:19:26Z

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

Search for maintainers and people that previously touched the
related code and @ mention them in a comment.
Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
irc.freenode.net.

stale · 2021-04-26T06:55:13Z

I marked this as stale due to inactivity. → More info

oxij requested a review from FRidh as a code owner February 17, 2018 15:36

GrahamcOfBorg added 6.topic: emacs Text editor 6.topic: python Python is a high-level, general-purpose programming language. labels Feb 17, 2018

oxij force-pushed the stdenv/src-meta branch from 8b3d6dc to c15ccb9 Compare February 17, 2018 15:40

oxij changed the title ~~stdenv: inherit src.meta~~ [WIP] stdenv: inherit src.meta Feb 17, 2018

GrahamcOfBorg added 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation labels Feb 17, 2018

oxij changed the title ~~[WIP] stdenv: inherit src.meta~~ stdenv: inherit src.meta Feb 17, 2018

oxij mentioned this pull request Feb 17, 2018

WIP: Fetch sources #34755

Closed

8 tasks

GrahamcOfBorg added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Feb 17, 2018

oxij changed the title ~~stdenv: inherit src.meta~~ stdenv: inherit src.meta to allow moving most of meta into src.meta Feb 17, 2018

7c6f434c changed the title ~~stdenv: inherit src.meta to allow moving most of meta into src.meta~~ [WIP] stdenv: inherit src.meta to allow moving most of meta into src.meta Feb 17, 2018

oxij force-pushed the stdenv/src-meta branch from 37dedf6 to fd2b3cb Compare February 17, 2018 22:41

oxij mentioned this pull request Feb 18, 2018

fetchurl: cleanup, better errors #35110

Merged

2 tasks

zimbatm reviewed Feb 18, 2018

View reviewed changes

oxij added 3 commits February 19, 2018 15:38

tree-wide: remove most uses of src.meta.homepage

bb57e8e

fetchurl: fail lazily

993f5f8

oxij force-pushed the stdenv/src-meta branch from fd2b3cb to 724b3c4 Compare February 19, 2018 15:39

oxij requested review from edolstra and nbp as code owners February 19, 2018 15:39

oxij force-pushed the stdenv/src-meta branch from 724b3c4 to 993f5f8 Compare February 19, 2018 15:41

GrahamcOfBorg removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation labels Feb 19, 2018

zimbatm reviewed Feb 20, 2018

View reviewed changes

oxij mentioned this pull request Mar 3, 2018

[RFC] Replace assert with meta.broken #36229

Closed

matthewbauer added the 2.status: work-in-progress label Mar 22, 2018

FRidh removed their request for review July 20, 2019 11:47

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020

ryantm added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 3, 2020

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 3, 2020

ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020

ryantm marked this pull request as draft October 23, 2020 03:13

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 26, 2021

adamcstephens closed this Sep 8, 2024

zimbatm mentioned this pull request Sep 18, 2024

src.meta.changelog #338366

Closed

13 tasks

Uh oh!

Conversation

oxij commented Feb 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

oxij commented Feb 17, 2018 via email

Uh oh!

oxij commented Feb 17, 2018

Uh oh!

oxij commented Feb 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

7c6f434c commented Feb 17, 2018

Uh oh!

oxij commented Feb 17, 2018

Uh oh!

7c6f434c commented Feb 17, 2018

Uh oh!

oxij commented Feb 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

7c6f434c commented Feb 17, 2018

Uh oh!

7c6f434c commented Feb 17, 2018

Uh oh!

oxij commented Feb 17, 2018

Uh oh!

7c6f434c commented Feb 17, 2018

Uh oh!

oxij commented Feb 18, 2018 via email

Uh oh!

zimbatm Feb 18, 2018

Choose a reason for hiding this comment

Uh oh!

7c6f434c Feb 19, 2018

Choose a reason for hiding this comment

Uh oh!

zimbatm Feb 18, 2018

Choose a reason for hiding this comment

Uh oh!

oxij Feb 19, 2018

Choose a reason for hiding this comment

Uh oh!

zimbatm commented Feb 19, 2018

Uh oh!

oxij commented Feb 19, 2018

Uh oh!

oxij commented Feb 19, 2018

Uh oh!

zimbatm Feb 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oxij commented Feb 20, 2018 via email

Uh oh!

stale bot commented Jun 1, 2020

Uh oh!

stale bot commented Apr 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

oxij commented Feb 17, 2018 •

edited

Loading

oxij commented Feb 17, 2018 •

edited

Loading

oxij commented Feb 17, 2018 •

edited

Loading

zimbatm Feb 20, 2018 •

edited

Loading