New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add nix-prefetch-source #21734

Merged
merged 1 commit into from Jan 23, 2017

Conversation

@timbertson
Contributor

timbertson commented Jan 7, 2017

Motivation:

I've always found updating sources to be somewhat awkward. I don't know how others do it, but for me it's usually a matter of running nix-prefetch-$whatever, then copy-pasting the relevant fields into my nix expression.

But tediously, I need to keep the source information updated in multiple locations:

  • the upstream project (so developers can nix-build or nix-shell from a checkout)
  • any other repos which depend on specific versions of a dependency
  • my own collection of development dependencies (a collection of nixexprs so I can access my unpublished or in-development tools wherever I go)
  • nixpkgs itself

In fact, this friction has prevented me from bothering adding packages to nixpkgs in the past, because the other requirements are already inconvenient enough. Which is sad, and I want to fix that.

Goals:

  • Make it painless (and automatable) to update a nix expression to build the latest upstream source code. I was particularly inspired by Rok's recent blog post on automating updates, and by discovering the existing support in maintainers/scripts/update.nix for scripted updates.

  • Make it less tedious to keep upstream .nix expressions in sync with the nixpkgs repository

Approach:

I've created a tool called nix-prefetch-source. It's a wrapper around the other nix-prefetch-* scripts, with the aim of making source updates as automatable as possible.

So for example currently you might:

  • run nix-prefetch-git <repo> HEAD
  • copy and paste bith rev and sha265 into your nix expression

It's not hard, but it's not automatable. With nix-prefetch-source, you store what you want to fetch as JSON:

{
  "url": "https://example.org/foo.git",
  "rev": "HEAD",
  "type": "fetchgit"
}

