experimental/multiple-versions-in-one-file

[MarcWeber]

This patch introduces mergeAttrsByVersion to lib.

In some cases it does make sense to keep "experimental" and "older" versions of
the same package in one file because it follows the "do it once only" principle.
Very often many versions share their build instructions - so even though I
understand Eelcos opinion that different versions should be put into their own
.nix files - its my feeling telling me that this is fastest for most cases.

I agree with Eelco that if tweaks for individual versions become too much they
should be split into individual files.

See comments above mergeAttrsByVersion learn about its usage.

Signed-off-by: Marc Weber marco-oweber@gmx.de

[MarcWeber]

I agree that this patch might be controversial. So take time to think and comment about it. Some more work I'd like to contribute depends on it - such as cvs version of gutenprint, PHP fpm implementation etc.

[shlevy]

I'm tentatively in favor of this. Can you show some examples that show how/when this improves over the current "separate file for each version" approach?

[MarcWeber]

Whenever you're tired of copy pasting and creating new files for simple version bumps, eg when you want to try whether a new version fixes issues etc. All advantages of code sharing apply, also all disadvantages. If you later want to change a maintainer for one version, then it may be best to start with two files. But usually nobody cares, things just get fixed.

It may be easier to maintain if you need dev versions. Eg some time Ago I needed bleeding edge gimp.
It looks like this: http://mawercer.de/tmp/gimp-git.patch. If you look at this patch you'll notice that I kept 2.6, 2.8 and git version working, all of them picking different gegl, babl versions. I don't say its the nicest way to maintain it, but it works reasonable well. The big issue is that you do not always know what will change in the future, being able to override the version is a quick way to try what works fast. That's all about it.

[bjornfor]

+1. Being able to choose whether to split out a new package version into a separate file or not seems like a good thing.

[peti]

If all code can be shared except for the src attribute, then why don't you just pass src to the function as an argument? That seems much simpler to me?

[MarcWeber]

Excerpts from Peter Simons's message of Tue Apr 23 15:11:20 +0200 2013:

If all code can be shared except for the src attribute, then why
don't you just pass src to the function as an argument?

Because this looks ugly (in all-packages.nix)

  foo_1_0 = callPackages { .. };
  foo_2_0 = foo_1_0.override {
    srcc = ..
  };

