From: Johannes Thyssen Tishman Subject: Re: Out-of-tree symlinks To: gameoftrees@openbsd.org Date: Thu, 31 Jul 2025 13:14:57 +0000 2025-05-12T11:22:35+0200 Stefan Sperling : > 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. A little late, but here's patch for the test together with stsp@'s diff. ok? commit 5aebe38bbd409b7c3caafbc5b5970e8418fad387 (main) from: Johannes Thyssen Tishman date: Thu Jul 31 13:12:05 2025 UTC preserve bad symlinks across merges M lib/worktree.c | 1+ 0- M regress/cmdline/histedit.sh | 93+ 0- 2 files changed, 94 insertions(+), 0 deletions(-) commit - ec7e15af96ef58f2abc62564fc58b38401903ab1 commit + 5aebe38bbd409b7c3caafbc5b5970e8418fad387 blob - e229bb2a3ce1e8dedaa1b6d9cbb7ee90751a6673 blob + 628ec31796975901db85d57d58dc1aea8554e73d --- lib/worktree.c +++ lib/worktree.c @@ -7402,6 +7402,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. blob - a5e4a5b4eb82a85c156e715f8a32adb4fe2f3256 blob + 26b2c3e0885bfb34a45095898ff615b88dd7db10 --- regress/cmdline/histedit.sh +++ regress/cmdline/histedit.sh @@ -2902,6 +2902,98 @@ EOF test_done "$testroot" 0 } +test_histedit_preserve_bad_link() { + local testroot=`test_init histedit_preserve_bad_link` + local orig_commit=`git_show_head $testroot/repo` + + got checkout $testroot/repo $testroot/wt > /dev/null + ret=$? + if [ $ret -ne 0 ]; then + echo "got checkout failed unexpectedly" + test_done "$testroot" "$ret" + return 1 + fi + + echo "file outside worktree" > $testroot/external_file + ln -sf $testroot/external_file $testroot/wt/bad_link + + (cd $testroot/wt && got add bad_link && \ + got commit -S -m 'add bad link') >/dev/null 2>&1 + ret=$? + if [ $ret -ne 0 ]; then + echo "got commit failed unexpectedly" >&2 + test_done "$testroot" "1" + return 1 + fi + local old_commit1=`git_show_head $testroot/repo` + + (cd $testroot/wt && got update -c $orig_commit \ + >/dev/null 2>&1) + ret=$? + if [ $ret -ne 0 ]; then + echo "got update failed unexpectedly" + test_done "$testroot" "$ret" + return 1 + fi + + cat > $testroot/editor.sh < $testroot/stdout \ + 2>$testroot/stderr) + + ret=$? + if [ $ret -ne 0 ]; then + echo "histedit failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + local old_commit2=`git_show_head $testroot/repo` + + local short_old_commit1=`trim_obj_id 12 $old_commit1` + local short_old_commit2=`trim_obj_id 12 $old_commit2` + + echo "A bad_link" > $testroot/stdout.expected + echo "$short_old_commit1 -> $short_old_commit2: add really bad link" \ + >> $testroot/stdout.expected + echo "Switching work tree to refs/heads/master" \ + >> $testroot/stdout.expected + + cmp -s $testroot/stdout.expected $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + cmp -s $testroot/external_file $testroot/wt/bad_link + ret=$? + if [ $ret -ne 0 ]; then + echo "contents of linked file and external file do not match" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + bad_link_path=$(readlink $testroot/wt/bad_link) + if [ "$bad_link_path" != "$testroot/external_file" ]; then + echo "link was not preserved" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + test_done "$testroot" "$ret" +} + test_parseargs "$@" run_test test_histedit_no_op run_test test_histedit_swap @@ -2931,3 +3023,4 @@ run_test test_histedit_mesg_filemode_change run_test test_histedit_drop_only run_test test_histedit_conflict_revert run_test test_histedit_no_eof_newline +run_test test_histedit_preserve_bad_link