(you can pass them directly as command line arguments if you don't want to store this as JSON)

Then you run nix-prefetch-source -i src.in.json -o src.json, and out pops the latest source for those inputs:

{
  "url": "https://example.org/foo.git",
  "rev": "HEAD",
  "type": "fetchgit",
  "fetchArgs": {
    "fetchSubmodules": true,
    "rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
    "sha256": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
    "url": "https://example.org/foo.git"
  }
}

This can be re-run at any time to get the latest sources. It contains whatever inputs were given, plus a set of attributes in fetchArgs suitable for passing directly to fetchgit.

And indeed my PR adds a convenience function, importSource which takes a path to this kind of file, and returns an object with all the same properties plus src, which is really just (getAttribute input.type) input.fetchArgs.

It's generic enough that it ought to be able to support every fetcher, by simply adding a handler to nix-prefetch-source, which knows how to perform the prefetch and then generate the arguments to give to the fetching function.

Here's how I've used to package a simple python package that I happened to be packaging today:

# nix/default.nix
{ pythonPackages, importSource, gnome3 }:
let source = (importSource ./src.json); in
pythonPackages.buildPythonPackage {
  inherit (source) src;
  name = "dconf-user-overrides-${source.version}";
  propagatedBuildInputs = with pythonPackages; [ pygobject3 ];
  postInstall = ''
    wrapProgram $out/bin/dconf-user-overrides \
      --suffix GIO_EXTRA_MODULES : ${gnome3.dconf}/lib/gio/modules \
      ;
  '';
}

# default.nix:
with import <nixpkgs> {} ; callPackage ./nix/default.nix {}

This separation conveniently means that nix/default.nix can be copied wholesale into nixpkgs, as long as src.json comes along with it. You can even use a different src.json while keeping the same nix expression, if for example you want to reference tagged releases in nixpkgs using fetchFromGitHub but need to reference a specific git commit via fetchgit in development.

Next steps:

Any thoughts? objections? I believe the PR should be mergeable and useable as-is, although there's opportunity for a bit more integration if we want to include nix-prefetch-source in nix-prefetch-scripts. I haven't done this yet because I wanted to validate the approach first, and I'd need to invert a dependency (currently it depends on nix-prefetch-scripts)

@Mic92

This comment has been minimized.

Contributor

Mic92 commented Jan 7, 2017

Related to a pull request of @zimbatm #19540

@domenkozar domenkozar referenced this pull request Jan 7, 2017

Closed

nix-prefetch-github? #21732

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 7, 2017

Thanks for the pointer, I hadn't noticed that discussion. @zimbatm and @garbas, I seem to be aiming at a similar thing to you, is there overlap? Have I missed an important use case? Have I just reimplemented your various wheels? ;)

@FRidh

This comment has been minimized.

Member

FRidh commented Jan 7, 2017

I'm very much 👍 for a src.json for derivations. I do think the JSON should be slightly different. The arguments you have to pass to your prefetcher are basically the same as the fetcher uses. Therefore, I think its better to have type corresponding to the function and args for the arguments the fetcher needs, instead of mixing arguments with the fetcher.

{
  "type" : "fetchurl",
  "args" : {
      "url" : "http://...",
      "sha256" : "fsf45"
  }
}

A logical improvement might then be making -i and -o optional, so that when only one file is passed it would overwrite the file but then with the additional arguments like the hash. Or it would automatically work on src.json.

@expipiplus1

This comment has been minimized.

Contributor

expipiplus1 commented Jan 7, 2017

Also of interest might be the update-nix-fetchgit tool https://github.com/expipiplus1/update-nix-fetchgit

@garbas garbas self-requested a review Jan 7, 2017

@garbas

i'm 👎 for this PR since for 2 reasons

  1. there is very little value if there is a script that just calculates hash of the changed revision. you still need to figure out manually which is the new revision.

  2. none of the update scripts are that generic as described here. example: when fetchFromGitHub is used you also need to know which update policy you would follow. should you follow the master branch, some other branch, etc...


~20 days we added an updateScript to the nixpkgs (56cb5b7) which lets you run updateScript for any expression that has updateScript defined. and i understand why you don't know about it, since it isn't yet used in many places (only 7 package use it).

here is what i would propose.

let
  src = fromGitHub { # <- this is the helper method that needs to be written to make this magic happen.
    owner = "NixOS";
    repo = "nixpkgs";
    branch = "master";  # <- this the branch we follow for updates
    path = "src.json";  # <- this is the file which we write/read source information to
  };
in stdenv.mkDerivation {
  name = "blabla";
  inherit src;
  passthru.updateScript = src.updateScript; # <- we can make this line obsolete but i though it would be nice show how everything works together with current updateScript solution
}
@garbas

This comment has been minimized.

Contributor

garbas commented Jan 7, 2017

ofcourse everything could be reduced to (if we would make use some default values)

stdenv.mkDerivation {
  name = "blabla";
  src = fetchFromGitHubWithUpdate {
    owner = "NixOS";
    repo = "nixpkgs";
    branch = "master";
 };
}

in above example we would change maintainers/scripts/update.nix to also check for existence of src.updateScript and the passthru.updateScript = ... line is not needed anymore.

@garbas

This comment has been minimized.

Contributor

garbas commented Jan 7, 2017

as a proof of concept I tried to implement it here: 45dcfb3

to run update for pdf2odt from above commit you need to run: nix-shell maintainers/scripts/update.nix --argstr package pdf2odt

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 8, 2017

@FRidh:

Fair point, the tool probably could now edit a file in-place. Actually the code should already work, we might just want to make it the default mode of operation.

As for where the arguments live, you're right that they're basically the same, but I don't want to force them to be the same (as your proposal would do).

In fact one of the first things I tried involved extra arguments which mean nothing to the fetcher. e.g.:

{
  "type": "fetchFromGitHub",
  "rev": "version-{version}",
  "version": "1.2.3",
  (...)
}

Here the fetcher doesn't know or care about version, because it's used to generate rev. But I still want it to appear in the results so that I can inherit (importSource ./src.json) src version;.

Ultimately the format of src.json is just an implementation detail, since the logic for generation and consumption is all in one place. So I don't mind what it looks like, as long as it works ;)


@garbas

you still need to figure out manually which is the new revision

Well, that depends. If you do have an automated way of knowing what the latest release is, you could always generate src.in.json (or just pass along that information as a command line argument). Or if it's general enough, add it to the relevant nix-prefetch-source handler.

none of the update scripts are that generic as described here. example: when fetchFromGitHub is used you also need to know which update policy you would follow. should you follow the master branch, some other branch, etc...

For this example you would specify which branch to track using the rev argument. And the inputs given to the git fetcher in nix-prefetch-src are not locked down - if there's need for it, we can add more attributes which drive the decision of what to update.

I did know about the update.nix script, it's actually part of what motivated me to write this utility :)

As far as I can see, you'd more or less be implementing the same logic whether you do it in fetchgit.updateScript or in nix-prefetch-source's fetchgit handler. You could rewrite one to the other easily enough, e.g.:

If we put the logic in nix-update-source, fetchgit.updateScript can execute nix-update-source with its attrs converted to JSON (or as command line args). And with fetchgit, nix-update-script could still exist but would have to pass all its paramaters to the individual fetcher update scripts in via environment variables or something.

Personally, I think it's cleaner to use JSON or command line arguments over env vars, and I think that implementing this in each fetcher would lead to inconsistent behaviour and ad-hoc argument parsing (as we already have with the various prefetch scripts), but I really don't mind where this functionality lives.


I didn't mention it earlier because my initial description was already long enough, but one more thing I'd like to have is a minimal way of specifying some arbitrary source code from the internet. One neat thing that putting everything in src.json gives us is that I can minimally reference a specific version of some software with an embedded nix expression in a very generic way - i.e.:

buildUpstream = srcJson: callPackage (importSource srcJson) {};

Mostly I use this for development versions, and don't expect it to appear in nixpkgs proper. But it's still a very useful thing to enable.

Keeping the logic in fetcher-specific nix expressions wouldn't easily allow this, as the information about which fetcher to use wouldn't be part of the JSON. You'd have to write:

buildUpstreamGit = srcJson: callPackage (fetchgit (importJSON srcJson)) {};
buildUpstreamUrl = srcJson: callPackage (fetchurl (importJSON srcJson)) {};
(...)

Basically I want to make sure that I can blindly import a piece of code from somewhere without knowing anything about it, using only the machine-generated src.json. If your approach can do that cleanly, then again - I don't really mind which way it's implemented :)

