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
abi-compliance-checker: init at 2.3 #47408
Conversation
@GrahamcOfBorg build abi-compliance-checker |
Success on x86_64-linux (full log) Attempted: abi-compliance-checker Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: abi-compliance-checker Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: abi-compliance-checker, abi-dumper, vtable-dumper Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: abi-compliance-checker, abi-dumper, vtable-dumper Partial log (click to expand)
|
installPhase = '' | ||
make prefix=$out install | ||
''; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add makeFlags = [ "prefix=$(out)" ];
and delete the phases? (all
is the default target of vtable-dumper.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in all 3 packages (along with the meta improvement). Looks a lot nicer now and I learned a new trick with the makeFlags
; thanks @orivej!
abi-dumper
still needs a preBuild = "mkdir -p $out";
, since the Makefile
shells out to perl
to do its install and will fail if the install dir does not exist, but for the other ones I could delete everything and make it much cleaner.
make prefix=$out install | ||
''; | ||
|
||
meta = with stdenv; { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally write meta = with stdenv.lib;
.
@@ -7924,6 +7924,8 @@ with pkgs; | |||
|
|||
abi-dumper = callPackage ../development/tools/misc/abi-dumper { }; | |||
|
|||
abi-compliance-checker = callPackage ../development/tools/misc/abi-compliance-checker { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you put this before abi-dumper
to keep them sorted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah thanks for the catch; not sure how I missed that (I'm usually very OCD about sorting).
5fb609b
to
3aaa8ad
Compare
Success on aarch64-linux (full log) Attempted: abi-compliance-checker, abi-dumper, vtable-dumper Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: abi-compliance-checker, abi-dumper, vtable-dumper Partial log (click to expand)
|
Ping? Happy to iterate further, but AFAIK this is good to go now. |
could you fixup the code review changes into their respective commits? ie:
|
Can-do; I cleaned up the commit history and rebased onto the latest origin/master. |
Success on aarch64-linux (full log) Attempted: abi-compliance-checker, abi-dumper, vtable-dumper Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: abi-compliance-checker, abi-dumper, vtable-dumper Partial log (click to expand)
|
Ping @orivej mind taking a look? Should be ready to merge I think. |
CC @matthewbauer would you mind reviewing please? |
Thanks! |
Motivation for this change
This adds
abi-compliance-checker
and its two dependencies, for dumping the API/ABI of C/C++ libraries. It is the tool used to produce the reports on this webpage:https://abi-laboratory.pro/
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)