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.17.1 -> 2.18.0, round 2 #42575

Merged
merged 2 commits into from Jun 29, 2018
Merged

git: 2.17.1 -> 2.18.0, round 2 #42575

merged 2 commits into from Jun 29, 2018

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Jun 25, 2018

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@dtzWill dtzWill changed the title fetchgit: adapt regex to git 2.18.0 git: 2.17.1 -> 2.18.0, round 2 Jun 25, 2018
@dtzWill
Copy link
Member Author

dtzWill commented Jun 25, 2018

@layus is the hero behind this! See: #42376 (comment)

Anyway I haven't tested this with the breakage it caused on various PR's, will do so shortly.

@dtzWill
Copy link
Member Author

dtzWill commented Jun 25, 2018

LGTM -- tried nix-build -A lollypop.src --check (might need to run without --check first time) with/without the fix from @layus, seems to work with and fails without. \o/

@dtzWill
Copy link
Member Author

dtzWill commented Jun 25, 2018

LGTM but I'd prefer someone else to merge / confirm since I/we got it "wrong" the first time around :).

@worldofpeace
Copy link
Contributor

I tested it too and it seems to work ✋

@ciil
Copy link
Member

ciil commented Jun 28, 2018

lgtm

@xeji
Copy link
Contributor

xeji commented Jun 28, 2018

the chicken says: staging, maybe?

@dtzWill
Copy link
Member Author

dtzWill commented Jun 28, 2018

Sounds like a plan, no rush here AFAIK anyway.

@dtzWill dtzWill requested a review from FRidh as a code owner June 28, 2018 12:01
@dtzWill dtzWill changed the base branch from master to staging June 28, 2018 12:01
@dtzWill dtzWill removed the request for review from FRidh June 28, 2018 12:02
@dtzWill
Copy link
Member Author

dtzWill commented Jun 28, 2018

@GrahamcOfBorg build git gitFull

@FRidh
Copy link
Member

FRidh commented Jun 28, 2018

What was causing the failures with the last major bump causing it to be reverted 3 times? Is it a reasonable risk here as well? Is it perhaps an idea to test it by building a smaller job?

@dtzWill
Copy link
Member Author

dtzWill commented Jun 28, 2018

What was causing the failures with the last major bump causing it to be reverted 3 times? Is it a reasonable risk here as well? Is it perhaps an idea to test it by building a smaller job?

Good question--after that silliness we (well, @layus mostly, IIRC) fixed and started always running git's (installCheck) tests to ensure things at least mostly work as expected.

These tests would have caught the problem we encountered last time, re:#38387.

Things were made worse by updates being merged separately (perhaps separate PR's? I don't recall) onto both master and staging, and we kept reverting as the two were merged into each other :(.

The update to 2.18.0 was reverted once already, but not due to general usage failure in the package itself but because our fetchgit apparently relied on behavior upstream changed-- but so IMO this is different since git was working "as intended". Since breakage resulting from git working as expected seems unlikely, I'd say staging is appropriate but if folks disagree I certainly won't stand in the way of performing additional testing first.

@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Attempted: git, gitFull

Partial log (click to expand)

cannot build derivation '/nix/store/zhdncxbqspnak1i3kn6sfl6bzn9w0f91-opensp-1.5.2.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/kck48jwbwip7b4b9xsz9g6ixj029ymb4-serf-1.3.9.drv': 10 dependencies couldn't be built
cannot build derivation '/nix/store/92cll8baa995mfympkqkzizxq65xr38p-docbook2X-0.8.8.drv': 16 dependencies couldn't be built
cannot build derivation '/nix/store/g8axfs3rwdlj2f1nhsdcqs2knfgk8464-libXrender-0.9.10.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/17p7x39ql012knmhf16ndkqbbby4ha7b-subversion-1.9.7.drv': 12 dependencies couldn't be built
cannot build derivation '/nix/store/ilc1khy1pld25c3sv5n5ck4flrfyhn3i-git-2.18.0.drv': 29 dependencies couldn't be built
cannot build derivation '/nix/store/85z1jahrsd2r6w5fj10hv9kpv5j1h3s9-libXft-2.3.2.drv': 8 dependencies couldn't be built
cannot build derivation '/nix/store/4bsihm57ns8rga954dz88ygcz52w1mr0-tk-8.6.6.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/8d53fx7mihp3mzc5hhg4vlni1wrdipvq-git-2.18.0.drv': 32 dependencies couldn't be built
�[31;1merror:�[0m build of '/nix/store/8d53fx7mihp3mzc5hhg4vlni1wrdipvq-git-2.18.0.drv', '/nix/store/ilc1khy1pld25c3sv5n5ck4flrfyhn3i-git-2.18.0.drv' failed

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: git, gitFull

Partial log (click to expand)

Files=816, Tests=19219, 216 wallclock secs ( 5.06 usr  1.05 sys + 287.42 cusr 275.58 csys = 569.11 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory '/build/git-2.18.0/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory '/build/git-2.18.0/t'
make[1]: Leaving directory '/build/git-2.18.0/t'
/nix/store/7zqshcw39jn3cv8dk5a08rjgl4hl4zxg-git-2.18.0
/nix/store/9c88lp6mwvakckpsf1alh5zp5qs8n7z1-git-2.18.0

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: git, gitFull

Partial log (click to expand)

Files=816, Tests=19219, 285 wallclock secs (81.41 usr 17.18 sys + 2735.20 cusr 6858.77 csys = 9692.56 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory '/build/git-2.18.0/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory '/build/git-2.18.0/t'
make[1]: Leaving directory '/build/git-2.18.0/t'
/nix/store/lp7rciszywn3jmwmmfprd0g5rjx1bhyi-git-2.18.0
/nix/store/7bs2p7dlq9vdnydsmwwy83hbgbrnj1r5-git-2.18.0

@xeji xeji merged commit 3892a3e into NixOS:staging Jun 29, 2018
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

7 participants