@garbas

This comment has been minimized.

Contributor

garbas commented Jan 8, 2017

@timbertson i have updated an example from before a little bit (45dcfb3) if you ignore the details of fetchFromGitHubWithUpdate function (which needs some more work to be reusable) then the this would be the change needed in pkgs/tools/typesetting/pdf2odt/default.nix.

from

stdenv.mkDerivation rec {
  version = "2014-12-17";
  name = "pdf2odt-${version}";
  src = fetchFromGitHub {
    owner = "gutschke";
    repo = "pdf2odt";
    rev = "master";
    sha256 = "14f9r5f0g6jzanl54jv86ls0frvspka1p9c8dy3fnriqpm584j0r";
  };
  ...
}

to

stdenv.mkDerivation rec {
  version = "2014-12-17";
  name = "pdf2odt-${version}";
  src = fetchFromGitHubWithUpdate {
    owner = "gutschke";
    repo = "pdf2odt";
    branch = "master";
  };
  ...
}

Maybe we could actually make changes directly to fetchFromGithub, to automatically gain update capabilities when branch and path argument is defined.

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 8, 2017

Yep, I definitely like getting this for free in fetchFromGitHub 👍 . The trick is having path be a string relative to the nix expression.

I still think it should be accessible from an external tool you can run as well though, since I will want to use this on packages which aren't in nixpkgs (i.e development versions). As far as I can tell the update mechanism can only work for official packages.

@Profpatsch

This comment has been minimized.

Member

Profpatsch commented Jan 9, 2017