• and sometimes you also have to "merge" some additional buildInputs or
  add more build configurations such as

  { version ? "1.0" # could be "git"
  , more args }:

  mkDerivation (byVersion "name" version {
  "git" = {
  name = "name-git";
  src = 'git or svn sources';
  buildInputs = [ automake autoconf libtool ];
  preConfigure = "autogen.sh";
  configureFlags = [ '--disable-foo' ]; // because I'm too lazy to care about it at the moment
  };
  "1.0" = {
  // nothing special about 1.0, default setup
  name = ...
  src = fetchurl { ... };
  } {

  meta = {
    ...
  }
  

  }

That's a common case. And such simple cases are the ones I'd like to use
it for. If things get too complicated it should be refactored.

Because its easy to understand the differences by looking
at one file. If you have multiple files you have to use diff and whatnot.

Marc Weber

[bbenoist]

I don't really understand why these kind of problems cannot be resolved with a version option and appropriate conditions above the .nix file only when necessary. 😕
I have previously managed to handle different builds of a same package with differing buildInputs and configuration options by simply toggling a boolean in all-packages.nix, why can't the version information act the same way with current code?
If this is specific to the version information (I thinked it was an optional one), what makes it more or less valuable in terms of consequences than any other information making a package differ from it's default build?

[MarcWeber]

You're right that it can be done. http://mawercer.de/tmp/gimp-git.patch shows that I have "regions" which update name and src at the same time - so that the derivation name always contains svn/git revision. That's harder doing the way you propose. Anyway, I was wrong: This patch is not required by my PHP patches - so maybe I should just create pull requests and see what happens ..

[peti]

I's subjective, of course, but code like

foo_1_0 = callPackage ../foo/bar { version = "1.0"; sha256 = "abcdef"; };
foo_2_0 = callPackage ../foo/bar { version = "2.0"; sha256 = "fedcba"; };
foo_2_1 = callPackage ../foo/bar { version = "2.1"; sha256 = "123456"; };

doesn't look ugly to me at all. I think that's a clear and precise description of the intended outcome.

[bjornfor]

foo_1_0 = callPackage ../foo/bar { version = "1.0"; sha256 = "abcdef"; };

This looks very nice and readable. Is it possible to use that in current nix/nixpkgs? I.e. does it it work out-of-the-box or does it require ../foo/bar/default.nix to do some tricks?

[peti]

Yes, this style can be used without any extensions whatsoever. The build
expression would simply expect version and sha256 as arguments,
insert them into the appropriate places, and that's it.

[Phreedom]

I guess it's been already established that this patch adds nothing new in terms of our ability to customize and override derivations. You can override pretty much everything using overrideDerivation on the all-packages.nix side, or pass your customizations or customization names via parameters.

The old-school approach for derivation-side customization would be something like buildInputs = [ dep1 ] ++ lib.optional (version =="2") dep2 ++ lib.optional (version == "3") dep3 etc wherever it's required. Marc's patch makes this stuff more concise by not having to repeatedly put lib.optional (version == something) all over the place. It is hard to tell whether having all differences in one place like Marc proposes is better than having all differences enumerated in each derivation parameter the old way. You'll never know until you try, although I suspect that the concise syntax will win. I would also suggest replacing mkDerivation (mergeAttrsByVersion $PARAMS) with versionedDerivation $PARAMS to make it look a bit cleaner.

And the hardest question is: do we want to enforce single-version-per-file approach. Multiple-versioned derivations risk breaking older versions if the developer is not careful when hacking the bleeding-edge version. This can be countered to a degree by writing a tool to check whether all available versions still build. This is balanced by making it more likely that improvements will be propagated back to older versions, such as nixpkgs-specific patches or newly-packaged optional dependencies.

Another consideration is being able to see the differences between derivations. Diffing derivations works, but provides a lot more visual clutter to sift thru. Formatting changes add more clutter and don't propagate back to older versions. Git handles renaming default.nix to old-version.nix well, but does it recognize copying default.nix to new-version.nix with a slight modification? When browsing the repository with qgit, a patch for a new version added to a multiple-versioned derivation would be concise and informative, is the same going to happen when a new version of a single-version derivation is committed?

[MarcWeber]

Let me clarify: I don't want to rewrite nixpkgs. - I don't want to force this solution on all dervations.
I just think this way of "keeping many versions" is convenient in some places - even if its temporary for testing only.
Its also faster because you don't have to create new files.

If you want to add comments (like I did in the PHP case) - there is one file to document a package. If you have many versions - documentation is cluttered.

About breakage: Even PHP-5.2 broke - not because PHP-5.2 was changed - but because dependencies changed.
So "not touching old versions" is no guarantee that they keep working.

I personally feel it could be useful for packages which are not used that often - so that tracking down bugs is little bit faster - packages like "apc" for PHP. I see it as util like misc.nix stuff.

Patching like this: NixOS/nixos#153 and the related nixpkgs patch ore more important to the community - because it makes php-fpm just work.

[Phreedom]

I wasn't talking about forcing anything either. Was trying to find out what would be wrong if people actually liked this approach and if it would spread. Just in case I wasn't clear, I'm in favor of this patch with a minor cosmetic cleanup.

[Phreedom]

Lets see if developers put this to good use. Any objections? @MarcWeber how about my suggestion "replacing mkDerivation (mergeAttrsByVersion $PARAMS) with versionedDerivation $PARAMS" to make it look nicer and more self-descriptive?

[MarcWeber]

I've added versionedDerivation name - and I've put it into misc.nix - because I think future will tell in which ways this might be (ab)used. It is an experiment. If it fails we should remove it again.

[Phreedom]

Rebased and merged. Going to try using it for a couple of packages and see how it works.

[edolstra]

@MarcWeber The place for experiments is branches, not the main tree. "Experiments" have a tendency to stick around for a long time, especially if there is code using them...

[edolstra]

Also, surely there are more descriptive function names than "mergeAttrsByFuncDefaultsClean" :-)

[edolstra]

Thinking about it a bit, is there any reason why you can't write it like this:

foo-2 = stdenv.mkDerivation (common // {
  src = fetchurl ...;
});

foo-git = stdenv.mkDerivation (common // {
  src = fetchurl ...;
  preConfigure = "autogen.sh"; 
  buildInputs = [automake autoconf libtool];
});

common = {
  ...
};

Note there's no passing around of weird version arguments like "2.0", and no use of .override that leads to a fragile base class problem.

[Phreedom]

This is an experiment whether we get an improvement in workflow or not. I don't understand how it can be done in a branch. Also it seems rather easy to get rid of the derivation written in this style: on the next version bump, you copy the common portion, append a version-specific portion and replace versionedDerivation with mkDerivation.

[Phreedom]

But this turns the derivation into a collection of packages... can you provide a more complete example, startin with what would be put into all-packages.nix. Also, what happens if common and foo-2 both have eg patches? Is the list going to be merged?

[MarcWeber]

Its still true: to get a discussion going - the only way is comitting something
to trunk:

How do I understand the base class problem?

If you change the base class, everything derived from it will change. Thus you
have to retest everything derived from it, right?

We have to kinds of packages:

• gcc like well maintained packages
• less maintained packages

If you base your package on gcc, rather than on gcc-4.3, you also have the base
class problem: If you update, packages will break. same applies to glib, gtk.
So the base class problem is everywhere.
If you avoid the base class, you end up having security risks, because some
packages still depend on old broken code (eg PHP 5.2 even does no longer compile,
not because the derivation code was changed, but because openssl had to be
updated for security reasons)

