eudev-project / eudev

Repository for eudev development
GNU General Public License v2.0
527 stars 144 forks source link

udevadm: prepend $(DESTDIR) for symlink target #165

Closed JoelsonCarl closed 5 years ago

blueness commented 5 years ago

This looks good. Was it causing a problem?

JoelsonCarl commented 5 years ago

It was an issue in buildroot (not sure if you're at all familiar with that). buildroot has the capability to generate an SDK. There was a patch set submitted recently (https://patchwork.ozlabs.org/project/buildroot/list/?series=80485) to fix up a bunch of symlinks in the folder it generates the SDK from, and part of that is it will error out if any of the symlinks are pointing outside of the folder it is essentially tarring up as the SDK. With the lack of $(DESTDIR), the symlink was pointing to /usr/bin/udevadm instead of /actual/install/path/to/the/built/usr/bin/udevadm and thus errors out.

blueness commented 5 years ago

I have used buildroot in the past. That's exactly what I was wondering --- in what context did the issue arise. Thanks.

yann-morin-1998 commented 5 years ago

I think this is wrong, because at runtime, the saymlink will be indeed pointing to $(DESTDIR)/$(bindir)/udevadm but $(DESTDIR) would not exist at runtime.

It happens to fix the problem for the Buildroot SDK, but is the wrong solution. It should instead be made into a relative symlink.

fjaell commented 5 years ago

I second yann-morin-1998, the suggested fix is wrong because it creates a dangling symlink

blueness commented 5 years ago

@yann-morin-1998 do you want to suggest a fix?

blueness commented 5 years ago

I've confirmed the that this is a problem, so I've reverted. If anyone wants to suggest another solution, I'll test it and commit if it doesn't break stuff.

yann-morin-1998 commented 5 years ago

@blueness The only generic solution I see, would be to create symlinks.

However, that requires a ln that is recent enough to support the -r option, which was introduced in 2012 (coreutils v8.16). That is old, but not old enough to be widely available. For example, some entreprise-grade distros (RHEL5, in extended support, or RHEL6, still supported) would ship older coreutils.

So buildsystems (such as Buildroot) only have the option of building their own local version of coreutils, which is pretty long just to get a ln that has the -r option (using busybox with a trimmed-down config would have been pretty fast, but busybox' ln does not have the -r option.)

In this particular case, keeping absolute links is OK. In Buildroot, I have taken a different approach that just ignores that issue (we don't care about that particular situation, in fact).

blueness commented 5 years ago

@yann-morin-1998 Thanks for the update.