btrfs-progs: build from source, not release tarball

[Atemu]

Motivation for this change

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Added a release notes entry if the change is major or breaking
• Fits CONTRIBUTING.md.

[alyssais]

Why?

[Atemu]

To make it easier to override with e.g. development versions. Also makes the build more reproducible since we actually build from source and not rely on some unknown version of autotools that were run on some machine somewhere to generate the configure script for us.

[alyssais]

If upstream is publishing a release tarball, that presumably means they want downstreams to build from the tarball. I don't think we should deviate from that (in general, not just for this package) unless we have a good reason that applies to the package itself, not overrides people might want to do.

I don't see how using configure scripts that are part of the tarball (and therefore protected against changing by the hash) could cause reproducibility issues.

[Atemu]

Well then we might as well fetch binaries since they're protected by a hash too. I'm not talking about the "different run, different result" type of r13y, I mean being able to reproduce the whole build, not just a part of it.

Our packages should be a pure transformation from human-made source code (which is -by definition- not reproducible) to the end-result IMO.

[alyssais]

That'd be a departure from existing practice -- we've always preferred using release tarballs, and not having autoconf/automake dependencies unless necessary for patching. There are probably hundreds of packages in Nixpkgs that work this way.

I don't think this is a change that should be made piecemeal, one package at a time, with different people making different arguments and coming to different conclusions every time.

[r-rmcgibbo]

Result of nixpkgs-review pr 124974 at 50f367a run on x86_64-linux 1

114 packages skipped due to time constraints:

• adapta-gtk-theme
• almanah
• bareos
• bedup (python38Packages.bedup)
• bees
• btrbk
• bubblemail
• buildah
• calls
• cantata
• ...

16 packages built successfully:

• btrfs-progs
• buildah-unwrapped
• charliecloud
• compsize
• containerd
• cri-o-unwrapped
• dive
• docker (docker-edge ,docker_20_10)
• docker-gc
• dockle
• fn-cli
• grype
• nix-prefetch-docker
• podman-unwrapped
• skopeo
• udisks (udisks2)

1 suggestion:

• warning: unclear-gpl

  gpl2 is a deprecated license, please check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

  Near pkgs/tools/filesystems/btrfs-progs/default.nix:65:5:

     |
  65 |     license = licenses.gpl2;
     |     ^
  

[SuperSandro2000]

Can we simplify this with autoreconfHook?

[Atemu]

Unfortunately not.

[dotlambda]

How about

Suggested change

	preConfigure = ''
	patchShebangs autogen.sh
	sh autogen.sh
	'';
	preAutoreconf = ''
	patchShebangs autogen.sh
	'';

?

[Atemu]

auroreconfHook doesn't work because autoconf --force returns this:

configure.ac:7: error: possibly undefined macro: AC_DEFINE
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure.ac:246: error: possibly undefined macro: AC_MSG_WARN

Running autogen.sh is the only way to get the source to configure AFAICT.

[Atemu]

patchShebangs was redundant.

[ajs124]

I completely agree with @alyssais on this. If we're going to do this, it should be through a policy decision that "this is how we do it from now on".

[alyssais]

(Just to clarify, I do think your points are good, and we probably should run autoconf ourselves.)

[Atemu]

What would be the best way to make such a policy decision? Feels a bit small for a full RFC.

A Nixpkgs issue with RFC perhaps?

[dotlambda]

What would be the best way to make such a policy decision? Feels a bit small for a full RFC.

A Nixpkgs issue with RFC perhaps?

The decision should be part of NixOS/rfcs#89.
cc @risicle

[7c6f434c]

Feels a bit small for a full RFC.

It is larger than NixOS/rfcs#45 — which took quite some time to build at least a universally resigned attitude that nothing in it is bad enough to block. Global-impact changes are worth an RFC, small is good. And there will be scope creep to fight here, just believe me on that.

The decision should be part of NixOS/rfcs#89.

Please no. A project to collect some data for packages, and a project to shift our approach to building the packages. These do not really depend on each other, and so should not block each other or come as a bundle. GitHub is bad enough at handling large discussions to avoid mixing two discussions that can be just as easily separated.

[stale]

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

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/reconsider-reusing-upstream-tarballs/42524/19