So I think its easy to say "the base class problem" is always there.

Let's get into details. Example:

test_derivation = stdenv.mkDerivation( stdenv.lib.mergeAttrsByVersion "name" "2.0" {

"git" = {
  # git requires something totally different, don't inherit
  mergeAttrBy.buildInputs = (a: b: b); # change b to a, and you'll get the exception
  buildInputs = [ A-new B-new C-new ]
  # X is broken in git version, --enable-X --disable-X will be passed, but the latter should win
  configureFlags = [ "--disable-X" ];
}
"2.0" = {
  buildInputs =  [ "input" "new-X" ];
};
"1.5" = {
  buildInputs =  [ "old-X" ];
}
} {
  name = "x";
  src = "none";
  buildInputs =  [ A B C ];
  configureFlags = [ "--enable-X" ];

  mergeAttrBy.src = "define source merge function"; # does not make sense, but could be done.
});

Because mergeAttrsByFuncDefaults(Clean) derives merge behaviour from the
mergeAttrBy name which is passed using the attrs its easy to extend.
Yes - there is another "base class problem", you have to lookup

mergeAttrs = listToAttrs (map (n : nameValuePair n lib.concat)
  [ "nativeBuildInputs" "buildInputs" "propagatedBuildInputs" "configureFlags" "prePhases" "postAll" "patches" ])
// listToAttrs (map (n : nameValuePair n lib.mergeAttrs) [ "passthru" "meta" "cfg" "flags" ])
// listToAttrs (map (n : nameValuePair n (a: b: "${a}\n${b}") ) [ "preConfigure" "postInstall" ])

default definition, which is also defined in lib/misc.nix to find out what is actually happening.
But there is a faster way: add (throw "x") to any list, and see whether there
is an evaluation error. If there is - things get merged :)

Having a look at the example above you'll notice that opting-out from merging
is possible - but things get complicated - so in such a case I'd agree to
remove the base class for the buildInputs attr name.
And that exactly is the strength: You can avoid duplication, while adopting a
derivation to new requirements (making a new version compile) - thereby minimizing
code duplication.

The worst thing which could happen is that you end up in:

test_derivation = stdenv.mkDerivation( stdenv.lib.mergeAttrsByVersion "name" "2.0" {
"git" = { all items here };
"2.0" = { all items here };
"1.5" = { all items here };
} {
meta = { this is left, still shared };
});

and you also got rid of the base class transition. However keep in mind that
using it did never - require to introduce new files. However the code was not
written for such extrem cases - it was written for the average case where only
some attrs (src, additional buildInputs) happen.

Another example is: https://github.com/NixOS/nixpkgs/blob/7d9607f1511bfe496fcd77ef735e7fc7f1c6eb9f/pkgs/development/libraries/dbus/default.nix
its only using mergeAttrsByFuncDefaults(Clean), and configureFlags get
naturally shared - which is what you usually want. Its like passing unpack code
to setup.sh by default - it tries to make the case I feel is more common, easy.

Overridding configure flags may just work - because the last flag should be
honored by the configure script.
If configureFlags fails, you can opt-out like show above, better is breaking
the base class problem the way Eelco prefers for at least that one attr name.

Let's summarize:

• If you don't abuse the usage - the main difference is that everything may end
  up in one file, but separate.
• If there are small changes, it allows to minimize the code which has to be
  changed, written, and read by humans
• Even if code supporting experiments is in trunk, it does not mean it has to
  be used in trunk everywhere, because "experimental branches" are always
  derived from trunk, correct?
• Its always very important to document which is the current most used/maintained
  version - and this is enforced by default ? "2.0" argument usually.

But I agree, I also like

foo_2_0 = {
}
foo_2_3 = {

}

so I'd even replace

foo.override { version = "2.3"; }

by foo."2.3" while foo still refers to the default version. (Could be done by
some passthru hackery) - I didn't care enough yet - because I use this code for
experimental experiments most often - such as introducing APC (PHP cache) -
without knowing whether new versions break older PHP versions - thus I'd like
to keep both for the time being. I know its "lazy" - which is sometimes good,
and sometimes bad. Time tells - you do not always know in advance.

About the naming: Eelco, do you have a better proposal?

[Phreedom]

A very good writeup. I also would prefer "foo."2.3" while foo still refers to the default version." It looks so much cleaner and is much more compatible with the solution that Eelco has proposed in the sense that it would be easier to move away from versionedDerivation should we find that in practice we don't need to customize merging and are ok with the replacement override strategy as offered by //. If you have time to implement this, please do so.

I have one more practical concern with multiple-version derivations(regardless of implementation): since diff doeesn't really understand what's going on, you might encounter merge conflicts when someone updates one of the versions in trunk, while you have updated another version in your branch. How often this is going to happen, only experience will tell. I don't expect it to happen very often and I'm mentioning it for completeness sake.