Re: [PATCH GSoC 5/5] cat-file: unify default format
"Pablo Sabater" <[email protected]> Wed, 29 Jul 2026 14:23:41 +0200
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
On Wed Jul 29, 2026 at 11:59 AM CEST, Chandra Pratap wrote: > On Sat, 25 Jul 2026 at 17:25, Pablo Sabater <[email protected]> wrote: >> >> %(objecttype) is supported both by the client and by the server. >> Change the temporary default format to the unified version that other >> commands use. > > s/other/the other Will fix thanks. > >> Update documentation to remove %(objecttype) from the caveats of >> remote-object-info. >> >> Update tests that used the default format to expect type. > > Not super accurate. We're updating the tests to expect the new default > format. I will reword it to something like: Now that type is supported and the default format unified, update the tests to expect the new default format. > >> Update documentation to show %(objecttype) support. >> >> Mentored-by: Karthik Nayak <[email protected]> >> Mentored-by: Chandra Pratap <[email protected]> >> Signed-off-by: Pablo Sabater <[email protected]> >> --- >> Documentation/git-cat-file.adoc | 17 ++++----- >> Documentation/gitprotocol-v2.adoc | 15 ++++++-- >> builtin/cat-file.c | 7 ---- >> t/t1017-cat-file-remote-object-info.sh | 70 ++++++++++++---------------------- >> 4 files changed, 42 insertions(+), 67 deletions(-) >> >> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc >> index ac3b528c6f..514bfc0032 100644 >> --- a/Documentation/git-cat-file.adoc >> +++ b/Documentation/git-cat-file.adoc >> @@ -348,15 +348,12 @@ newline. The available atoms are: >> after that first run of whitespace (i.e., the "rest" of the >> line) are output in place of the `%(rest)` atom. >> >> -The command `remote-object-info` only supports the `%(objectname)` and >> -`%(objectsize)` placeholders. See `CAVEATS` below for more information. >> +The command `remote-object-info` only supports the `%(objectname)`, >> +`%(objectsize)` and `%(objecttype)` placeholders. See `CAVEATS` below for more >> +information. >> >> If no format is specified, the default format is `%(objectname) >> -%(objecttype) %(objectsize)`, except for `remote-object-info` commands which >> -use `%(objectname) %(objectsize)` because `%(objecttype)` is not supported yet. >> - >> -WARNING: When "%(objecttype)" is supported, the default format WILL be unified, >> -so DO NOT RELY on the current default format to stay the same!!! >> +%(objecttype) %(objectsize)`. >> >> If `--batch` is specified, or if `--batch-command` is used with the `contents` >> command, the object information is followed by the object contents (consisting >> @@ -453,9 +450,9 @@ scripting purposes. >> CAVEATS >> ------- >> >> -Note that only `%(objectname)` and `%(objectsize)` are currently >> -supported by the `remote-object-info` command. Using any other placeholder in >> -the format string will return an empty string in its position. >> +Note that only `%(objectname)`, `%(objectsize)` and `%(objecttype)` are >> +currently supported by the `remote-object-info` command. Using any other >> +placeholder in the format string will return an empty string in its position. >> >> Note that the sizes of objects on disk are reported accurately, but care >> should be taken in drawing conclusions about which refs or objects are >> diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc >> index 7bf62014c3..de4bfb776e 100644 >> --- a/Documentation/gitprotocol-v2.adoc >> +++ b/Documentation/gitprotocol-v2.adoc >> @@ -558,14 +558,17 @@ object-info >> >> `object-info` is the command to retrieve information about one or more objects. >> Its main purpose is to allow a client to make decisions based on this >> -information without having to fully fetch objects. Object size is the only >> -information that is currently supported. >> +information without having to fully fetch objects. Object size and type are the >> +only information that is currently supported. > > s/is currently/are currently Will fix. > >> An `object-info` request takes the following arguments: >> >> size >> Requests size information to be returned for each listed object id. >> >> + type >> + Requests type information to be returned for each listed object id. >> + >> oid <oid> >> Indicates to the server an object which the client wants to obtain >> information for. They must be full OIDs. >> @@ -580,11 +583,15 @@ space. >> info = *PKT-LINE(attr LF) >> *PKT-LINE(obj-info LF) >> >> - attr = "size" >> + attr = "size" | "type" >> >> obj-size = 1*DIGIT >> >> - obj-info = obj-id [SP [obj-size]] >> + obj-type = "blob" | "tree" | "commit" | "tag" >> + >> + obj-val = obj-size | obj-type >> + >> + obj-info = obj-id [SP [obj-val *(SP obj-val)]] >> >> If the server does not recognize the OID, the response will be `<oid> SP` >> regardless of the number of attributes requested. >> diff --git a/builtin/cat-file.c b/builtin/cat-file.c >> index 884b6d5ad3..8288511b19 100644 >> --- a/builtin/cat-file.c >> +++ b/builtin/cat-file.c >> @@ -841,15 +841,9 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, >> struct object_info *remote_object_info = NULL; >> struct oid_array object_info_oids = OID_ARRAY_INIT; >> struct string_list object_info_options = STRING_LIST_INIT_NODUP; >> - const char *saved_format = opt->format; >> >> if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE) >> die(_("remote-object-info command too long")); >> - /* >> - * TODO: Use the default format once %(objecttype) is supported. >> - */ >> - if (!opt->format) >> - opt->format = "%(objectname) %(objectsize)"; >> >> line_to_split = xstrdup(line); >> count = split_cmdline(line_to_split, &argv); >> @@ -904,7 +898,6 @@ static void parse_cmd_remote_object_info(struct batch_options *opt, >> data->is_remote = 0; >> } >> data->skip_object_info = 0; >> - opt->format = saved_format; >> >> for (size_t i = 0; i < object_info_oids.nr; i++) >> free_object_info_contents(&remote_object_info[i]); >> diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh >> index 175f778cc9..741bdf34a0 100755 >> --- a/t/t1017-cat-file-remote-object-info.sh >> +++ b/t/t1017-cat-file-remote-object-info.sh >> @@ -139,10 +139,10 @@ test_expect_success 'batch-command remote-object-info git:// default filter' ' >> set_transport_variables "$daemon_parent" && >> cd "$daemon_parent/daemon_client_empty" && >> >> - echo "$hello_oid $hello_size" >expect && >> - echo "$tree_oid $tree_size" >>expect && >> - echo "$commit_oid $commit_size" >>expect && >> - echo "$tag_oid $tag_size" >>expect && >> + echo "$hello_oid $hello_type $hello_size" >expect && >> + echo "$tree_oid $tree_type $tree_size" >>expect && >> + echo "$commit_oid $commit_type $commit_size" >>expect && >> + echo "$tag_oid $tag_type $tag_size" >>expect && >> >> git cat-file --batch-command >actual <<-EOF && >> remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid $tree_oid >> @@ -152,28 +152,6 @@ test_expect_success 'batch-command remote-object-info git:// default filter' ' >> ) >> ' >> >> -test_expect_success 'remote-object-info does not change the default format of info' ' >> - ( >> - set_transport_variables "$daemon_parent" && >> - cd "$daemon_parent/daemon_client_empty" && >> - >> - local_content="local object" && >> - local_oid=$(echo_without_newline "$local_content" | git hash-object -w --stdin) && >> - local_size=$(strlen "$local_content") && >> - >> - echo "$local_oid blob $local_size" >expect && >> - echo "$hello_oid $hello_size" >>expect && >> - echo "$local_oid blob $local_size" >>expect && >> - >> - git cat-file --batch-command >actual <<-EOF && >> - info $local_oid >> - remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid >> - info $local_oid >> - EOF >> - test_cmp expect actual >> - ) >> -' >> - > > I feel like deleting this test removes the only test in this file that validates > calling info and remote-object-info in the same cat-file --batch-command > session. > > Instead of deleting it, we should update it. Perhaps something like this: > > test "remote-object-info and info can be mixed using the unified > default format": > // 1. Environment setup > // 2. Prepare a local object for the 'info' command > // 3. Construct the expected output. Since the default format is > now unified, > // both commands should output exactly: <OID> <TYPE> <SIZE> > // 4. Execute the batch command > // 5. Validate This test was because we were modifying the default format whenever remote-object-info was used. The workaround to have them work together was to store the previous format, save the format as the capped one for remote-object-info and at the end restore it. Now that it has been unified for both it is implicit that they can work together. However, no harm in having a test explicitly proving it. I will make what you suggest. Thanks for the feedback, Pablo