2025-05-12T11:22:35+0200 Stefan Sperling <stsp@stsp.name>:
> 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 <jtt@openbsd.org>
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 <<EOF
+#!/bin/sh
+ed -s "\$1" <<-EOF
+	,s/.*/add really bad link/
+	w
+	EOF
+EOF
+	chmod +x $testroot/editor.sh
+
+	(cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
+		VISUAL="$testroot/editor.sh" \
+		got histedit -m > $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