Git package unnecessarily depends on GCC

demin-dmitriy commented 5 years ago

Issue description

On current master c7276b2b3d05e5191a9b5b7236d63edf49204f79 package gitMinimal depends on gcc, which blows up it's closure size.

Steps to reproduce

$ nix-store -qR $(nix-build . -A gitMinimal --no-out-link) | grep gcc
/nix/store/dmgpvd9nl7qa0ydzlqxvdhmxgmq1d1il-gcc-7.4.0-lib
/nix/store/n4q167a25rkvb043rf0sw7z4cixgv48x-gcc-7.4.0

$ nix path-info -f . -r gitMinimal -S | sort -k2h | tail -n3
/nix/store/vyqkax7dmh1jb62klccmwkg00b9951zz-openssl-1.0.2s-dev             90747512
/nix/store/n4q167a25rkvb043rf0sw7z4cixgv48x-gcc-7.4.0                     177044600
/nix/store/835jdm4r0xwwnpi0a1683dmmj40065mm-git-2.22.0                    345567656

Technical details

On very old version of nixpkgs (6c064e6b1f34a8416f990db0cc617a7195f71588) there was no such problem:

$ nix-store -qR $(nix-build . -A gitMinimal --no-out-link) | grep gcc
/nix/store/qc84dvliy1dzpidw10yvpi3di5f0q4vj-gcc-7.3.0-lib

$ nix path-info -f . -r gitMinimal -S | sort -k2h | tail -n3
/nix/store/ggb7k5x9855j10dz99467djx4rplg32b-perl-5.24.3                    79216672
/nix/store/1c0dmikm575as2jqfi07zydwzfpf5jzy-perl-libwww-perl-6.15          80209176
/nix/store/mivirzq63vv8rwm58fl7x7nc6jfzr27w-git-2.17.0                    152274288

gloaming commented 5 years ago

$ cd $(nix-build . -A gitMinimal --no-out-link)

$ nix-store -q --tree . | grep 'gcc-7.4.0 '
|   +---/nix/store/n4q167a25rkvb043rf0sw7z4cixgv48x-gcc-7.4.0 [...]

$ ag -u n4q167a25rkvb043rf0sw7z4cixgv48x
Binary file libexec/git-core/git-stash matches.

$ strings libexec/git-core/git-stash | ag n4q167a25rkvb043rf0sw7z4cixgv48x
/nix/store/n4q167a25rkvb043rf0sw7z4cixgv48x-gcc-7.4.0/lib/gcc/x86_64-unknown-linux-gnu/7.4.0/include
** repeated 330 times **
/nix/store/n4q167a25rkvb043rf0sw7z4cixgv48x-gcc-7.4.0/lib/gcc/x86_64-unknown-linux-gnu/7.4.0/crtbegin.o
/nix/store/n4q167a25rkvb043rf0sw7z4cixgv48x-gcc-7.4.0/lib/gcc/x86_64-unknown-linux-gnu/7.4.0/crtend.o

No idea.

demin-dmitriy commented 5 years ago

The breaking commit is eeb9e6c7623450f0cb986d315d7813abd59f041d which just bumps git version. Maybe they added -g flag to their makefile or something like that.

gloaming commented 5 years ago

(master)$ ag 'CFLAGS.*-g' Makefile
1155:CFLAGS = -g -O2 -Wall

Removing it doesn't help, though. I think stripping would remove anything from -g, anyway.

gloaming commented 5 years ago

The upstream commit that breaks it is: https://github.com/git/git/commit/40af14683432285b94407e8488eab6942d0779dc But I don't see why.

N.B. They broke the (Nix) build around here as well, use this to get it working:

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 6906aae441..eaee2ac8d6 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -4,7 +4,7 @@ all::
 -include ../../config.mak.autogen
 -include ../../config.mak

-prefix ?= /usr/local
+prefix ?= $(out)
 gitexecdir ?= $(prefix)/libexec/git-core
 mandir ?= $(prefix)/share/man
 man1dir ?= $(mandir)/man1

demin-dmitriy commented 5 years ago

I don't completely understand what's happening but I'm pretty sure that postInstall script is breaking it. If I understood correctly git-stash used to be a shell script, and now it's a symlink. This patch fixes the issue.

diff --git a/pkgs/applications/version-management/git-and-tools/git/default.nix b/pkgs/applications/version-management/git-and-tools/git/default.nix
index 7d0b28bd6cc..e49c4ad57b8 100644
--- a/pkgs/applications/version-management/git-and-tools/git/default.nix
+++ b/pkgs/applications/version-management/git-and-tools/git/default.nix
@@ -162,7 +162,7 @@ stdenv.mkDerivation {
       EOS
       )"
       perl -0777 -i -pe "$SCRIPT" \
-        $out/libexec/git-core/git-{sh-setup,filter-branch,merge-octopus,mergetool,quiltimport,request-pull,stash,submodule,subtree,web--browse}
+        $out/libexec/git-core/git-{sh-setup,filter-branch,merge-octopus,mergetool,quiltimport,request-pull,submodule,subtree,web--browse}

       # Also put git-http-backend into $PATH, so that we can use smart

I haven't tested this on master yet. What I don't understand is why resulting package is even working after that script.

demin-dmitriy commented 5 years ago

A-ha. I understand what's happening on master. I'm not sure why I see something different on eeb9e6c.

That perl script is overwriting symlink to git with a copy of git. And it also incorrectly patches it so strip refuses to work on it:

strip:out/libexec/git-core/git-stash: file format not recognized

(this error is ignored and silenced to /dev/null, so you won't see it in build log)

Anyway, I'll make a pull request.

gloaming commented 5 years ago

Nice! Yes, I saw that strip,lld, and nm barfed on it but I thought it was some kind of linker error - didn't realise it was getting mutilated!

That postInstall script is horrendous. I take it you intend to make it check the files are actually scripts first - this could easily happen again with a different git command, plus it wastes disk space :)

gloaming commented 5 years ago

Yeah, it's amazing the binaries still worked at all!

jabranham commented 5 years ago

I think this issue can be closed, right?

demin-dmitriy commented 5 years ago

Right. I forgot that it was still open.

NixOS / nixpkgs