| Message ID | ad66d1f3e2bc84498b3168416ae1ccdd41f8a7ce.1644609683.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add cat-file --batch-command flag | expand |
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
Hi Phillip On 14 Feb 2022, at 8:59, Phillip Wood wrote: > 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 good catch! > >> + 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 Thanks for this suggestion! this would allow us to test flushing in a much more straightforward way without having to open up fifo pipes. This addresses Junio's concern in [1] about this test hanging in the future when there's a regression. Above you suggested having a timeout with sleep, which I was considering as well. However, I feel like using this env var is overall simpler and safer, so maybe we can use this for both testing the case when we get a flush and when we do not get a flush 1. https://lore.kernel.org/git/xmqqpmnt9ngx.fsf@gitster.g/ > > > >> + 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 Good point. I'll see if I can adjust this in the next version. > > 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
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:: + 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`. +-- ++ + --batch-all-objects:: Instead of reading a list of objects on stdin, perform the requested batch operation on all objects in the repository and @@ -110,7 +136,7 @@ OPTIONS that a process can interactively read and write from `cat-file`. With this option, the output uses normal stdio buffering; this is much more efficient when invoking - `--batch-check` on a large number of objects. + `--batch-check` or `--batch-command` on a large number of objects. --unordered:: When `--batch-all-objects` is in use, visit objects in an @@ -202,6 +228,13 @@ from stdin, one per line, and print information about them. By default, the whole line is considered as an object, as if it were fed to linkgit:git-rev-parse[1]. +When `--batch-command` is given, `cat-file` will read commands from stdin, +one per line, and print information based on the command given. With +`--batch-command`, the `info` command followed by an object will print +information about the object the same way `--batch-check` would, and the +`contents` command followed by an object prints contents in the same way +`--batch` would. + You can specify the information shown for each object by using a custom `<format>`. The `<format>` is copied literally to stdout for each object, with placeholders of the form `%(atom)` expanded, followed by a @@ -237,9 +270,9 @@ newline. The available atoms are: If no format is specified, the default format is `%(objectname) %(objecttype) %(objectsize)`. -If `--batch` is specified, the object information is followed by the -object contents (consisting of `%(objectsize)` bytes), followed by a -newline. +If `--batch` is specified, or if `--batch-command` is used with the `contents` +command, the object information is followed by the object contents (consisting +of `%(objectsize)` bytes), followed by a newline. For example, `--batch` without a custom format would produce: diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 5e38af82af1..6d54a0eb38d 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -20,6 +20,7 @@ enum batch_mode { BATCH_MODE_CONTENTS, BATCH_MODE_INFO, + BATCH_MODE_QUEUE_AND_DISPATCH, }; struct batch_options { @@ -513,6 +514,127 @@ static int batch_unordered_packed(const struct object_id *oid, data); } +typedef void (*parse_cmd_fn_t)(struct batch_options *, const char *, + struct strbuf *, struct expand_data *); + +struct queued_cmd { + parse_cmd_fn_t fn; + char *line; +}; + +static void parse_cmd_contents(struct batch_options *opt, + const char *line, + struct strbuf *output, + struct expand_data *data) +{ + opt->batch_mode = BATCH_MODE_CONTENTS; + batch_one_object(line, output, opt, data); +} + +static void parse_cmd_info(struct batch_options *opt, + const char *line, + struct strbuf *output, + struct expand_data *data) +{ + opt->batch_mode = BATCH_MODE_INFO; + batch_one_object(line, output, opt, data); +} + +static void dispatch_calls(struct batch_options *opt, + struct strbuf *output, + struct expand_data *data, + struct queued_cmd *cmd, + int nr) +{ + int i; + + for (i = 0; i < nr; i++){ + cmd[i].fn(opt, cmd[i].line, output, data); + free(cmd[i].line); + } + + fflush(stdout); +} + +static const struct parse_cmd { + const char *prefix; + parse_cmd_fn_t fn; + unsigned takes_args; +} commands[] = { + { "contents", parse_cmd_contents, 1}, + { "info", parse_cmd_info, 1}, +}; + +static void batch_objects_command(struct batch_options *opt, + struct strbuf *output, + struct expand_data *data) +{ + struct strbuf input = STRBUF_INIT; + struct queued_cmd *queued_cmd = NULL; + size_t alloc = 0, nr = 0; + + while (!strbuf_getline(&input, stdin)) { + int i; + const struct parse_cmd *cmd = NULL; + const char *p = NULL, *cmd_end; + struct queued_cmd call = {0}; + + if (!input.len) + die(_("empty command in input")); + if (isspace(*input.buf)) + die(_("whitespace before command: '%s'"), input.buf); + + if (skip_prefix(input.buf, "flush", &cmd_end)) { + if (!opt->buffer_output) + die(_("flush is only for --buffer mode")); + if (*cmd_end) + die(_("flush takes no arguments")); + + dispatch_calls(opt, output, data, queued_cmd, nr); + nr = 0; + continue; + } + + for (i = 0; i < ARRAY_SIZE(commands); i++) { + if (!skip_prefix(input.buf, commands[i].prefix, &cmd_end)) + continue; + + cmd = &commands[i]; + if (cmd->takes_args) { + if (*cmd_end != ' ') + die(_("%s requires arguments"), + commands[i].prefix); + + p = cmd_end + 1; + } else if (*cmd_end) { + die(_("%s takes no arguments"), + commands[i].prefix); + } + + break; + } + + if (!cmd) + die(_("unknown command: '%s'"), input.buf); + + if (!opt->buffer_output) { + cmd->fn(opt, p, output, data); + continue; + } + + ALLOC_GROW(queued_cmd, nr + 1, alloc); + call.fn = cmd->fn; + call.line = xstrdup_or_null(p); + queued_cmd[nr++] = call; + } + + if (opt->buffer_output && nr) + dispatch_calls(opt, output, data, queued_cmd, nr); + + free(queued_cmd); + strbuf_release(&input); +} + static int batch_objects(struct batch_options *opt) { struct strbuf input = STRBUF_INIT; @@ -595,6 +717,10 @@ static int batch_objects(struct batch_options *opt) save_warning = warn_on_object_refname_ambiguity; warn_on_object_refname_ambiguity = 0; + if (opt->batch_mode == BATCH_MODE_QUEUE_AND_DISPATCH) { + batch_objects_command(opt, &output, &data); + goto cleanup; + } while (strbuf_getline(&input, stdin) != EOF) { if (data.split_on_whitespace) { /* @@ -613,6 +739,7 @@ static int batch_objects(struct batch_options *opt) batch_one_object(input.buf, &output, opt, &data); } + cleanup: strbuf_release(&input); strbuf_release(&output); warn_on_object_refname_ambiguity = save_warning; @@ -645,6 +772,8 @@ static int batch_option_callback(const struct option *opt, bo->batch_mode = BATCH_MODE_CONTENTS; else if (!strcmp(opt->long_name, "batch-check")) bo->batch_mode = BATCH_MODE_INFO; + else if (!strcmp(opt->long_name, "batch-command")) + bo->batch_mode = BATCH_MODE_QUEUE_AND_DISPATCH; else BUG("%s given to batch-option-callback", opt->long_name); @@ -695,6 +824,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) N_("like --batch, but don't emit <contents>"), PARSE_OPT_OPTARG | PARSE_OPT_NONEG, batch_option_callback), + OPT_CALLBACK_F(0, "batch-command", &batch, N_("format"), + N_("read commands from stdin"), + PARSE_OPT_OPTARG | PARSE_OPT_NONEG, + batch_option_callback), OPT_CMDMODE(0, "batch-all-objects", &opt, N_("with --batch[-check]: ignores stdin, batches all known objects"), 'b'), /* Batch-specific options */ 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 && + mkfifo input && + test_when_finished 'rm input' + mkfifo output && + exec 9<>output && + 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 () { + type=$1 + sha1=$2 + size=$3 + + touch output && + 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 && + 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 +' + 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