On Sat, Apr 26, 2025 at 03:29:53PM +0000, Johannes Thyssen Tishman wrote:
> As discussed earlier on irc, I noticed that symlinks that point outside
> of the work tree are lost at least during update or histedit operations
> even after using the -S flag to stage and commit the symlink. Below are
> a few short scripts to reproduce what I tested (mostly the same commands
> but I included all commands every time so that they are easy to copy
> paste, hope that's okay).
> 
> # test update -c and update -b after stage and commit *with* -S
> # expected: symlink survives operation
> mkdir foo
> echo "blah" > foo/bar
> echo "blah" > file_out_of_tree
> got init test.git
> got import -r test.git/ -m'initial commit' foo/
> got checkout test.git/
> cd test
> ln -sf ../file_out_of_tree .
> got add file_out_of_tree
> got sg -S file_out_of_tree
> got commit -m 'symlink test' -S file_out_of_tree
> got up -c :head:-1
> got up -b main
> cat file_out_of_tree
> # output is target path (no EOL)

The above can be considered expected behaviour. While you might
reasonably expect the symlink to remain untouched, we do reserve the
right to transform such symlinks to regular files whenever we want.
If you want the symlink to be left alone always then do not put this
symlink under version control in the first place.

> # test update -c and histedit -m after stage and commit *with* -S
> # expected: symlink survives operation
> mkdir foo
> echo "blah" > foo/bar
> echo "blah" > file_out_of_tree
> got init test.git
> got import -r test.git/ -m'initial commit' foo/
> got checkout test.git/
> cd test
> ln -sf ../file_out_of_tree .
> got add file_out_of_tree
> got sg -S file_out_of_tree
> got commit -m 'symlink test' -S file_out_of_tree
> got up -c :head:-1
> got histedit -m # edit commit message
> # got: /path/to/file_out_of_tree: symbolic link points outside of paths under version control
> got histedit -a # <-- file is lost
> cat file_out_of_tree
> # cat: file_out_of_tree: No such file or directory

Here we have the problem that the commit step which is run by histedit
under to hood lacks the -S option. I think we should enable this flag
during rebase/histedit, just like 'got merge' already does.

With the patch below, the above case preserves the symlink as-is in
the 'test' work tree. When another work tree is checked out then the
symlink gets transformed into a regular file, as expected.

This shows a gap in our test coverage. We should rewrite your above
recipe as a proper cmdline regression test and put it in together with
the patch below.

> # test update -c and update -b after stage and commit *without* -S
> # expected: symlink is lost and file contains target path
> mkdir foo
> echo "blah" > foo/bar
> echo "blah" > file_out_of_tree
> got init test.git
> got import -r test.git/ -m'initial commit' foo/
> got checkout test.git/
> cd test
> ln -sf ../file_out_of_tree .
> got add file_out_of_tree
> got sg file_out_of_tree
> got commit -m 'symlink test' file_out_of_tree
> got up -c :head:-1 # <-- merge conflict here
> got up -b main
> cat file_out_of_tree
> # merge conflict (symlink was deleted)

It is expected that a merge conflict involving a symlink results in
a text file which contains information about the conflict, rather
than having the symlink preserved.

> Not sure my expectations are correct, but overall the results seem
> inconsistent to me. Please let me know if there's anything else you'd
> like me to test.

As far as I can see, the only issue we need to fix is this one:

diff /home/stsp/src/got
path + /home/stsp/src/got
commit - 866e94146b2586459ca83c90f49ece8c91e1fdff
blob - 8dde07e115ca4bfbb8f8c8498d793847a4343c27
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -7367,6 +7367,7 @@ rebase_commit(struct got_object_id **new_commit_id,
 	cc_arg.repo = repo;
 	cc_arg.have_staged_files = 0;
 	cc_arg.commit_conflicts = allow_conflict;
+	cc_arg.allow_bad_symlinks = 1; /* preserve bad symlinks across merges */
 	/*
 	 * If possible get the status of individual files directly to
 	 * avoid crawling the entire work tree once per rebased commit.