core/sdk: fix relative symlinks in generated tarball (branch yem/sdk)

Hello All!

This two-patch series is my counter-proposal to Joel's path to try and fix the issue:

    https://patchwork.ozlabs.org/patch/1009168/

The first patch fixes the absolute symlinks, that were so far not accounted for at all. A new helper scripts changes them into relative symlinks.

The second patch is the actual fix to the issue Joel's patch was trying to solve. I believe it is a better and simpler solution, although I have to admit the patsubst trickery is not obvious.

Regards,
Yann E. MORIN.

The following changes since commit ee58353462f32f46f3f5b93f6dc78e94b31bad78

DEVELOPERS: remove Vicente Olivert Riera (2018-12-22 08:10:03 +0100)

are available in the git repository at:

git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to 235301274e7a921bb0b98aa606403b12fbee08be

core/sdk: don't mangle symlinks with '.' or '..' at start (2018-12-22 13:55:04 +0100)

----------------------------------------------------------------
Joel Carlson (1):
core/sdk: don't mangle symlinks with '.' or '..' at start

Yann E. MORIN (1):
core: make symlinks relative when preparing the SDK

 Makefile                     |  5 ++--
 support/scripts/fix-symlinks | 71 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100755 support/scripts/fix-symlinks

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

The SDK is supposed to be relocatable, so symlinks must not be absolute.

Add a new helper script, that replaces all absolute symlinks with relative ones.

The ideal solution would use the shortest relative path for the destination, but it is non-trivial to come up with. We just ensure the relative path does not cross a common base directory, and compute all the paths relative to that anchor.

This can give non-optimum relative symlinks, like:

    /base-dir/bin/foo -> ../bin/bar

when the optimum would have been:

    /base-dir/bin/foo -> bar

Finally, as a sanity check, ensure there is not relative symlink pointing out of the base directory, because their targets would be missing from the SDK.

We do one exception to that last rule: any symlink in a bin directory of staging, that point s to a bin directory of the host, is simply ignored. They are supposed to be validate on the target, and so are supposed to point to target executables, so we are not going to execute them anyway (we're still going to execute the foo-config scripts, but they are not symlinks, so we're OK).

---
Changes v2 -> v3:
- account for absolute symlinks in staging's bin dirs (Andreas)

---
 Makefile                     |  1 +
 support/scripts/fix-symlinks | 71 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)
 create mode 100755 support/scripts/fix-symlinks

index c5b78b3274..e1b58e7bc3 100644
@@ -586,6 +586,7 @@ prepare-sdk: world
 	@$(call MESSAGE,"Rendering the SDK relocatable")
 	$(TOPDIR)/support/scripts/fix-rpath host
 	$(TOPDIR)/support/scripts/fix-rpath staging
+	$(TOPDIR)/support/scripts/fix-symlinks
 	$(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)/relocate-sdk.sh
 	mkdir -p $(HOST_DIR)/share/buildroot
 	echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location

2.14.1

Hello,

This script is really not trivial. This additional complexity raises a simple question: how many packages install such absolute symlinks? Should we instead detect such absolute symlinks, and use that to fix the problematic packages ?

I am not saying at all that the approach of fixing all symlinks is wrong, but I'd like to see a proper discussion on the balance between fixing the problematic packages vs. taking the "brutal" approach of this new fix-symlinks script.

Best regards,

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

That was actually a good question. I did a build where I turned on some 450+ random packages (estimated by counting the folders in build/ other than host-* folders). I encountered the following absolute links:
iptables - 1 absolute symlink (sysroot/usr/bin/iptables-xml -> /usr/sbin/xtables-multi)
ntfs-3g - 2 absolute symlinks (sysroot/sbin/mount.ntfs-3g -> /usr/bin/ntfs-3g and sysroot/sbin/mount.lowntfs-3g -> /usr/bin/lowntfs-3g)
fontconfig - 16 absolute symlinks (sysroot/etc/fonts/conf.d/* -> /usr/share/fontconfig/conf.avail/*)
And then not in my build, but pointed out by someone on a previous email thread, the eudev package produces 1 absolute symlink

I could dump my actual config somewhere if desired.

So with such a small amount of absolute symlinks so far, and all of them being for things we generally don't care about in the SDK, then perhaps just check for absolute symlinks in a whitelist of areas like /lib (or perhaps blacklist areas like /etc or /usr/bin). Error out on an absolute symlink in the spots we care about, and fix that upstream?

The thought behind the whitelist/blacklist as I understand it is because I tried fixing iptables and eudev, but did it incorrectly. The discussion on my eudev change [1] makes it sound a bit harder to solve (relies on ln command supporting "-r" which may not be everywhere), but being a link we don't care about in the SDK, we could just ignore it with a whitelist/blacklist.

[1] https://github.com/gentoo/eudev/pull/165

Hi,

Am 05.01.19 um 01:53 schrieb Joel Carlson:

I've been toying with this patch as well and among ~100 packages also found the absolute symlinks in iptables and eudev (only). Short of time and knowledge of how to suppress them being created I made two patches in the form:

+define EUDEV_REMOVE_BROKEN_SYMLINK
+ $(RM) $(STAGING_DIR)/sbin/udevadm
+endef
+
+EUDEV_POST_INSTALL_STAGING_HOOKS += EUDEV_REMOVE_BROKEN_SYMLINK +

However, I don't propose this as a solution as I think a blacklist would concentrate the issue in one place rather than spreading exceptions all over the place.

regards,
Andreas

From: Joel Carlson <[email protected]>

The current transform changes any '.' at the start of a filename to $(BR2_SDK_PREFIX). This also applies to the target of a symlink, when it is relative.

We thus 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

We fix that by making sure we always remove a known prefix, i.e. we remove the path to host dir. The obvious solution would be to cd into $(HOST_DIR)/.. , then tar ./host/ and finally use a --transfrom pattern as 's,^\./$(notdir $(HOST_DIR)),$(BR2_SDK_PREFIX)'.

Since $(HOST_DIR) can point to a user-supplied location, we don't know very well how the pattern may patch.

Instead, we cd into / and tar the full path to $(HOST_DIR).

Since tar removes any leading '/', it would spurr a warning message, which is annoying. So we explicitly remove the leading '/' from $(HOST_DIR) when we tar it.

Finally, we transform all filenames to replace a leading $(HOST_DIR) (without a leading /) to the prefix to use.

[yann.morin.1998 at free.fr:
- use a single transform pattern
- use full HOST_DIR path as pattern to replace
- update commit log accordingly
]

---
Changes v1 -> v2: (Yann)
- changes in the commit log, above.

---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index e1b58e7bc3..8fc7984006 100644
@@ -599,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#^$(patsubst /%,%,$(HOST_DIR))#$(BR2_SDK_PREFIX)#' \
+		-C / $(patsubst /%,%,$(HOST_DIR))
 
 RSYNC_VCS_EXCLUSIONS = \
 	--exclude .svn --exclude .git --exclude .hg --exclude .bzr \

2.14.1

Applied to master, thanks.

Regards,
Arnout

Hi,

Committed to 2018.11.x, thanks.

Bye, Peter Korsgaard