Download raw body.
Weird behaviour when staging binary files interactively
Stefan Sperling <stsp@stsp.name> wrote:
> On Mon, Nov 04, 2024 at 09:56:04PM +1100, Mark Jamsek wrote:
> > Stefan Sperling <stsp@stsp.name> wrote:
> > > Should errors returned from install_blob() not be checked here?
> >
> > We break on the next line and go straight to done: already because
> > path_content is NULL when revert_binary_file is set, which is why I
> > left it out, but it's much clearer to have an explicit check there.
> > The below diff adds that, which is the only change to lib/worktree.c
> > plus expands the stage -p binary test.
>
> An explicit error check reads better indeed, thanks.
>
> > I still need to add unstage and revert coverage, though, but I'm not
> > sure I'll do that tonight as my eyes are falling out of their sockets
> > already :)
>
> Then please put it in with my OK and add more tests later. Good night :)
Here's the follow-up regress covering revert and unstage -p of binaries
commit 0b37c0f8a849d3232ba9a7ce7db2eb4f9bbbb386
from: Mark Jamsek <mark@jamsek.dev>
date: Tue Nov 5 06:03:06 2024 UTC
regress: got {revert,unstage} -p with binary file
M regress/cmdline/revert.sh | 307+ 0-
M regress/cmdline/unstage.sh | 249+ 0-
2 files changed, 556 insertions(+), 0 deletions(-)
commit - 441adc5c23cd0a3e8accffe63ec1a34bb95961ae
commit + 0b37c0f8a849d3232ba9a7ce7db2eb4f9bbbb386
blob - c3c3a66d88d49db2c7c858fa6cc6cc2c592fbe49
blob + 011e20bae0544b37165ccaf2b12d702d4869e2f0
--- regress/cmdline/revert.sh
+++ regress/cmdline/revert.sh
@@ -1559,6 +1559,312 @@ test_revert_umask() {
test_done "$testroot" 0
}
+test_revert_patch_binary() {
+ local testroot=$(test_init revert_patch_binary)
+
+ dd if=/dev/urandom of=$testroot/repo/binary bs=1024 count=16 \
+ > /dev/null 2>&1
+ git -C $testroot/repo add binary
+ git_commit $testroot/repo -m "add binary file"
+
+ got checkout $testroot/repo $testroot/wt > /dev/null
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got checkout failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ ed -s $testroot/wt/binary <<-EOF
+ 2m8
+ 7m16
+ 15m24
+ 23m32
+ w
+ EOF
+
+ cp $testroot/wt/binary $testroot/binary.expected
+
+ # cancel reverting changes with 'n' response
+ printf "n\n" > $testroot/patchscript
+ (cd $testroot/wt && got revert -F $testroot/patchscript -p binary \
+ > $testroot/stdout)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got revert command failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ revert this change? [y/n/q] n
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo "M binary" > $testroot/stdout.expected
+ (cd $testroot/wt && got status > $testroot/stdout)
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ cmp -s $testroot/binary.expected $testroot/wt/binary
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "binary file changes discarded by canceled revert" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # cancel reverting changes with 'q' response
+ printf "q\n" > $testroot/patchscript
+ (cd $testroot/wt && got revert -F $testroot/patchscript -p binary \
+ > $testroot/stdout)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got revert command failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ revert this change? [y/n/q] q
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo "M binary" > $testroot/stdout.expected
+ (cd $testroot/wt && got status > $testroot/stdout)
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ cmp -s $testroot/binary.expected $testroot/wt/binary
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "binary file changes discarded by canceled revert" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # confirm reverting changes with 'y'
+ printf "y\n" > $testroot/patchscript
+ (cd $testroot/wt && got revert -F $testroot/patchscript -p binary \
+ > $testroot/stdout)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got revert command failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ revert this change? [y/n/q] y
+ R binary
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo -n > $testroot/stdout.expected
+ (cd $testroot/wt && got status > $testroot/stdout)
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ seq 16 > $testroot/wt/numbers
+ (cd $testroot/wt && got add numbers > /dev/null)
+ (cd $testroot/wt && got commit -m "add numbers" numbers > /dev/null)
+
+ ed -s $testroot/wt/numbers <<-EOF
+ ,s/^2$/x/
+ ,s/^8$/y/
+ ,s/^16$/z/
+ w
+ EOF
+
+ # restore changed binary
+ cp -f $testroot/binary.expected $testroot/wt/binary
+
+ # revert last numbers hunk but cancel reverting binary file changes
+ printf "n\nn\nn\ny\n" > $testroot/patchscript
+ (cd $testroot/wt && got revert -F $testroot/patchscript -p -R .\
+ > $testroot/stdout)
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ revert this change? [y/n/q] n
+ -----------------------------------------------
+ @@ -1,5 +1,5 @@
+ 1
+ -2
+ +x
+ 3
+ 4
+ 5
+ -----------------------------------------------
+ M numbers (change 1 of 3)
+ revert this change? [y/n/q] n
+ -----------------------------------------------
+ @@ -5,7 +5,7 @@
+ 5
+ 6
+ 7
+ -8
+ +y
+ 9
+ 10
+ 11
+ -----------------------------------------------
+ M numbers (change 2 of 3)
+ revert this change? [y/n/q] n
+ -----------------------------------------------
+ @@ -13,4 +13,4 @@
+ 13
+ 14
+ 15
+ -16
+ +z
+ -----------------------------------------------
+ M numbers (change 3 of 3)
+ revert this change? [y/n/q] y
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo "M binary" > $testroot/stdout.expected
+ echo "M numbers" >> $testroot/stdout.expected
+ (cd $testroot/wt && got status > $testroot/stdout)
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ cmp -s $testroot/binary.expected $testroot/wt/binary
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "binary file changes discarded by canceled revert" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # revert first numbers hunk and binary file changes
+ printf "y\ny\nn\n" > $testroot/patchscript
+ (cd $testroot/wt && got revert -F $testroot/patchscript -p -R . \
+ > $testroot/stdout)
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ revert this change? [y/n/q] y
+ R binary
+ -----------------------------------------------
+ @@ -1,5 +1,5 @@
+ 1
+ -2
+ +x
+ 3
+ 4
+ 5
+ -----------------------------------------------
+ M numbers (change 1 of 2)
+ revert this change? [y/n/q] y
+ -----------------------------------------------
+ @@ -5,7 +5,7 @@
+ 5
+ 6
+ 7
+ -8
+ +y
+ 9
+ 10
+ 11
+ -----------------------------------------------
+ M numbers (change 2 of 2)
+ revert this change? [y/n/q] n
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ echo "M numbers" > $testroot/stdout.expected
+ (cd $testroot/wt && got status > $testroot/stdout)
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ test_done "$testroot" 0
+}
+
test_parseargs "$@"
run_test test_revert_basic
run_test test_revert_rm
@@ -1578,3 +1884,4 @@ run_test test_revert_deleted_subtree
run_test test_revert_symlink
run_test test_revert_patch_symlink
run_test test_revert_umask
+run_test test_revert_patch_binary
blob - 797487d03ec67c7d78cb9ee815c911e4a58cbfe6
blob + 6bb970784b5cf067c17b918409a825a1287df1e1
--- regress/cmdline/unstage.sh
+++ regress/cmdline/unstage.sh
@@ -1507,6 +1507,254 @@ EOF
test_done "$testroot" "$ret"
}
+test_unstage_patch_binary() {
+ local testroot=$(test_init unstage_patch_binary)
+
+ dd if=/dev/urandom of=$testroot/repo/binary bs=1024 count=16 \
+ > /dev/null 2>&1
+ git -C $testroot/repo add binary
+ git_commit $testroot/repo -m "add binary file"
+
+ got checkout $testroot/repo $testroot/wt > /dev/null
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got checkout failed unexpectedly" >&2
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ ed -s $testroot/wt/binary <<-EOF
+ 2m8
+ 7m16
+ 15m24
+ 23m32
+ w
+ EOF
+
+ (cd $testroot/wt && got stage > /dev/null)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got stage command failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ # cancel unstaging with 'n' response
+ printf "n\n" > $testroot/patchscript
+ (cd $testroot/wt && got unstage -F $testroot/patchscript -p \
+ > $testroot/stdout)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got unstage command failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ unstage this change? [y/n/q] n
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ (cd $testroot/wt && got status > $testroot/stdout)
+ echo " M binary" > $testroot/stdout.expected
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # cancel unstaging with 'q' response
+ printf "q\n" > $testroot/patchscript
+ (cd $testroot/wt && got unstage -F $testroot/patchscript -p \
+ > $testroot/stdout)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got unstage command failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ unstage this change? [y/n/q] q
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ (cd $testroot/wt && got status > $testroot/stdout)
+ echo " M binary" > $testroot/stdout.expected
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ # confirm unstage with 'y'
+ printf "y\n" > $testroot/patchscript
+ (cd $testroot/wt && got unstage -F $testroot/patchscript -p \
+ > $testroot/stdout)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got unstage command failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ unstage this change? [y/n/q] y
+ G binary
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ (cd $testroot/wt && got status > $testroot/stdout)
+ echo "M binary" > $testroot/stdout.expected
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ seq 16 > $testroot/wt/numbers
+ (cd $testroot/wt && got add numbers > /dev/null)
+ (cd $testroot/wt && got commit -m "add numbers" numbers > /dev/null)
+
+ ed -s $testroot/wt/numbers <<-EOF
+ ,s/^2$/x/
+ ,s/^8$/y/
+ ,s/^16$/z/
+ w
+ EOF
+
+ (cd $testroot/wt && got stage > /dev/null)
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo "got stage command failed unexpectedly" >&2
+ test_done "$testroot" "1"
+ return 1
+ fi
+
+ # unstage last numbers hunk and binary file
+ printf "y\nn\nn\ny\n" > $testroot/patchscript
+ (cd $testroot/wt && got unstage -F $testroot/patchscript -p \
+ > $testroot/stdout)
+
+ cat > $testroot/stdout.expected <<-EOF
+ -----------------------------------------------
+ Binary files binary and binary differ
+ -----------------------------------------------
+ M binary (change 1 of 1)
+ unstage this change? [y/n/q] y
+ G binary
+ -----------------------------------------------
+ @@ -1,5 +1,5 @@
+ 1
+ -2
+ +x
+ 3
+ 4
+ 5
+ -----------------------------------------------
+ M numbers (change 1 of 3)
+ unstage this change? [y/n/q] n
+ -----------------------------------------------
+ @@ -5,7 +5,7 @@
+ 5
+ 6
+ 7
+ -8
+ +y
+ 9
+ 10
+ 11
+ -----------------------------------------------
+ M numbers (change 2 of 3)
+ unstage this change? [y/n/q] n
+ -----------------------------------------------
+ @@ -13,4 +13,4 @@
+ 13
+ 14
+ 15
+ -16
+ +z
+ -----------------------------------------------
+ M numbers (change 3 of 3)
+ unstage this change? [y/n/q] y
+ G numbers
+ EOF
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ (cd $testroot/wt && got status > $testroot/stdout)
+ echo "M binary" > $testroot/stdout.expected
+ echo "MM numbers" >> $testroot/stdout.expected
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ (cd $testroot/wt && got diff binary | grep '^Binary files' \
+ > $testroot/stdout)
+ echo "Binary files binary and binary differ" \
+ > $testroot/stdout.expected
+
+ cmp -s $testroot/stdout.expected $testroot/stdout
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ diff -u $testroot/stdout.expected $testroot/stdout
+ test_done "$testroot" "$ret"
+ return 1
+ fi
+
+ test_done "$testroot" 0
+}
+
test_parseargs "$@"
run_test test_unstage_basic
run_test test_unstage_unversioned
@@ -1517,3 +1765,4 @@ run_test test_unstage_patch_removed
run_test test_unstage_patch_quit
run_test test_unstage_symlink
run_test test_unstage_patch_symlink
+run_test test_unstage_patch_binary
--
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
Weird behaviour when staging binary files interactively