I’m very much against ad-hoc editing of files in place. That just screams unmaintainable.

I have created a simple POC a few days ago that explores the design space for a general update API. It does not yet contain the code for making fetchers updateable. Also I haven’t had the time to compare it to @garbas update.nix.

It also depends on the nixpkgs testing structure I am very close to creating a PR for, since one definitely doesn’t want automatic updates without being able to test if something breaks.

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 9, 2017

one definitely doesn’t want automatic updates without being able to test if something breaks.

Agreed, although you need to update an expression before you can test it - I think they're independent concerns.

@dezgeg

This comment has been minimized.

Contributor

dezgeg commented Jan 9, 2017

FWIW, I worked on a similar thing in #21766.

@edolstra

This comment has been minimized.

Member

edolstra commented Jan 9, 2017

I'm very much against src.json or src.nix or whatever. This would add literally thousands of files to the Nixpkgs repository. Nix expressions are ideally relatively short and self-contained. I don't want to have to read/edit multiple files when updating a package.

@garbas

This comment has been minimized.

Contributor

garbas commented Jan 9, 2017

@edolstra the purpose is not to update src.json/src.nix manually but to have the update process scripted. instead of updating sha/version manually you would run nix-shell maintainers/scripts/update.nix --argstr package pdf2odt

i personally prefer this to be in a separate file, but on the end - as it is already the case - it is up to the maintainer and the package you are talking. eg. this makes little sense for simple packages, but makes a lot of sense for thunderbird-bin/firefox-bin.

the most important thing is that there is one way how to run the update scripts and that we go away from manually updating versions.

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 9, 2017

@edolstra as Rok said, the intent is to remove the need for manual editing of source infromation at all (in practice, this means copy/pasting from a nix-prefetch-*tool). And the separation of source code from the rest of the derivation is useful enough that there are more than 200 src.nix files in nixpkgs currently.

There's certainly nothing forcing maintainers to adopt this process, but if you want to be able to automate the updating of sources, I don't see any way to allow that other than:

  • keeping that information in a separate machine-writeable file, or
  • have a script edit the .nix file in place to just replace the src attribute and leave all other modifications. This is surely possible, but much more risky (and hacky)
@Profpatsch

This comment has been minimized.

Member

Profpatsch commented Jan 10, 2017

have a script edit the .nix file in place to just replace the src attribute and leave all other modifications. This is surely possible, but much more risky (and hacky)

Except if @edolstra gifts us a full-blown semantic introspection framework. :P

@expipiplus1

This comment has been minimized.

Contributor

expipiplus1 commented Jan 10, 2017

@timbertson,

Editing the .nix file in place is exactly what update-nix-fetchgit does. The hnix package returns a source tree annotated with the location of every expression and construct. It's pretty simple to go from that and generate a list of span replacements to apply.

This tool could greatly benefit from having branch information available.

There's also not much stopping it from being used for non-git sources.

CC @DavidEGrayson

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 10, 2017

@expipiplus1 that's indeed quite impressive. I worry that having this system be implemented in haskell might limit the ability of people (myself included) to contribute though - I've written some haskell and liked in the past, but it's not necessarily easy. Don't get me wrong, if this tool could do everything I want[1] then I'd be all for it, but I think it would take quite a lot of work to get to that point, and I don't know who'd be willing to do that work.

And I feel like there's quite a lot to be said for using a simple, ubiquitous file format for machine-editable data rather than taking on the complexity of editing .nix files in-place. I fact, when I added nix syntax output to nix-prefetch-git there was significant backlash over using nix syntax instead of something much simpler like JSON (which in retrospect I agree with, and was very pleased to see the importJSON functionality land as a consequence).

[1]: off the top of my head: fetchfromGithub, fetchurl, and allowing me to direct the update algorithm either via a config file or arguments

@Profpatsch

This comment has been minimized.

Member

Profpatsch commented Jan 10, 2017

might limit the ability of people (myself included) to contribute though

