Skip to content
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

git: 2.10.0 -> 2.10.1 #19248

Merged
merged 1 commit into from
Oct 5, 2016
Merged

git: 2.10.0 -> 2.10.1 #19248

merged 1 commit into from
Oct 5, 2016

Conversation

FRidh
Copy link
Member

@FRidh FRidh commented Oct 5, 2016

Motivation for this change
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Not yet tested!

@mention-bot
Copy link

@FRidh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @phanimahesh, @peti and @edolstra to be potential reviewers.

@dezgeg
Copy link
Contributor

dezgeg commented Oct 5, 2016

Why would updating git be a mass rebuild (it doesn't affect fetchgit output hashes)?

@FRidh
Copy link
Member Author

FRidh commented Oct 5, 2016

Judging from the nox-review output there's still a great deal of packages affected by it.

@phanimahesh
Copy link
Contributor

Yes, I was worried about that in my previous update to git, too. git depends on many things, (svn and tcl are among the not very obvious ones if i remember correctly), and somehow way too many things depend on git.

@grahamc
Copy link
Member

grahamc commented Oct 5, 2016

How about this one, @vcunat re staging?

@peti
Copy link
Member

peti commented Oct 5, 2016

The mass-rebuild label is intended for stdenv changes. git is a fundamental package, no doubt, but it's nowhere near as expensive as a change to, say, gcc.

@peti peti merged commit fad5794 into NixOS:master Oct 5, 2016
@grahamc
Copy link
Member

grahamc commented Oct 5, 2016

@peti I think I've heard conflicting ideas on that from @vcunat, but okay :)

@FRidh FRidh deleted the git branch October 5, 2016 11:49
@peti
Copy link
Member

peti commented Oct 5, 2016

@grahamc, we've been merging git updates straight into master for the last 7,5 years. I'd be surprised if @vcunat disagreed with that practice.

@grahamc
Copy link
Member

grahamc commented Oct 5, 2016

Perhaps not disagrees, but based on what he was saying it sounded like updates that trigger many rebuilds are candidates for staging to reduce friction for other contributions. Since this one did exactly that, I wanted his feedback.

Edited to say: my understanding was the same, that staging was just for stdenv changes, and now I'm trying to suss out the finer points.

@vcunat
Copy link
Member

vcunat commented Oct 5, 2016

It is a mass rebuild, unfortunately. The dependency chain starts like this: git -> fontforge -> dejavu_fonts -> fontconfig. (The last step here was introduced by me, relatively recently 4f73633.)

@Mic92
Copy link
Member

Mic92 commented Oct 5, 2016

fontconfig needs git to create a new mac os x release bundle.
I could disable it by using:

diff --git a/pkgs/tools/misc/fontforge/default.nix b/pkgs/tools/misc/fontforge/default.nix
index ba1154a..7517dc2 100644
--- a/pkgs/tools/misc/fontforge/default.nix
+++ b/pkgs/tools/misc/fontforge/default.nix
@@ -1,5 +1,5 @@
 { stdenv, fetchFromGitHub, fetchpatch, lib
-, autoconf, automake, gnum4, libtool, git, perl, gnulib, uthash, pkgconfig, gettext
+, autoconf, automake, gnum4, libtool, perl, gnulib, uthash, pkgconfig, gettext
 , python, freetype, zlib, glib, libungif, libpng, libjpeg, libtiff, libxml2, pango
 , withGTK ? false, gtk2
 , withPython ? true
@@ -17,6 +17,9 @@ stdenv.mkDerivation rec {
     sha256 = "15nacq84n9gvlzp3slpmfrrbh57kfb6lbdlc46i7aqgci4qv6fg0";
   };

+  # git isn't really used, but configuration fails without it
+  GIT=/usr/bin/env;
+
   patches = [(fetchpatch {
     name = "use-system-uthash.patch";
     url = "http://pkgs.fedoraproject.org/cgit/fontforge.git/plain/"
@@ -27,7 +30,7 @@ stdenv.mkDerivation rec {

   # FIXME: git isn't really used, but configuration fails without it
   buildInputs = [
-    git autoconf automake gnum4 libtool perl pkgconfig gettext uthash
+    autoconf automake gnum4 libtool perl pkgconfig gettext uthash
     python freetype zlib glib libungif libpng libjpeg libtiff libxml2
   ]
     ++ lib.optionals withGTK [ gtk2 pango ]

@vcunat
Copy link
Member

vcunat commented Oct 5, 2016

That doesn't build for me.

@vcunat
Copy link
Member

vcunat commented Oct 5, 2016

... but that's just due to sandboxing; the approach works in principle and I think we should take it.

vcunat added a commit that referenced this pull request Oct 5, 2016
... to avoid git changes being mass rebuilds.
Thanks to Mic92 for the solution idea.
See discussion under: #19248
@vcunat
Copy link
Member

vcunat commented Oct 5, 2016

Staged a variant of this that works with sandboxing: 07d12fb. @Mic92: thanks!

@Mic92
Copy link
Member

Mic92 commented Oct 5, 2016

strange, I thought I have also sandbox enabled:

# /etc/nixos/configuration.nix

nix = {
  extraOptions = ''
    build-use-sandbox = true
  ''
}

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

Successfully merging this pull request may close these issues.

None yet

8 participants