| Message ID | 20181207035052.316-1-JoelsonCarl@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] core/sdk: don't mangle symlinks with '.' or '..' at start | expand |
Joel, All, On 2018-12-06 20:50 -0700, Joel Carlson spake thusly: > The previous transform of changing any '.' at the start of a line to > $(BR2_SDK_PREFIX) mangles any symlinks with a relative path starting > with '.' or '..' in the host folder, as --transform operates on the link > target as opposed to the link name. > > You might end up with something like: > $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar -> > $(BR2_SDK_PREFIX)./opt/ext-toolchain/bin/aarch64-linux-gnu-ar Dang right... :-/ > when it should be: > $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar -> > ../opt/ext-toolchain/bin/aarch64-linux-gnu-ar > > Instead, don't change to HOST_DIR when creating the tarball, and pass > the path to HOST_DIR. Tar will strip the leading / from member names, > so then transform that path (HOST_DIR with leading / removed) to > BR2_SDK_PREFIX. > > However hardlinks are still linking to HOST_DIR, so do an additional > transform of $(HOST_DIR) (keeping the leading slash) to BR2_SDK_PREFIX > in order to catch those. > > Signed-off-by: Joel Carlson <JoelsonCarl@gmail.com> > --- > If accepted, this should probably get put on 2018.11.x. > --- > Makefile | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 37df98520e..668c3e09e0 100644 > --- a/Makefile > +++ b/Makefile > @@ -591,6 +591,7 @@ prepare-sdk: world > echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location > > BR2_SDK_PREFIX ?= $(GNU_TARGET_NAME)_sdk-buildroot > +HOST_DIR_NO_LEAD_SLASH=$(shell echo $(HOST_DIR) | tail -c +2) > .PHONY: sdk > sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) > @$(call MESSAGE,"Generating SDK tarball") > @@ -598,8 +599,8 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) > $(Q)mkdir -p $(BINARIES_DIR) > $(TAR) czf "$(BINARIES_DIR)/$(BR2_SDK_PREFIX).tar.gz" \ > --owner=0 --group=0 --numeric-owner \ > - --transform='s#^\.#$(BR2_SDK_PREFIX)#' \ > - -C $(HOST_DIR) "." > + --transform='s#^$(HOST_DIR_NO_LEAD_SLASH)#$(BR2_SDK_PREFIX)#' \ > + --transform='s#^$(HOST_DIR)#$(BR2_SDK_PREFIX)#' $(HOST_DIR) Well, I am not too fond of this new HOST_DIR_NO_LEAD_SLASH variable, since it is used in a single location. And calling to the shell is not nice either. What about replaciong that with just: $(patsubst /%,%,$(HOST_DIR)) Now, this fixup is not enough, in fact, because thisleaves symlinks to absolute paths borked, e.g.: /path/to/host/bin/foo -> /path/to/host/bin/bar is turned into: /path/to/new/host/bin/foo -> sdkprefix/bin/bar which obviously is incorrect. So we need more work in this area, and the first thing we should do is to transform absolute symlinks into relative ones, to be done in the prepare-sdk rule, I suppose... Regards, Yann E. MORIN. > RSYNC_VCS_EXCLUSIONS = \ > --exclude .svn --exclude .git --exclude .hg --exclude .bzr \ > -- > 2.17.1 >
On Fri, Dec 7, 2018 at 4:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote: <snip> > > --- > > Makefile | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 37df98520e..668c3e09e0 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -591,6 +591,7 @@ prepare-sdk: world > > echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location > > > > BR2_SDK_PREFIX ?= $(GNU_TARGET_NAME)_sdk-buildroot > > +HOST_DIR_NO_LEAD_SLASH=$(shell echo $(HOST_DIR) | tail -c +2) > > .PHONY: sdk > > sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) > > @$(call MESSAGE,"Generating SDK tarball") > > @@ -598,8 +599,8 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) > > $(Q)mkdir -p $(BINARIES_DIR) > > $(TAR) czf "$(BINARIES_DIR)/$(BR2_SDK_PREFIX).tar.gz" \ > > --owner=0 --group=0 --numeric-owner \ > > - --transform='s#^\.#$(BR2_SDK_PREFIX)#' \ > > - -C $(HOST_DIR) "." > > + --transform='s#^$(HOST_DIR_NO_LEAD_SLASH)#$(BR2_SDK_PREFIX)#' \ > > + --transform='s#^$(HOST_DIR)#$(BR2_SDK_PREFIX)#' $(HOST_DIR) > > Well, I am not too fond of this new HOST_DIR_NO_LEAD_SLASH variable, > since it is used in a single location. And calling to the shell is not > nice either. > > What about replaciong that with just: > > $(patsubst /%,%,$(HOST_DIR)) > I didn't read the documentation on patsubst close enough and didn't realize you could make it replace only the first occurrence. That suggestion works and I'll use it. > Now, this fixup is not enough, in fact, because thisleaves symlinks to > absolute paths borked, e.g.: > > /path/to/host/bin/foo -> /path/to/host/bin/bar > > is turned into: > > /path/to/new/host/bin/foo -> sdkprefix/bin/bar > > which obviously is incorrect. > > So we need more work in this area, and the first thing we should do is > to transform absolute symlinks into relative ones, to be done in the > prepare-sdk rule, I suppose... Symlinks with an absolute path is not a case I had considered. I will work this further and submit a v2 patch. Thanks!
Joel, All, On 2018-12-07 11:08 -0700, Joel Carlson spake thusly: > On Fri, Dec 7, 2018 at 4:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote: [--SNIP--] > > $(patsubst /%,%,$(HOST_DIR)) > I didn't read the documentation on patsubst close enough and didn't > realize you could make it replace only the first occurrence. That > suggestion works and I'll use it. > > > Now, this fixup is not enough, in fact, because thisleaves symlinks to > > absolute paths borked, e.g.: [--SNIP--] > Symlinks with an absolute path is not a case I had considered. > I will work this further and submit a v2 patch. Thanks! Note that I al ready sent a v2 with your patch ammended, and the absolute symlinks handled: https://patchwork.ozlabs.org/project/buildroot/list/?series=80485 Regards, Yann E. MORIN.
diff --git a/Makefile b/Makefile index 37df98520e..668c3e09e0 100644 --- a/Makefile +++ b/Makefile @@ -591,6 +591,7 @@ prepare-sdk: world echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location BR2_SDK_PREFIX ?= $(GNU_TARGET_NAME)_sdk-buildroot +HOST_DIR_NO_LEAD_SLASH=$(shell echo $(HOST_DIR) | tail -c +2) .PHONY: sdk sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) @$(call MESSAGE,"Generating SDK tarball") @@ -598,8 +599,8 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) $(Q)mkdir -p $(BINARIES_DIR) $(TAR) czf "$(BINARIES_DIR)/$(BR2_SDK_PREFIX).tar.gz" \ --owner=0 --group=0 --numeric-owner \ - --transform='s#^\.#$(BR2_SDK_PREFIX)#' \ - -C $(HOST_DIR) "." + --transform='s#^$(HOST_DIR_NO_LEAD_SLASH)#$(BR2_SDK_PREFIX)#' \ + --transform='s#^$(HOST_DIR)#$(BR2_SDK_PREFIX)#' $(HOST_DIR) RSYNC_VCS_EXCLUSIONS = \ --exclude .svn --exclude .git --exclude .hg --exclude .bzr \
The previous transform of changing any '.' at the start of a line to $(BR2_SDK_PREFIX) mangles any symlinks with a relative path starting with '.' or '..' in the host folder, as --transform operates on the link target as opposed to the link name. You might end up with something like: $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar -> $(BR2_SDK_PREFIX)./opt/ext-toolchain/bin/aarch64-linux-gnu-ar when it should be: $(BR2_SDK_PREFIX)/bin/aarch64-linux-gnu-ar -> ../opt/ext-toolchain/bin/aarch64-linux-gnu-ar Instead, don't change to HOST_DIR when creating the tarball, and pass the path to HOST_DIR. Tar will strip the leading / from member names, so then transform that path (HOST_DIR with leading / removed) to BR2_SDK_PREFIX. However hardlinks are still linking to HOST_DIR, so do an additional transform of $(HOST_DIR) (keeping the leading slash) to BR2_SDK_PREFIX in order to catch those. Signed-off-by: Joel Carlson <JoelsonCarl@gmail.com> --- If accepted, this should probably get put on 2018.11.x. --- Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)