nixpkgs is mostly written in perl, for Stallman’s sake

import = path:
let
json = lib.importJSON path;
fetchFunction = builtins.getAttr json.type pkgs;

This comment has been minimized.

@zimbatm

zimbatm Jan 10, 2017

Member

Do you think it's safe to use any function from the pkgs as a fetcher? In my version I was thinking of classifying them into their own attrset.

This comment has been minimized.

@timbertson

timbertson Jan 10, 2017

Contributor

You mean just a subset of allowed functions as fetchers? That should be fine. I didn't expect to need to protect against malicious input, but it probably can't hurt to limit the options to only what we need.

This comment has been minimized.

@zimbatm

zimbatm Jan 10, 2017

Member

Yeah I can see how the generated file will probably come from an untrusted source in the future. Anyways, it's an improvement and shouldn't be a blocker for this PR.

@DavidEGrayson

This comment has been minimized.

Contributor

DavidEGrayson commented Jan 10, 2017

@timbertson mentions the "complexity of editing .nix files in place". As someone who has made a tool to edit Nix files in place (update-nix-fetchgit, along with @expipiplus1), I'd say it is actually not that complex. The only thing you need to do when updating to the latest source version is to change certain strings in the file; you don't need to reorganize/refactor/reindent the Nix expression. So you just need a parser that can parse Nix files and retain source location information (like hnix), then you need a thing that scans the parse tree for known patterns to find places where source code is being fetched (Haskell is great at pattern matching), and then you just need to replace the strings you found with their updated versions.

The advantage of updating things in place is that you don't need to have JSON files or restructure all the Nix expressions; you can mostly just work with the expressions we already have, which are more readable. I agree with @edolstra's comment, though that was mostly about editing. I think that it's nice to have fewer files to look at when you are trying to understand a Nix expression, regardless of whether you are editing it or not. It's nice to just have one file that has both the version number of the program you are compiling and the instructions for compiling it. Like most code, Nix expressions are probably read more than they are written, no? So splitting up every Nix package into two separate pieces just to make life easier for developers of automated updater tools at the expense of readability doesn't seem like a good tradeoff.

Regarding this PR: I have not looked too carefully at it, but there are already a bunch of tools in nixpkgs for updating or prefretching source code and they don't have much cohesion, so adding another one to the mix does not seem too bad. Just putting it in the repository does not force people to use it, or tell people that it's the recommended tool.

In summary, I am in favor of tools that provide in-place editing of Nix expressions and are written in high-level languages. I don't think such a tool would need a huge development effort, so a difficult language like Haskell is OK. And update-nix-fetchgit is a good example.

@copumpkin

This comment has been minimized.

Member

copumpkin commented Jan 10, 2017

My concern with updating nix files programmatically is that the space of possible nix files is far larger than anything we can reason about statically.

fetchgit {
  url = "blah";
  sha256 = "omg";
}

is pretty easy to track and update programmatically, but what about this?

let
  apply = x: y: x y; # or `x: x` :)
in apply fetchgit { url = "blah"; sha256 = "zomg"; }

Should it update that? Take for example what I used to have in goPackages (now defunct, but I still maintain that it was a good idea), which factored out a ton of repetition from a bajillion fetchFromGitHub calls by programmatically computing the arguments to them due to commonalities with how Go packages work.

My fear with something like update-nix-fetchgit is that it effectively disincentivizes abstraction/DRY, and makes it so that you either get auto-updates or avoid repetition. And who's going to opt out of auto-updates? So instead we end up with a bunch of first-order calls to fetchgit all over the place.

Edit: to be clear, this is a more general concern than just update-nix-fetchgit or even Nix. In general, machine manipulation of code feels like a way to circumvent proper data modeling exercises that would clarify the underlying commonalities.

Edit 2: some previous thoughts on related topics: #19582 (comment), #19582 (comment), #19582 (comment)

Edit 3: NixOS/nix#520 almost feels like the real crux of the matter, but that's even more speculative

@expipiplus1

