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

valgrind: Apply upstream patch for Makefile race in coregrind #51107

Merged
merged 1 commit into from Nov 28, 2018

Conversation

srhb
Copy link
Contributor

@srhb srhb commented Nov 27, 2018

Motivation for this change

Supercedes #51082
Thanks @delroth for noticing the commit upstream. :)

Things done
  • 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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@srhb
Copy link
Contributor Author

srhb commented Nov 27, 2018

Hmm, not actually sure that URL is stable... Investigating.

@srhb srhb force-pushed the valgrind-coregrind-makefile-race branch from 20c8bdb to a067e88 Compare November 27, 2018 11:46
@srhb
Copy link
Contributor Author

srhb commented Nov 27, 2018

Vendored the patch. Should be obvious when we need to get rid of it. :)

@srhb
Copy link
Contributor Author

srhb commented Nov 27, 2018

@FRidh Are you okay with adding this to staging-next? fwiw last eval after master was merged into staging-next failed again: https://hydra.nixos.org/build/84945477

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: valgrind

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/a3cxvxcbxwb932vyd8by0df844a5zgrz-valgrind-3.14.0-man
gzipping man pages under /nix/store/a3cxvxcbxwb932vyd8by0df844a5zgrz-valgrind-3.14.0-man/share/man/
patching script interpreter paths in /nix/store/a3cxvxcbxwb932vyd8by0df844a5zgrz-valgrind-3.14.0-man
checking for references to /build/ in /nix/store/a3cxvxcbxwb932vyd8by0df844a5zgrz-valgrind-3.14.0-man...
shrinking RPATHs of ELF executables and libraries in /nix/store/nrx9xlvadg7sblsqx9xr0kap81bch1bn-valgrind-3.14.0-doc
patching script interpreter paths in /nix/store/nrx9xlvadg7sblsqx9xr0kap81bch1bn-valgrind-3.14.0-doc
checking for references to /build/ in /nix/store/nrx9xlvadg7sblsqx9xr0kap81bch1bn-valgrind-3.14.0-doc...
shrinking RPATHs of ELF executables and libraries in /nix/store/597s26lr4nxwj9qn6hgfakill568n10w-valgrind-3.14.0-debug
patching script interpreter paths in /nix/store/597s26lr4nxwj9qn6hgfakill568n10w-valgrind-3.14.0-debug
checking for references to /build/ in /nix/store/597s26lr4nxwj9qn6hgfakill568n10w-valgrind-3.14.0-debug...

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: valgrind

Partial log (click to expand)

/nix/store/8dxrc9gf2i3ax4cvrca93hmbcinvd6mn-valgrind-3.14.0/bin/callgrind_control: interpreter directive changed from " /usr/bin/perl -w" to "/nix/store/qsihc3zr098a4kjvvk8zaawpvpi7yzxk-perl-5.28.0/bin/perl -w"
strip is /nix/store/fsirl1jzjb31850n0zd5navr9ahrf936-cctools-binutils-darwin/bin/strip
stripping (with command strip and flags -S) in /nix/store/1n5gvhg952122al60dxd3rpfd7cs6qbd-valgrind-3.14.0-dev/lib
patching script interpreter paths in /nix/store/1n5gvhg952122al60dxd3rpfd7cs6qbd-valgrind-3.14.0-dev
gzipping man pages under /nix/store/r610sxv1qi6qqa7a196yfsswh1jdmllq-valgrind-3.14.0-man/share/man/
strip is /nix/store/fsirl1jzjb31850n0zd5navr9ahrf936-cctools-binutils-darwin/bin/strip
patching script interpreter paths in /nix/store/r610sxv1qi6qqa7a196yfsswh1jdmllq-valgrind-3.14.0-man
strip is /nix/store/fsirl1jzjb31850n0zd5navr9ahrf936-cctools-binutils-darwin/bin/strip
patching script interpreter paths in /nix/store/hd5wxsqp2qzr85g9isypy1fqdsxvm11r-valgrind-3.14.0-doc
/nix/store/8dxrc9gf2i3ax4cvrca93hmbcinvd6mn-valgrind-3.14.0

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: valgrind

Partial log (click to expand)

gzipping man pages under /nix/store/5n1yw2aa25rcd6fwipsvir0rxwq84qx7-valgrind-3.14.0-man/share/man/
patching script interpreter paths in /nix/store/5n1yw2aa25rcd6fwipsvir0rxwq84qx7-valgrind-3.14.0-man
checking for references to /build/ in /nix/store/5n1yw2aa25rcd6fwipsvir0rxwq84qx7-valgrind-3.14.0-man...
shrinking RPATHs of ELF executables and libraries in /nix/store/b3bj4saq1rnhhj5qaa987035qw4ll440-valgrind-3.14.0-doc
patching script interpreter paths in /nix/store/b3bj4saq1rnhhj5qaa987035qw4ll440-valgrind-3.14.0-doc
checking for references to /build/ in /nix/store/b3bj4saq1rnhhj5qaa987035qw4ll440-valgrind-3.14.0-doc...
shrinking RPATHs of ELF executables and libraries in /nix/store/05x6hzqgqc32hnlgmgxjiklkgxb9v6av-valgrind-3.14.0-debug
patching script interpreter paths in /nix/store/05x6hzqgqc32hnlgmgxjiklkgxb9v6av-valgrind-3.14.0-debug
checking for references to /build/ in /nix/store/05x6hzqgqc32hnlgmgxjiklkgxb9v6av-valgrind-3.14.0-debug...
/nix/store/flgb5zfdc43kn1d8b3gli31gnvg0dndc-valgrind-3.14.0

@srhb srhb merged commit 9caab8f into NixOS:staging-next Nov 28, 2018
@srhb srhb deleted the valgrind-coregrind-makefile-race branch November 28, 2018 08:45
@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/cherry-picking-commits-to-upgrade-unstable-nixos-channels/1553/1

@hedning hedning mentioned this pull request Dec 4, 2018
10 tasks
hedning added a commit to hedning/nixpkgs that referenced this pull request Dec 4, 2018
NixOS#51107 added a patch to Makefile.am to fix
a race condition in the build. It was unfortunately never picked up as we we're
using the generated makefile from the tarball.
hedning added a commit to hedning/nixpkgs that referenced this pull request Dec 14, 2018
We fixed a race condition in NixOS#51505 and NixOS#51107. This required running
autoreconfHook to pick up the `coregrind-makefile-race.patch` patch.

Unfortunately this broke darwin's postPatch fixes as autoreconfHook would run
afterwards regenerating the fixed makefiles.

Moving the postPatch fixes to preConfigure should resolve the issue.

I left `postPatch = ""` in to avoid a rebuild on linux.
hedning added a commit to hedning/nixpkgs that referenced this pull request Dec 19, 2018
We fixed a race condition in NixOS#51505 and NixOS#51107. This required running
autoreconfHook to pick up the `coregrind-makefile-race.patch` patch.

Unfortunately this broke darwin's postPatch fixes as autoreconfHook would run
afterwards regenerating the fixed makefiles.

Moving the postPatch fixes to preConfigure should resolve the issue.

I left `postPatch = ""` in to avoid a rebuild on linux.

(cherry picked from commit a6d4a0c)
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

3 participants