| Message ID | 20171103160627.6468-3-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Per-package SDK and target directories | expand |
Thomas, All, Here is a summary of our discussion on IRC... On 2017-11-03 17:06 +0100, Thomas Petazzoni spake thusly: > Currently, our strategy for the RPATH of host binaries is as follows: > > - During the build, we force an absolute RPATH to be encoded into > binaries, by passing -Wl,-rpath,$(HOST_DIR)/lib. This ensures that > host binaries will find their libraries. > > - In the "make sdk" target, when preparing the SDK to be relocatable, > we use patchelf to replace those absolute RPATHs by relative RPATHs > that use $ORIGIN. > > Unfortunately, the use of absolute RPATH during the build is not > compatible with the move to per-package SDK, where a different host > directory is used for the build of each package. Therefore, we need to > move to a different strategy for RPATH handling. > > The new strategy is as follows: > > - During the build, we do not encode any RPATH into the host > binaries. Instead, we specify LD_LIBRARY_PATH environment to point > to the current HOST_DIR/lib directory (for the package being > currently built). In the past, we explicitly got rid of LD_LIBRARY_PATH because it does not work. See 34d081674a (core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment). Briefly, it does not work when we build a library that is also present on the distro, which is used by a tool from the distro, and we build it in an incompatibl way. So, we'll have to come up with another solution, like your suggested alternative. > - At the end of the build, in order to make sure that binaries are > usable without LD_LIBRARY_PATH, we fixup the host binaries to use > $ORIGIN/../lib. This is more-or-less what was done previously in > the "make sdk" target, except that instead of turning absolute > paths into relative paths in the RPATH, we are simply setting the > RPATH to $ORIGIN/../lib. > > In order to implement this, the fix-rpath script logic is adjusted > for the "host" case. > > An alternative strategy would have been to keep the > -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the > host binaries, and fix such RPATH at the end of the build of every > package. However, that would require calling fix-rpath after the > installation of every package, which is a bit expensive. I wonder how expensive that would be. It would be nice to time this. Also note that, even if the overhead is noticeable, this would be compensated by the mere fact that we are now doing a parallel build, so we would end up winning. Regards, Yann E. MORIN. > This change is independent from the per-package SDK functionality, and > could be applied separately. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > Makefile | 4 +++- > package/Makefile.in | 2 +- > package/pkg-generic.mk | 8 -------- > support/scripts/fix-rpath | 15 ++++++++++----- > 4 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/Makefile b/Makefile > index 79db7fe48a..5496273329 100644 > --- a/Makefile > +++ b/Makefile > @@ -231,6 +231,8 @@ LEGAL_MANIFEST_CSV_HOST = $(LEGAL_INFO_DIR)/host-manifest.csv > LEGAL_WARNINGS = $(LEGAL_INFO_DIR)/.warnings > LEGAL_REPORT = $(LEGAL_INFO_DIR)/README > > +export LD_LIBRARY_PATH = $(if $(PKG),$(HOST_DIR)/lib) > + > ################################################################################ > # > # staging and target directories do NOT list these as > @@ -558,7 +560,6 @@ world: target-post-image > .PHONY: sdk > sdk: world > @$(call MESSAGE,"Rendering the SDK relocatable") > - $(TOPDIR)/support/scripts/fix-rpath host > $(TOPDIR)/support/scripts/fix-rpath staging > $(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)/relocate-sdk.sh > echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location > @@ -679,6 +680,7 @@ $(TARGETS_ROOTFS): target-finalize > .PHONY: target-finalize > target-finalize: $(PACKAGES) > @$(call MESSAGE,"Finalizing target directory") > + $(TOPDIR)/support/scripts/fix-rpath host > $(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep)) > rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \ > $(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \ > diff --git a/package/Makefile.in b/package/Makefile.in > index a1a5316051..e94a75c230 100644 > --- a/package/Makefile.in > +++ b/package/Makefile.in > @@ -220,7 +220,7 @@ HOST_CPPFLAGS = -I$(HOST_DIR)/include > HOST_CFLAGS ?= -O2 > HOST_CFLAGS += $(HOST_CPPFLAGS) > HOST_CXXFLAGS += $(HOST_CFLAGS) > -HOST_LDFLAGS += -L$(HOST_DIR)/lib -Wl,-rpath,$(HOST_DIR)/lib > +HOST_LDFLAGS += -L$(HOST_DIR)/lib > > # The macros below are taken from linux 4.11 and adapted slightly. > # Copy more when needed. > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index cca94ba338..82f8c06821 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -105,14 +105,6 @@ endef > > GLOBAL_INSTRUMENTATION_HOOKS += check_bin_arch > > -# This hook checks that host packages that need libraries that we build > -# have a proper DT_RPATH or DT_RUNPATH tag > -define check_host_rpath > - $(if $(filter install-host,$(2)),\ > - $(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR))) > -endef > -GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath > - > define step_check_build_dir_one > if [ -d $(2) ]; then \ > printf "%s: installs files in %s\n" $(1) $(2) >&2; \ > diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath > index 15705a3b0d..eed751f5da 100755 > --- a/support/scripts/fix-rpath > +++ b/support/scripts/fix-rpath > @@ -61,7 +61,7 @@ main() { > local rootdir > local tree="${1}" > local find_args=( ) > - local sanitize_extra_args=( ) > + local sanitize_args=( ) > > if ! "${PATCHELF}" --version > /dev/null 2>&1; then > echo "Error: can't execute patchelf utility '${PATCHELF}'" > @@ -89,7 +89,8 @@ main() { > cp "${PATCHELF}" "${PATCHELF}.__to_be_patched" > > # we always want $ORIGIN-based rpaths to make it relocatable. > - sanitize_extra_args+=( "--relative-to-file" ) > + sanitize_args+=( "--set-rpath" ) > + sanitize_args+=( "\$ORIGIN/../lib" ) > ;; > > staging) > @@ -101,14 +102,18 @@ main() { > done > > # should be like for the target tree below > - sanitize_extra_args+=( "--no-standard-lib-dirs" ) > + sanitize_args+=( "--no-standard-lib-dirs" ) > + sanitize_args+=( "--make-rpath-relative" ) > + sanitize_args+=( "${rootdir}" ) > ;; > > target) > rootdir="${TARGET_DIR}" > # we don't want $ORIGIN-based rpaths but absolute paths without rootdir. > # we also want to remove rpaths pointing to /lib or /usr/lib. > - sanitize_extra_args+=( "--no-standard-lib-dirs" ) > + sanitize_args+=( "--no-standard-lib-dirs" ) > + sanitize_args+=( "--make-rpath-relative" ) > + sanitize_args+=( "${rootdir}" ) > ;; > > *) > @@ -125,7 +130,7 @@ main() { > # make files writable if necessary > changed=$(chmod -c u+w "${file}") > # call patchelf to sanitize the rpath > - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" > + ${PATCHELF} ${sanitize_args[@]} "${file}" > # restore the original permission > test "${changed}" != "" && chmod u-w "${file}" > fi > -- > 2.13.6 >
Hello, On Sun, 5 Nov 2017 09:57:56 +0100, Yann E. MORIN wrote: > In the past, we explicitly got rid of LD_LIBRARY_PATH because it does > not work. See 34d081674a (core/pkg-infrastructures: remove LD_LIBRARY_PATH > from the environment). > > Briefly, it does not work when we build a library that is also present > on the distro, which is used by a tool from the distro, and we build it > in an incompatibl way. > > So, we'll have to come up with another solution, like your suggested > alternative. > > > - At the end of the build, in order to make sure that binaries are > > usable without LD_LIBRARY_PATH, we fixup the host binaries to use > > $ORIGIN/../lib. This is more-or-less what was done previously in > > the "make sdk" target, except that instead of turning absolute > > paths into relative paths in the RPATH, we are simply setting the > > RPATH to $ORIGIN/../lib. > > > > In order to implement this, the fix-rpath script logic is adjusted > > for the "host" case. > > > > An alternative strategy would have been to keep the > > -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the > > host binaries, and fix such RPATH at the end of the build of every > > package. However, that would require calling fix-rpath after the > > installation of every package, which is a bit expensive. > > I wonder how expensive that would be. It would be nice to time this. > > Also note that, even if the overhead is noticeable, this would be > compensated by the mere fact that we are now doing a parallel build, so > we would end up winning. Thanks for the feedback. We already discussed this on IRC, and I agree. I'll have a look at implementing the solution of doing the patchelf adding $ORIGIN/../lib as an RPATH directly after the installation of each package. Thanks! Thomas
On 05-11-17 15:44, Thomas Petazzoni wrote: > Hello, > > On Sun, 5 Nov 2017 09:57:56 +0100, Yann E. MORIN wrote: > >> In the past, we explicitly got rid of LD_LIBRARY_PATH because it does >> not work. See 34d081674a (core/pkg-infrastructures: remove LD_LIBRARY_PATH >> from the environment). >> >> Briefly, it does not work when we build a library that is also present >> on the distro, which is used by a tool from the distro, and we build it >> in an incompatibl way. >> >> So, we'll have to come up with another solution, like your suggested >> alternative. >> >>> - At the end of the build, in order to make sure that binaries are >>> usable without LD_LIBRARY_PATH, we fixup the host binaries to use >>> $ORIGIN/../lib. This is more-or-less what was done previously in >>> the "make sdk" target, except that instead of turning absolute >>> paths into relative paths in the RPATH, we are simply setting the >>> RPATH to $ORIGIN/../lib. >>> >>> In order to implement this, the fix-rpath script logic is adjusted >>> for the "host" case. >>> >>> An alternative strategy would have been to keep the >>> -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the >>> host binaries, and fix such RPATH at the end of the build of every >>> package. However, that would require calling fix-rpath after the >>> installation of every package, which is a bit expensive. >> >> I wonder how expensive that would be. It would be nice to time this. >> >> Also note that, even if the overhead is noticeable, this would be >> compensated by the mere fact that we are now doing a parallel build, so >> we would end up winning. > > Thanks for the feedback. We already discussed this on IRC, and I agree. > I'll have a look at implementing the solution of doing the patchelf > adding $ORIGIN/../lib as an RPATH directly after the installation of > each package. Note that according to Wolfgang's measurements (if I remember correctly), doing fix-rpath on the host dir didn't take too long, but doing it in staging does. There aren't that many host files. Regards, Arnout
On 03-11-17 17:06, Thomas Petazzoni wrote: > Currently, our strategy for the RPATH of host binaries is as follows: > > - During the build, we force an absolute RPATH to be encoded into > binaries, by passing -Wl,-rpath,$(HOST_DIR)/lib. This ensures that > host binaries will find their libraries. > > - In the "make sdk" target, when preparing the SDK to be relocatable, > we use patchelf to replace those absolute RPATHs by relative RPATHs > that use $ORIGIN. > > Unfortunately, the use of absolute RPATH during the build is not > compatible with the move to per-package SDK, where a different host > directory is used for the build of each package. Therefore, we need to > move to a different strategy for RPATH handling. > > The new strategy is as follows: > > - During the build, we do not encode any RPATH into the host > binaries. Instead, we specify LD_LIBRARY_PATH environment to point > to the current HOST_DIR/lib directory (for the package being > currently built). > > - At the end of the build, in order to make sure that binaries are > usable without LD_LIBRARY_PATH, we fixup the host binaries to use > $ORIGIN/../lib. This is more-or-less what was done previously in > the "make sdk" target, except that instead of turning absolute > paths into relative paths in the RPATH, we are simply setting the > RPATH to $ORIGIN/../lib. > > In order to implement this, the fix-rpath script logic is adjusted > for the "host" case. > > An alternative strategy would have been to keep the > -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the > host binaries, and fix such RPATH at the end of the build of every > package. However, that would require calling fix-rpath after the > installation of every package, which is a bit expensive. As discussed, this would be the preferred approach after all. > > This change is independent from the per-package SDK functionality, and > could be applied separately. Shouldn't this type of comment be below the --- ? > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- [snip] > @@ -89,7 +89,8 @@ main() { > cp "${PATCHELF}" "${PATCHELF}.__to_be_patched" > > # we always want $ORIGIN-based rpaths to make it relocatable. > - sanitize_extra_args+=( "--relative-to-file" ) > + sanitize_args+=( "--set-rpath" ) > + sanitize_args+=( "\$ORIGIN/../lib" ) One advantage of --make-rpath-relative over --set-rpath is that the former will also remove rpaths that are not needed (which I guess is often the case for host tools). OTOH for host tools this is not really important. And it doesn't even save size, patchelf doesn't shift sections to the left (IIRC). But it does save a tiny amount of startup time - for gcc, this could have an impact on overall build time. Regards, Arnout [snip]
Hello, On Tue, 7 Nov 2017 22:15:38 +0100, Arnout Vandecappelle wrote: > > An alternative strategy would have been to keep the > > -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the > > host binaries, and fix such RPATH at the end of the build of every > > package. However, that would require calling fix-rpath after the > > installation of every package, which is a bit expensive. > > As discussed, this would be the preferred approach after all. Yes, agreed. > > This change is independent from the per-package SDK functionality, and > > could be applied separately. > > Shouldn't this type of comment be below the --- ? Well, it's not really that I expected the v1 of the patches to be merged :-) > > @@ -89,7 +89,8 @@ main() { > > cp "${PATCHELF}" "${PATCHELF}.__to_be_patched" > > > > # we always want $ORIGIN-based rpaths to make it relocatable. > > - sanitize_extra_args+=( "--relative-to-file" ) > > + sanitize_args+=( "--set-rpath" ) > > + sanitize_args+=( "\$ORIGIN/../lib" ) > > One advantage of --make-rpath-relative over --set-rpath is that the former will > also remove rpaths that are not needed (which I guess is often the case for host > tools). Well, --set-rpath sets the RPATH, so I believe it entirely removes any existing RPATH and replaces them all by just $ORIGIN/../lib. So I don't see how it could be more "efficient", since we're down to a single RPATH, which is the only rpath that is needed. Perhaps the only problem that I can think of is if some host binaries do have libraries installed in non-standard locations, and really need a custom absolute RPATH such as /home/foo/buildroot/output/per-package/host/lib/baz/ to be turned into $ORIGIN/../lib/baz/. However, I don't think we have such non-standard library locations for host packages, so I simply ignored this possible problem for now. It is a problem that can be fixed later on if needed by adjusting the host rpath mangling logic. Best regards, Thomas
Hello, On Sun, 5 Nov 2017 09:57:56 +0100, Yann E. MORIN wrote: > > An alternative strategy would have been to keep the > > -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the > > host binaries, and fix such RPATH at the end of the build of every > > package. However, that would require calling fix-rpath after the > > installation of every package, which is a bit expensive. > > I wonder how expensive that would be. It would be nice to time this. OK, to time this, I did a quick hacky shell script (http://code.bulix.org/uboevd-223829). What it does is: - Generate a mix of ELF files and random data files (half of the files are ELF files, half are random data files). This is the "generate" target of the script. - Fixup rpath to $ORIGIN/../lib if not already fixed. We don't want to fixup files repeatedly, as we don't want to duplicate files over and over by breaking hard links. So we really want to set the RPATH only once per binary. Results are as follows: $ ./rpath-evaluation.sh generate 0 500 ... generate 250 random data files, 250 ELF programs with a crappy rpath ... $ time ./rpath-evaluation.sh fixup real 0m0.976s user 0m0.546s sys 0m0.457s $ time ./rpath-evaluation.sh fixup real 0m0.610s user 0m0.393s sys 0m0.234s So the first fixup pass, which actually fixes the RPATH of 250 binaries takes about one second. The next fixup pass doesn't do anything, except checking that all binaries already have a fixed RPATH. So we see that it will take ~600ms to scan 500 files, provided half of them are ELF files, verify that they already have the correct RPATH. To me, this sounds very reasonable. What do you think? Thomas
On 07-11-17 22:22, Thomas Petazzoni wrote: > Hello, > > On Tue, 7 Nov 2017 22:15:38 +0100, Arnout Vandecappelle wrote: [snip] >> One advantage of --make-rpath-relative over --set-rpath is that the former will >> also remove rpaths that are not needed (which I guess is often the case for host >> tools). > > Well, --set-rpath sets the RPATH, so I believe it entirely removes any > existing RPATH and replaces them all by just $ORIGIN/../lib. So I don't > see how it could be more "efficient", since we're down to a single > RPATH, which is the only rpath that is needed. In many cases, no rpath is needed because it doesn't link with any Buildroot-built libraries. In that case, no rpath is slightly more efficient for application startup. > > Perhaps the only problem that I can think of is if some host binaries > do have libraries installed in non-standard locations, and really need > a custom absolute RPATH such > as /home/foo/buildroot/output/per-package/host/lib/baz/ to be turned > into $ORIGIN/../lib/baz/. However, I don't think we have such > non-standard library locations for host packages, so I simply ignored > this possible problem for now. It is a problem that can be fixed later > on if needed by adjusting the host rpath mangling logic. I've taken a quick look at what I have in my build dirs, and there is no .so file in another location except for dlopen()'d stuff. Regards, Arnout
On 07-11-17 23:41, Thomas Petazzoni wrote: > Hello, > > On Sun, 5 Nov 2017 09:57:56 +0100, Yann E. MORIN wrote: > >>> An alternative strategy would have been to keep the >>> -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the >>> host binaries, and fix such RPATH at the end of the build of every >>> package. However, that would require calling fix-rpath after the >>> installation of every package, which is a bit expensive. >> >> I wonder how expensive that would be. It would be nice to time this. > > OK, to time this, I did a quick hacky shell script > (http://code.bulix.org/uboevd-223829). What it does is: > > - Generate a mix of ELF files and random data files (half of the files > are ELF files, half are random data files). This is the "generate" > target of the script. > > - Fixup rpath to $ORIGIN/../lib if not already fixed. We don't want to > fixup files repeatedly, as we don't want to duplicate files over and > over by breaking hard links. So we really want to set the RPATH only > once per binary. > > Results are as follows: > > $ ./rpath-evaluation.sh generate 0 500 > ... generate 250 random data files, 250 ELF programs with a crappy > rpath ... > > $ time ./rpath-evaluation.sh fixup > > real 0m0.976s > user 0m0.546s > sys 0m0.457s > $ time ./rpath-evaluation.sh fixup > > real 0m0.610s > user 0m0.393s > sys 0m0.234s > > So the first fixup pass, which actually fixes the RPATH of 250 binaries > takes about one second. The next fixup pass doesn't do anything, except > checking that all binaries already have a fixed RPATH. > > So we see that it will take ~600ms to scan 500 files, provided half > of them are ELF files, verify that they already have the correct RPATH. > > To me, this sounds very reasonable. What do you think? 500 files is not a lot if you include staging as well, so I've repeated the experiment with 50000 files and on a HDD. First pass takes 3m34, second pass 3m43. Hm, perhaps I should have left my computer alone for a while during this experiment :-). Anyway, I think the bottom line is that the fixup script should avoid going over staging. But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway. Regards, Arnout
Hello, On Wed, 8 Nov 2017 00:50:15 +0100, Arnout Vandecappelle wrote: > > To me, this sounds very reasonable. What do you think? > > 500 files is not a lot if you include staging as well Why would I include staging? I don't see at all why fixing up RPATH in staging after each package build would be necessary. > so I've repeated the experiment with 50000 files and on a HDD. First > pass takes 3m34, second pass 3m43. Hm, perhaps I should have left my > computer alone for a while during this experiment :-). It's not very logical that the second pass is longer than the first pass, since the first pass does do patchelf invocations per file, while the second pass does only one patchelf invocation per file. > Anyway, I think the bottom line is that the fixup script should > avoid going over staging. Why would it need to go over staging? > But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway. Yes, that was my intent. If needed, we could optimize it by only doing the fixup at the end of a host package installation. But since we have a few target packages installing host stuff (toolchain, qt, etc.), it would require them to explicitly state that they need host rpath fixups. Perhaps it's easiest for now to just do the rpath fixup after every package installation. Best regards, Thomas
On 08-11-17 09:55, Thomas Petazzoni wrote: > Hello, > > On Wed, 8 Nov 2017 00:50:15 +0100, Arnout Vandecappelle wrote: > >>> To me, this sounds very reasonable. What do you think? >> >> 500 files is not a lot if you include staging as well > > Why would I include staging? I don't see at all why fixing up RPATH in > staging after each package build would be necessary. The simplest way would be to do find $(HOST_DIR) -type f but that includes staging. > >> so I've repeated the experiment with 50000 files and on a HDD. First >> pass takes 3m34, second pass 3m43. Hm, perhaps I should have left my >> computer alone for a while during this experiment :-). > > It's not very logical that the second pass is longer than the first > pass, since the first pass does do patchelf invocations per file, while > the second pass does only one patchelf invocation per file. > >> Anyway, I think the bottom line is that the fixup script should >> avoid going over staging. > > Why would it need to go over staging? > >> But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway. So this is not true, we also need $(HOST_DIR)/$(GNU_TARGET_NAME)/{bin,lib}. So then it's perhaps easier to just -prune staging. > Yes, that was my intent. If needed, we could optimize it by only doing > the fixup at the end of a host package installation. But since we have > a few target packages installing host stuff (toolchain, qt, etc.), it > would require them to explicitly state that they need host rpath > fixups. Perhaps it's easiest for now to just do the rpath fixup after > every package installation. After every package installation would be better, yes. Regards, Arnout > > Best regards, > > Thomas >
Hello, On Wed, 8 Nov 2017 11:55:19 +0100, Arnout Vandecappelle wrote: > >>> To me, this sounds very reasonable. What do you think? > >> > >> 500 files is not a lot if you include staging as well > > > > Why would I include staging? I don't see at all why fixing up RPATH in > > staging after each package build would be necessary. > > The simplest way would be to do find $(HOST_DIR) -type f but that includes staging. Hum, you didn't reply to my question, which was: "Why would we need to fixup RPATHs in STAGING_DIR" ? > >> But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway. > > So this is not true, we also need $(HOST_DIR)/$(GNU_TARGET_NAME)/{bin,lib}. > So then it's perhaps easier to just -prune staging. Absolutely. > > Yes, that was my intent. If needed, we could optimize it by only doing > > the fixup at the end of a host package installation. But since we have > > a few target packages installing host stuff (toolchain, qt, etc.), it > > would require them to explicitly state that they need host rpath > > fixups. Perhaps it's easiest for now to just do the rpath fixup after > > every package installation. > > After every package installation would be better, yes. ACK. Thomas
On 08-11-17 13:30, Thomas Petazzoni wrote: > Hello, > > On Wed, 8 Nov 2017 11:55:19 +0100, Arnout Vandecappelle wrote: > >>>>> To me, this sounds very reasonable. What do you think? >>>> >>>> 500 files is not a lot if you include staging as well >>> >>> Why would I include staging? I don't see at all why fixing up RPATH in >>> staging after each package build would be necessary. >> >> The simplest way would be to do find $(HOST_DIR) -type f but that includes staging. > > Hum, you didn't reply to my question, which was: "Why would we need to > fixup RPATHs in STAGING_DIR" ? We don't, obviously. However, if we naively go through HOST_DIR we will include STAGING_DIR. I thought that was obvious :-) Regards, Arnout > >>>> But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway. >> >> So this is not true, we also need $(HOST_DIR)/$(GNU_TARGET_NAME)/{bin,lib}. >> So then it's perhaps easier to just -prune staging. > > Absolutely. > >>> Yes, that was my intent. If needed, we could optimize it by only doing >>> the fixup at the end of a host package installation. But since we have >>> a few target packages installing host stuff (toolchain, qt, etc.), it >>> would require them to explicitly state that they need host rpath >>> fixups. Perhaps it's easiest for now to just do the rpath fixup after >>> every package installation. >> >> After every package installation would be better, yes. > > ACK. > > Thomas >
diff --git a/Makefile b/Makefile index 79db7fe48a..5496273329 100644 --- a/Makefile +++ b/Makefile @@ -231,6 +231,8 @@ LEGAL_MANIFEST_CSV_HOST = $(LEGAL_INFO_DIR)/host-manifest.csv LEGAL_WARNINGS = $(LEGAL_INFO_DIR)/.warnings LEGAL_REPORT = $(LEGAL_INFO_DIR)/README +export LD_LIBRARY_PATH = $(if $(PKG),$(HOST_DIR)/lib) + ################################################################################ # # staging and target directories do NOT list these as @@ -558,7 +560,6 @@ world: target-post-image .PHONY: sdk sdk: world @$(call MESSAGE,"Rendering the SDK relocatable") - $(TOPDIR)/support/scripts/fix-rpath host $(TOPDIR)/support/scripts/fix-rpath staging $(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)/relocate-sdk.sh echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location @@ -679,6 +680,7 @@ $(TARGETS_ROOTFS): target-finalize .PHONY: target-finalize target-finalize: $(PACKAGES) @$(call MESSAGE,"Finalizing target directory") + $(TOPDIR)/support/scripts/fix-rpath host $(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep)) rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \ $(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \ diff --git a/package/Makefile.in b/package/Makefile.in index a1a5316051..e94a75c230 100644 --- a/package/Makefile.in +++ b/package/Makefile.in @@ -220,7 +220,7 @@ HOST_CPPFLAGS = -I$(HOST_DIR)/include HOST_CFLAGS ?= -O2 HOST_CFLAGS += $(HOST_CPPFLAGS) HOST_CXXFLAGS += $(HOST_CFLAGS) -HOST_LDFLAGS += -L$(HOST_DIR)/lib -Wl,-rpath,$(HOST_DIR)/lib +HOST_LDFLAGS += -L$(HOST_DIR)/lib # The macros below are taken from linux 4.11 and adapted slightly. # Copy more when needed. diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index cca94ba338..82f8c06821 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -105,14 +105,6 @@ endef GLOBAL_INSTRUMENTATION_HOOKS += check_bin_arch -# This hook checks that host packages that need libraries that we build -# have a proper DT_RPATH or DT_RUNPATH tag -define check_host_rpath - $(if $(filter install-host,$(2)),\ - $(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR))) -endef -GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath - define step_check_build_dir_one if [ -d $(2) ]; then \ printf "%s: installs files in %s\n" $(1) $(2) >&2; \ diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath index 15705a3b0d..eed751f5da 100755 --- a/support/scripts/fix-rpath +++ b/support/scripts/fix-rpath @@ -61,7 +61,7 @@ main() { local rootdir local tree="${1}" local find_args=( ) - local sanitize_extra_args=( ) + local sanitize_args=( ) if ! "${PATCHELF}" --version > /dev/null 2>&1; then echo "Error: can't execute patchelf utility '${PATCHELF}'" @@ -89,7 +89,8 @@ main() { cp "${PATCHELF}" "${PATCHELF}.__to_be_patched" # we always want $ORIGIN-based rpaths to make it relocatable. - sanitize_extra_args+=( "--relative-to-file" ) + sanitize_args+=( "--set-rpath" ) + sanitize_args+=( "\$ORIGIN/../lib" ) ;; staging) @@ -101,14 +102,18 @@ main() { done # should be like for the target tree below - sanitize_extra_args+=( "--no-standard-lib-dirs" ) + sanitize_args+=( "--no-standard-lib-dirs" ) + sanitize_args+=( "--make-rpath-relative" ) + sanitize_args+=( "${rootdir}" ) ;; target) rootdir="${TARGET_DIR}" # we don't want $ORIGIN-based rpaths but absolute paths without rootdir. # we also want to remove rpaths pointing to /lib or /usr/lib. - sanitize_extra_args+=( "--no-standard-lib-dirs" ) + sanitize_args+=( "--no-standard-lib-dirs" ) + sanitize_args+=( "--make-rpath-relative" ) + sanitize_args+=( "${rootdir}" ) ;; *) @@ -125,7 +130,7 @@ main() { # make files writable if necessary changed=$(chmod -c u+w "${file}") # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + ${PATCHELF} ${sanitize_args[@]} "${file}" # restore the original permission test "${changed}" != "" && chmod u-w "${file}" fi
Currently, our strategy for the RPATH of host binaries is as follows: - During the build, we force an absolute RPATH to be encoded into binaries, by passing -Wl,-rpath,$(HOST_DIR)/lib. This ensures that host binaries will find their libraries. - In the "make sdk" target, when preparing the SDK to be relocatable, we use patchelf to replace those absolute RPATHs by relative RPATHs that use $ORIGIN. Unfortunately, the use of absolute RPATH during the build is not compatible with the move to per-package SDK, where a different host directory is used for the build of each package. Therefore, we need to move to a different strategy for RPATH handling. The new strategy is as follows: - During the build, we do not encode any RPATH into the host binaries. Instead, we specify LD_LIBRARY_PATH environment to point to the current HOST_DIR/lib directory (for the package being currently built). - At the end of the build, in order to make sure that binaries are usable without LD_LIBRARY_PATH, we fixup the host binaries to use $ORIGIN/../lib. This is more-or-less what was done previously in the "make sdk" target, except that instead of turning absolute paths into relative paths in the RPATH, we are simply setting the RPATH to $ORIGIN/../lib. In order to implement this, the fix-rpath script logic is adjusted for the "host" case. An alternative strategy would have been to keep the -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the host binaries, and fix such RPATH at the end of the build of every package. However, that would require calling fix-rpath after the installation of every package, which is a bit expensive. This change is independent from the per-package SDK functionality, and could be applied separately. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- Makefile | 4 +++- package/Makefile.in | 2 +- package/pkg-generic.mk | 8 -------- support/scripts/fix-rpath | 15 ++++++++++----- 4 files changed, 14 insertions(+), 15 deletions(-)