This comment has been minimized.

Contributor

expipiplus1 commented Jan 10, 2017

@copumpkin

There's certainly a balance to be found here.

Perhaps the tool could have a "lenient" mode where it updates every attribute set with url and sha256 in (perhaps with some guidance from the user as to what tool is expected to consume that attribute set).

Alternatively the updater could be taught about particular idioms being used, such as the one in goPackages. The easy way to do this would be to add code to match and update these idioms in the updating program.

The argument could certainly be made that if one has automatic updating of these sections in the source, does it matter as much that they aren't as unrepetitive as possible? I suspect that there are cases where the repetition is worth getting automatic updates and other times when it reduces the clarity of the source.

@Profpatsch

This comment has been minimized.

Member

Profpatsch commented Jan 11, 2017

In general, machine manipulation of code feels like a way to circumvent proper data modeling exercises that would clarify the underlying commonalities.

Holy crap you speak from my heart. What good is a code generation tool if the programmer has to constrain hirself to make it work?
My rule of thumb at the moment is to never mix handwritten and autogenerated content, until somebody shows me a better strategy. (Does not only apply to nix but coding in general.)

Perhaps the tool

There won’t be a single tool (because that’s stupid). There will be a bunch of scripts written as nix functions that should allow for composition.

@Profpatsch

This comment has been minimized.

Member

Profpatsch commented Jan 17, 2017

that's because I wasn't using _propagated_buildinputs

Yeah, I know that feeling

is just a side effect

Well, side effects should never be “just” sideeffects.

“It prefetches stuff. Oh, by the way, it also overwrites code in the repository.”

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 18, 2017

“It prefetches stuff. Oh, by the way, it also overwrites code in the repository.”

Well, it writes to the file you told it to (via --output), which isn't usually a feature that warrants specific mention in the name of a tool. Having -and- in the title of a package seems pretty awkward to me. Does anyone else have strong opinions on the name? I'd be OK renaming it to nix-update-source, but I'd rather not rename it more than once if I can help it ;)

Do you'd agree that nix-update-source is specific enough, @Profpatsch? Perhaps prefetching doesn't imply updating, but I'd say that updating a source implies prefetching.

@Profpatsch

This comment has been minimized.

Member

Profpatsch commented Jan 20, 2017

Sounds good to me.

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 21, 2017

OK, https://github.com/timbertson/nix-update-source it is! I've pushed new code to this branch to reflect the rename.

