Bug 229208

Summary: `install -l rs` (relative symlink to source) into directory creates invalid symlink
Product: Base System Reporter: Andreas Sommer <andreas.sommer87>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: New ---    
Severity: Affects Some People CC: bapt, emaste, kevans
Priority: ---    
Version: 11.1-RELEASE   
Hardware: Any   
OS: Any   
Description Flags
Unit test which depicts expected behavior but currently fails none

Description Andreas Sommer 2018-06-21 14:21:43 UTC
Created attachment 194459 [details]
Unit test which depicts expected behavior but currently fails

Good on 10.3-RELEASE:

$ rm -rf /tmp/dest && mkdir /tmp/dest && touch /tmp/myfile && cd /tmp && install -l rs ./myfile /tmp/dest && ls -la /tmp/dest
total 146
drwxr-xr-x   2 rmg-builder  wheel    3 Jun 21 14:12 .
drwxrwxrwt  16 root         wheel  613 Jun 21 14:12 ..
lrwxr-xr-x   1 rmg-builder  wheel    9 Jun 21 14:12 myfile -> ../myfile

Bad on 11.1-RELEASE and 11.2-rc3:

$ rm -rf /tmp/dest && mkdir /tmp/dest && touch /tmp/myfile && cd /tmp && install -l rs ./myfile /tmp/dest && ls -la /tmp/dest
total 8
drwxr-xr-x  2 vagrant  wheel  512 Jun 21 13:01 .
drwxrwxrwt  7 root     wheel  512 Jun 21 13:01 ..
lrwxr-xr-x  1 vagrant  wheel    8 Jun 21 13:01 myfile -> ./myfile

It seems that unit tests were only added to the source code after 10.4, and I don't even know how the tool should behave. My expectation would be the previous behavior, obviously, also because `install src dest_which_is_a_directory` would normally be expected to create `dest_which_is_a_directory/basename_of_src`.

Please find attached a new unit test to check for the old behavior. Did not dig too deep into the code (usr.bin/xinstall/xinstall.c) yet to find a solution. My feeling is that the function `makelink(...)` isn't even called in the buggy case but probably should. Sharing it so more people can take a look.

I stumbled upon this because the ports tree macro `RLN` (see bsd.commands.mk) is doing exactly this, and broke our software build on 11.x for internal ports.
Comment 1 Andreas Sommer 2018-06-21 19:13:02 UTC
I was mistaken – the `makelink` function *is* called but has the following, very explicit logic to handle this case

	if (*from_name != '/') {
		/* this is already a relative link */
		do_symlink(from_name, to_name, target_sb);
		/* XXX: from_name may point outside of destdir. */
		metadata_log(to_name, "link", NULL, from_name, NULL, 0);

So it seems on purpose, but is still a breaking change from FreeBSD 10.x. Please advise. The manpage has no hint about the exact behavior. It reads

    The file(s) are copied (or linked if the -l option is specified) to the
    target file or directory.  If the destination is a directory, then the file
    is copied into directory with its original filename.

which leaves it open for humans to interpret what happens.
Comment 2 Andreas Sommer 2018-06-22 08:53:05 UTC
For users who are hit by this issue: the workaround is to specify the source as absolute path (see code snippet which distinguishes here).
Comment 3 Kyle Evans freebsd_committer 2019-03-11 19:21:29 UTC
I'm throwing bapt@ on the CC, as the committer of r284881 that (semi-)intentionally broke this.

Would it make sense to throw in a sanity check that doesn't do the short-circuit if the path relative to the destination doesn't exist/isn't a file? It seems we could do this and the bypass would still be just as effective.