Hi John
I've concentrated on the tests again, I think the flush tests still need
some work but the others are looking good
>[...]> Documentation/git-cat-file.txt | 41 +++++++-
builtin/cat-file.c | 133 ++++++++++++++++++++++++
t/t1006-cat-file.sh | 178 ++++++++++++++++++++++++++++++++-
3 files changed, 347 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index bef76f4dd06..e8da704477d 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -96,6 +96,32 @@ OPTIONS
need to specify the path, separated by whitespace. See the
section `BATCH OUTPUT` below for details.
+--batch-command::
+--batch-command=<format>::
as we also take an optional format string
+ Enter a command mode that reads commands and arguments from stdin. May
+ only be combined with `--buffer`, `--textconv` or `--filters`. In the
+ case of `--textconv` or `--filters`, the input lines also need to specify
+ the path, separated by whitespace. See the section `BATCH OUTPUT` below
+ for details.
++
+`--batch-command` recognizes the following commands:
++
+--
+contents `<object>`::
+ Print object contents for object reference `<object>`. This corresponds to
+ the output of `--batch`.
+
+info `<object>`::
+ Print object info for object reference `<object>`. This corresponds to the
+ output of `--batch-check`.
+
+flush::
+ Used with `--buffer` to execute all preceding commands that were issued
+ since the beginning or since the last flush was issued. When `--buffer`
+ is used, no output will come until a `flush` is issued. When `--buffer`
+ is not used, commands are flushed each time without issuing `flush`.
+--
++
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 145eee11df9..a501dbcc39b 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -177,6 +177,24 @@ $content"
test_cmp expect actual
'
+ for opt in --buffer --no-buffer
+ do
+ test -z "$content" ||
+ test_expect_success "--batch-command $opt output of $type content is correct" '
+ maybe_remove_timestamp "$batch_output" $no_ts >expect &&
+ maybe_remove_timestamp "$(test_write_lines "contents $sha1" \
+ | git cat-file --batch-command $opt)" $no_ts >actual &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "--batch-command $opt output of $type info is correct" '
+ echo "$sha1 $type $size" >expect &&
+ test_write_lines "info $sha1" \
+ | git cat-file --batch-command $opt >actual &&
+ test_cmp expect actual
+ '
+ done
+
test_expect_success "custom --batch-check format" '
echo "$type $sha1" >expect &&
echo $sha1 | git cat-file --batch-check="%(objecttype) %(objectname)" >actual &&
@@ -213,6 +231,84 @@ $content"
'
}
+run_buffer_test_flush () {
+ type=$1
+ sha1=$2
+ size=$3
+
+ rm -f input output &&
I think that this should not be needed with the addition of
"test_when_finished 'rm input output'" in run_buffer_test_no_flush()
+ mkfifo input &&
+ test_when_finished 'rm input'
+ mkfifo output &&
+ exec 9<>output &&
To address the worries about this test hanging rather than failing if
something goes wrong I wonder if we could do something like
(
sleep 10
echo "error: timeout" >&2
echo TIMEOUT >&9
) &
watchdog_pid=$! &&
test_when_finished 'kill $watchdog_pid; wait $watchdog_pid'
That should unblock any reads from fd 9 if the test hangs
+ test_when_finished 'rm output; exec 9<&-'
+ (
+ # TODO - Ideally we'd pipe the output of cat-file
+ # through "sed s'/$/\\/'" to make sure that that read
+ # would consume all the available
+ # output. Unfortunately we cannot do this as we cannot
+ # control when sed flushes its output. We could write
+ # a test helper in C that appended a '\' to the end of
+ # each line and flushes its output after every line.
+ git cat-file --buffer --batch-command <input 2>err &
+ echo $! &&
+ wait $!
+ echo $?
+ ) >&9 &
+ sh_pid=$! &&
+ read cat_file_pid <&9 &&
+ test_when_finished "kill $cat_file_pid
+ kill $sh_pid; wait $sh_pid; :" &&
+ (
+ test_write_lines "info $sha1" flush "info $sha1" &&
+ # TODO - consume all available input, not just one
+ # line (see above).
+ read actual <&9 &&
+ echo "$actual" >actual &&
+ echo "$sha1 $type $size" >expect &&
+ test_cmp expect actual
+ ) >input &&
+ # check output is flushed on exit
+ read actual <&9 &&
+ echo "$actual" >actual &&
+ test_cmp expect actual &&
+ test_must_be_empty err &&
+ read status <&9 &&
+ test "$status" -eq 0
+}
+
+run_buffer_test_no_flush () {
This test reliably hangs for me when running with --stress
+ type=$1
+ sha1=$2
+ size=$3
+
+ touch output &&
If output is missing at the end it means cat-file never ran which is an
error which we do not want to hide. This is because the subshell creates
output after opening input and before it executes cat-file below. As
input is a fifo the open will block until it is opened for writing by
another process and nothing wrote to it in V4 so I think that is why you
saw an error there.
+ test_when_finished 'rm output'
+ mkfifo input &&
+ test_when_finished 'rm input'
+ mkfifo pid &&
+ exec 9<>pid &&
+ test_when_finished 'rm pid; exec 9<&-'
+ (
+ git cat-file --buffer --batch-command <input >>output &
+ echo $! &&
+ wait $!
+ echo $?
+ ) >&9 &
+ sh_pid=$! &&
+ read cat_file_pid <&9 &&
+ test_when_finished "kill $cat_file_pid
+ kill $sh_pid; wait $sh_pid; :" &&
+ (
+ test_write_lines "info $sha1" "info $sha1" &&
+ kill $cat_file_pid &&
+ read status <&9 &&
This is where the test hangs. There seems to be a race (which I don't
understand) where we're able to read the pid of cat-file but it is not
killed by the kill above (the subshell above is blocked on "wait $!").
Adding "sleep 1" before the kill above makes everything work but I'm not
very comfortable with it. I think we might be better taking a different
approach and introducing an environment variable such as
GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT which stops cat-file flushing its
output on exit and having a test along the lines of
test_write_lines "info $sha1" | GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT=1 git
cat-file --batch-command --buffer >output &&
test_must_be_empty_output
+ test "$status" -ne 0 &&
+ test_must_be_empty output
+ ) >input
+}
+
+
hello_content="Hello World"
hello_size=$(strlen "$hello_content")
hello_sha1=$(echo_without_newline "$hello_content" | git hash-object --stdin)
@@ -224,6 +320,14 @@ test_expect_success "setup" '
run_tests 'blob' $hello_sha1 $hello_size "$hello_content" "$hello_content"
+test_expect_success PIPE '--batch-command --buffer with flush for blob info' '
+ run_buffer_test_flush blob $hello_sha1 $hello_size
+'
+
+test_expect_success PIPE '--batch-command --buffer without flush for blob info' '
+ run_buffer_test_no_flush blob $hello_sha1 $hello_size false
+'
+
test_expect_success '--batch-check without %(rest) considers whole line' '
echo "$hello_sha1 blob $hello_size" >expect &&
git update-index --add --cacheinfo 100644 $hello_sha1 "white space" &&
@@ -267,7 +371,7 @@ test_expect_success \
"Reach a blob from a tag pointing to it" \
"test '$hello_content' = \"\$(git cat-file blob $tag_sha1)\""
-for batch in batch batch-check
+for batch in batch batch-check batch-command
do
for opt in t s e p
do
@@ -373,6 +477,43 @@ test_expect_success "--batch-check with multiple sha1s gives correct format" '
"$(echo_without_newline "$batch_check_input" | git cat-file --batch-check)"
'
+test_expect_success '--batch-command with multiple info calls gives correct format' '
+ cat >expect <<-EOF &&
+ $hello_sha1 blob $hello_size
+ $tree_sha1 tree $tree_size
+ $commit_sha1 commit $commit_size
+ $tag_sha1 tag $tag_size
+ deadbeef missing
+ EOF
+
+ test_write_lines "info $hello_sha1"\
+ "info $tree_sha1"\
+ "info $commit_sha1"\
+ "info $tag_sha1"\
+ "info deadbeef" | git cat-file --batch-command --buffer >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--batch-command with multiple command calls gives correct format' '
+ cat >expect <<-EOF &&
+ $hello_sha1 blob $hello_size
+ $hello_content
+ $commit_sha1 commit $commit_size
+ $commit_content
+ $tag_sha1 tag $tag_size
+ $tag_content
+ deadbeef missing
+ EOF
+
+ maybe_remove_timestamp "$(cat expect)" 1 >expect &&
+ maybe_remove_timestamp "$(test_write_lines "contents $hello_sha1"\
+ "contents $commit_sha1"\
+ "contents $tag_sha1"\
+ "contents deadbeef"\
+ "flush" | git cat-file --batch-command --buffer)" 1 >actual &&
+ test_cmp expect actual
It is a shame that maybe_remove_timestamp does no read from stdin, this
test would look much nicer if it did. Apart from that these and the ones
below are looking good
Best Wishes
Phillip
+'
+
test_expect_success 'setup blobs which are likely to delta' '
test-tool genrandom foo 10240 >foo &&
{ cat foo && echo plus; } >foo-plus &&
@@ -963,5 +1104,40 @@ test_expect_success 'cat-file --batch-all-objects --batch-check ignores replace'
echo "$orig commit $orig_size" >expect &&
test_cmp expect actual
'
+test_expect_success 'batch-command empty command' '
+ echo "" >cmd &&
+ test_expect_code 128 git cat-file --batch-command <cmd 2>err &&
+ grep "^fatal:.*empty command in input.*" err
+'
+
+test_expect_success 'batch-command whitespace before command' '
+ echo " info deadbeef" >cmd &&
+ test_expect_code 128 git cat-file --batch-command <cmd 2>err &&
+ grep "^fatal:.*whitespace before command.*" err
+'
+
+test_expect_success 'batch-command unknown command' '
+ echo unknown_command >cmd &&
+ test_expect_code 128 git cat-file --batch-command <cmd 2>err &&
+ grep "^fatal:.*unknown command.*" err
+'
+
+test_expect_success 'batch-command missing arguments' '
+ echo "info" >cmd &&
+ test_expect_code 128 git cat-file --batch-command <cmd 2>err &&
+ grep "^fatal:.*info requires arguments.*" err
+'
+
+test_expect_success 'batch-command flush with arguments' '
+ echo "flush arg" >cmd &&
+ test_expect_code 128 git cat-file --batch-command --buffer <cmd 2>err &&
+ grep "^fatal:.*flush takes no arguments.*" err
+'
+
+test_expect_success 'batch-command flush without --buffer' '
+ echo "flush arg" >cmd &&
+ test_expect_code 128 git cat-file --batch-command <cmd 2>err &&
+ grep "^fatal:.*flush is only for --buffer mode.*" err
+'
test_done