python3Packages.buildPythonApplication rec {
version = "0.2.1";
name = "nix-update-source-${version}";
src = fetchurl {

This comment has been minimized.

@Profpatsch

Profpatsch Jan 21, 2017

Member

fetchFromGitHub

This comment has been minimized.

@timbertson

timbertson Jan 22, 2017

Contributor

Good point - done.

@Profpatsch

This comment has been minimized.

Member

Profpatsch commented Jan 22, 2017

@garbas I think we can merge it in this stage.

@zimbatm zimbatm merged commit ca38ef7 into NixOS:master Jan 23, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@timbertson timbertson referenced this pull request Jan 25, 2017

Merged

gup: 0.5.5 -> 0.6.0 and extract src into JSON #21516

3 of 7 tasks complete
@edolstra

This comment has been minimized.

Member

edolstra commented Jan 30, 2017

Sorry, but I've reverted this for two reasons:

  • It relies on importJSON, which I think is unnecessary. JSON doesn't add anything over Nix expressions and requires users to constantly switch between two syntaxes when editing a Nix package.
  • It enshrines a style of having external src.json (or src.nix) files, rather than having the version info in the main package expression. IMHO having one file is more readable (and doesn't pollute the repo with lots of tiny files). I realize that this is subjective, but it is the standard structure for Nix packages which shouldn't be changed without good reason. (And "a separate src.json is easier to generate" is not a great reason, because it prioritizes the convenience of tool builders over the convenience of human readers.)

edolstra added a commit that referenced this pull request Jan 30, 2017

Revert "add nix-prefetch-source (#21734)"
This reverts commit ca38ef7 due its
use of importJSON and external source info files, which is
non-idiomatic.
@copumpkin

This comment has been minimized.

Member

copumpkin commented Jan 30, 2017

@garbas

This comment has been minimized.

Contributor

garbas commented Jan 30, 2017

@edolstra Hi, I think this commit became a tool which some want to use and not imposing any changes that would include src.json. A tool can easily be used outside nixpks as well. I would vote for reverting your revert.

As for the separate src.json file thing. I do believe this is up for the maintainers how they want to keep packages they maintain up-to-date. I find your arguments weak (1)src.json files not meant to be used by humans but use a separate update process. For packages, I maintain I trust more the computer to do the right job than me. (2) polluting the repository? I'm not sure how more files will hurt us, but I might be missing something. I rather have more files, up-to-date packages and achieve consensus through examples.

Will my contributions be rejected if I continue like this? Just wondering if I should spend my efforts in nixpkgs or create an overlay where I keep things I care updated (automatically). I don't want to sound too negative, but I have a strong preference for src.json. Reason: I don't really want to write a Nix parser, using JSON I can avoid this. I firmly believe that what is stopping us from moving forward is that we are still manually updating packages which consume a time of 2 (if not more) nixpkgs contributors. I don't think this is a way forward.

Please reconsider your stand on this I allow some maintainers to choose their way of updating packages.

@zimbatm

This comment has been minimized.

Member

zimbatm commented Jan 30, 2017

@edolstra sorry for piling up but I'm quite shocked to see this PR reverted based on, as you say it, subjective reasons. Please consider the effect it might have on the contributors when doing something like this.

@jgeerds

This comment has been minimized.

Member

jgeerds commented Jan 30, 2017

Is there no way to automatically update src inside a nix expression? I think this is the main issue (and src.json is more like a workaround)

@Profpatsch

This comment has been minimized.

Member

Profpatsch commented Jan 30, 2017

I firmly believe that what is stopping us from moving forward is that we are still manually updating packages which consume a time of 2 (if not more) nixpkgs contributors. I don't think this is a way forward.

+1 on this

@dezgeg

This comment has been minimized.

Contributor

dezgeg commented Jan 30, 2017

Is there no way to automatically update src inside a nix expression?

There is work for that in #21766.

@globin

This comment has been minimized.

Member

globin commented Jan 30, 2017

I'd be 👍 on this if it updated version and hash in the nix file in place but I don't like the src.json approach, too and would agree with @edolstra here..

@vcunat

This comment has been minimized.

Member

vcunat commented Jan 30, 2017

I firmly believe that what is stopping us from moving forward is that we are still manually updating packages which consume a time of 2 (if not more) nixpkgs contributors. I don't think this is a way forward.

I might be an outlier, but when I update packages, the time spent on bumping the version and hash is typically a very insignificant fraction of the whole process. (Just my 2 cents.)

@DavidEGrayson

This comment has been minimized.

Contributor

DavidEGrayson commented Jan 30, 2017

@edolstra You might consider removing the upstream-updater tool that @7c6f434c made from nixpkgs, which is designed to have two separate files for every Nix package: src-for-file.nix and src-info-for-file.nix. You would probably want to remove the associated updateWalker attribute too, and the mentions of it in the documentation.

This is partially a response to what @garbas said: I would prefer to have style guides and rules that are adhered to throughout nixpkgs rather than leaving big decisions up to individual package maintainers. That way, it's easier to work on any part of the project without learning new patterns. A senior member of the project can dictate what those rules are. It would also be great to get some official rules about how to write automatic Nix expression updater tools, like what languages and libraries to use, how the user interface should work, etc. (My two cents is to prefer Haskell and hnix but I can see the advantage of C++ and tolerate other languages too.)

@garbas: Wouldn't src.json be used by humans because humans need to read Nix expressions to understand what software is being compiled? You said "src.json files not meant to be used by humans but use a separate update process".

@jgeerds: If the src you want to update is a fetchgit or fetchFromGitHub call, check out our project: https://github.com/expipiplus1/update-nix-fetchgit

@olejorgenb

This comment has been minimized.

Contributor

olejorgenb commented Jan 30, 2017

I must admit I haven't bothered to update a minor release of zsh-completion, mostly because it's kind of a hassle to get the hash and update the file manually (+ create the commit and PR). This is partially because I don't do it very often and have fumbled a bit in the past getting the correct hash. (esp. with fetchFromGitHub)

Of course one should spend some time testing a new package version and make sure the dependencies haven't changed too. But if you actually use the package testing is simply getting the new version and using it as you normally would for a while.

I think a script that handles these simple updates would result in more up to date packages. Ideally the script would create a branch and commit too :)

@DavidEGrayson

This comment has been minimized.

Contributor

DavidEGrayson commented Jan 30, 2017

A language with lambdas/abstraction like Nix, even if we had easy AST manipulation tools, is fundamentally harder/impossible to programmatically modify because people can write something equivalent to a given expression in a bajillion ways, some of which don't even obviously terminate

@copumpkin For the sake of human-readability, I would say that you should be structuring your Nix expressions in a way where it is somewhat easy to tell what sources could be downloaded. And if you do that, I don't think it's too hard to have a computer program parse it and update it. I don't think you should be doing something too complex to calculate the URL of some source code or its hash.

@copumpkin

This comment has been minimized.

Member

copumpkin commented Jan 30, 2017

@7c6f434c

This comment has been minimized.

Member

7c6f434c commented Jan 31, 2017

@edolstra I would say that most of NixPkgs is pollution with little files (when compared with python-packages.nix, for example; and such a large file does make tracing some things more convenient).

@timbertson

This comment has been minimized.

Contributor

timbertson commented Jan 31, 2017

@edolstra I'm sorry to hear that. I'd like to address a few of your comments:

requires users to constantly switch between two syntaxes when editing a Nix package.

I find it very hard to believe this would be a problem in practice. JSON is just as readable as nix. And you will never be writing JSON source information wholesale, you'll be:

  • reading it (negligible difference)
  • generating it (now dramatically easier), or
  • modifying individual fields (why? maybe for a quick hack? whatever your reasons, this stays roughly as awkward as it is today)

It enshrines a style of having external src.json (or src.nix) files

There's been no agreement or encouragement that this is the one true way to automate updates or to structure derivations, it's just one option available to maintainers to choose if they want to.

I hope a smarter tool will appear which has the same features but can update inline nix expressions. But that doesn't exist, and I doubt it ever will - it's simply much trouble to bother with. Those that disagree are welcome to build it, and I'll gladly use it. But until then, I'm saddened to see a useful option being denied when there are no viable alternatives.

it prioritizes the convenience of tool builders over the convenience of human readers

I've got two responses to this:

First, the information in a src attribute is inherently boring. Yes, you might want to know where it comes from, or what version it is. But the information about the exact URL of the source code and in particular the sha256 hash is more mechanics than code - it's not like you'll actually know what the source contains by looking at a URL. Extracting that kind of information may not be on its own a positive move, but it also doesn't seem that damaging either.

Second, there are a lot of packages in nixpkgs, and they're mostly (all?) maintained by humans. Forgoing easy automation in favour of an inline syntax which must be updated manually[1] increases the effort required from every maintainer, for every package, for every version bump. Each instance is a small inconvenience, which some people aren't bothered by. But some are, and that can add up to a big deterrent. It's personally kept me from contributing multiple packages to nixpkgs - not because it's hard, but because it's tedious and error prone, and I already have enough work to do maintaining software.

[1]: Yes, I'm ignoring update-nix-fetchgit. It's impressive, but it doesn't do what I need. If that's the bar of complexity you need to pass in order to automate updating of source code, I for one am going to be stuck manually updating nixpkgs for a long time yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment