Comparing changes
Open a pull request
base repository: NixOS/nixpkgs
base: c86fd3ed13f5
head repository: NixOS/nixpkgs
compare: e1deb56b101e
- 11 commits
- 2 files changed
- 3 contributors
Commits on Feb 10, 2019
-
autoPatchelfHook: do not patch statically linked files
Also speed up quite significantly due less forking. (cherry picked from commit 58a97df)
-
autoPatchelfHook: Only check PT_INTERP on execs
If the ELF file is not an executable, we do not get a PT_INTERP section, because after all, it's a *shared* library. So instead of checking for PT_INTERP (to avoid statically linked executables) for all ELF files, we add another check to see if it's an executable and *only* skip it when it is and there's no PT_INTERP. Signed-off-by: aszlig <aszlig@nix.build> (cherry picked from commit 9920215)
-
autoPatchelfHook: Silence errors in isExecutable
The "maxx" package recursively runs isExecutable on a bunch of files and since the change to use "readelf" instead of "file" a lot of errors like this one are printed during build: readelf: Error: Not an ELF file - it has the wrong magic bytes at the start While the isExecutable was never meant to be used outside of the autoPatchelfHook, it's still a good idea to silence the errors because whenever readelf fails, it clearly indicates that the file in question is not a valid ELF file. Signed-off-by: aszlig <aszlig@nix.build> (cherry picked from commit b452604)
-
autoPatchelfHook: Correctly detect PIE binaries
I originally thought it would just be enough to just check for an INTERP section in isExecutable, however this would mean that we don't detect statically linked ELF files, which would break our recent improvement to gracefully handle those. In theory, we are only interested in ELF files that have an INTERP section, so checking for INTERP would be enough. Unfortunately the isExecutable function is already used outside of autoPatchelfHook, so we can't easily get rid of it now, so let's actually strive for more correctness and make isExecutable actually match ELF files that are executable. So what we're doing instead now is to check whether either the ELF type is EXEC *or* we have an INTERP section and if one of them is true we should have an ELF executable, even if it's statically linked. Along the way I also set LANG=C for the invocations of readelf, just to be sure we don't get locale-dependent output. Tested this with the following command (which contains almost[1] all the packages using autoPatchelfHook), checking whether we run into any library-related errors: nix-build -E 'with import ./. { config.allowUnfree = true; }; runCommand "test-executables" { drvs = [ anydesk cups-kyodialog3 elasticsearch franz gurobi masterpdfeditor oracle-instantclient powershell reaper sourcetrail teamviewer unixODBCDrivers.msodbcsql17 virtlyst vk-messenger wavebox zoom-us ]; } ("for i in $drvs; do for b in $i/bin/*; do " + "[ -x \"$b\" ] && timeout 10 \"$b\" || :; done; done") ' Apart from testing against library-related errors I also compared the resulting store paths against the ones prior to this commit. Only anydesk and virtlyst had the same as they didn't have self-references, everything else differed only because of self-references, except elasticsearch, which had the following PIE binaries: * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/autoconfig * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/autodetect * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/categorize * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/controller * modules/x-pack/x-pack-ml/platform/linux-x86_64/bin/normalize These binaries were now patched, which is what this commit is all about. [1]: I didn't include the "maxx" package (MaXX Interactive Desktop) because the upstream URLs are no longer existing and I couldn't find them elsewhere on the web. Signed-off-by: aszlig <aszlig@nix.build> Fixes: #48330 Cc: @gnidorah (for MaXX Interactive Desktop) (cherry picked from commit c64624b)
-
autoPatchelfHook: Make easier to run autoPatchelf
The autoPatchelf main function which is run against all of the outputs was pretty much tailored towards this specific setup-hook and was relying on $prefix to be set globally. So if you wanted to run autoPatchelf manually - let's say during buildPhase - you would have needed to run it like this: prefix=/some/directory autoPatchelf This is now more intuitive and all you need to do is run the following: autoPatchelf /some/directory Signed-off-by: aszlig <aszlig@nix.build> (cherry picked from commit d03e4ff)
-
autoPatchelfHook: Allow to prevent automatic run
If you want to only run autoPatchelf on a specific path and leave everything else alone, we now have a $dontAutoPatchelf environment variable, which causes the postFixup hook to not run at all. The name "dontAutoPatchelf" probably is a bit weird in conjunction with putting "autoPatchelfHook" in nativeBuildInputs, but unless someone comes up with a better name I keep it that way because it's consistent with all the other dontStrip, dontPatchShebangs, dontPatchELF and whatnot. A specific example where this is needed is when building the Android SDK emulator, which contains a few ARM binaries in subdirectories that should not be patched. If we were to run autoPatchelf on all outputs unconditionally we'd run into errors because some ARM libraries couldn't be found. Signed-off-by: aszlig <aszlig@nix.build> (cherry picked from commit e4fbb24)
-
autoPatchelfHook: Add --no-recurse flag
This is to be used with the autoPatchelf command and allows to only patch a specific file or directory without recursing into subdirectories. Apart from being able to run the command in a standalone way, as detailled in the previous commit this is also needed for the Android SDK emulator, because according to @svanderburg there are subdirectories we don't want to patch. The reason why I didn't use GNU getopt is that it might not be available on all operating systems and the getopts bash builtin doesn't support long arguments. Apart from that, the implementation for recognizing the flag is pretty trivial and it's also using bash builtins only, so if we want to do something really fancy someday, we can still change it. Signed-off-by: aszlig <aszlig@nix.build> (cherry picked from commit 3ca35ce)
-
autoPatchelfHook: Add addAutoPatchelfSearchPath
This function is useful if autoPatchelf is invoked during some of the phases of a build and allows to add arbitrary shared objects to the search path. So far the same functionality was in autoPatchelf itself, but not available as a separate function, so when adding shared objects to the dependency cache one would have to do so manually. The function also has the --no-recurse flag, which prevents recursing into subdirectories. Signed-off-by: aszlig <aszlig@nix.build> (cherry picked from commit 2faf905)
-
autoPatchelfHook: Fix type of norecurse variable
While declaring it as an array doesn't do any harm in our usage, it might be a bit confusing when reading the code. Signed-off-by: aszlig <aszlig@nix.build> (cherry picked from commit 9f23a63)
-
autoPatchelfHook: Skip on missing segment headers
If the file in question is not a shared object file but an ELF, we really want to skip the file, because we won't have anything to patch there. For example if the file is created via "gcc -c -o foo.o foo.c", we don't get a segment header and so far autoPatchelf was trying to patch such a file. By checking for missing segment headers, we're now no longer going to attempt patching such a file. Signed-off-by: aszlig <aszlig@nix.build> Reported-by: Sander van der Burg <svanderburg@gmail.com> (cherry picked from commit 4a6e3e4)
Commits on Feb 13, 2019
-
Merge pull request #55538 from worldofpeace/autopatchelf-backport
backport changes to autoPatchelfHook
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff c86fd3ed13f5...e1deb56b101e