Skip to content

cat-file: add %(objectmode) and submodule message to batch commands #1929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

cat-file: add %(objectmode) and submodule message to batch commands #1929

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e05c5be
t1006: update 'run_tests' to test generic object specifiers
vdye Jan 3, 2024
1ff39aa
cat-file: add %(objectmode) atom
vdye Jan 3, 2024
980ab7f
cat-file.c: add batch handling for submodules
vdye May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Documentation/git-cat-file.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,11 @@ newline. The available atoms are:
`objecttype`::
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jeff King wrote (reply to this):

On Mon, Jun 02, 2025 at 06:55:54PM +0000, Victoria Dye via GitGitGadget wrote:

> Add a formatting atom, used with the --batch-check/--batch-command options,
> that prints the octal representation of the object mode if a given revision
> includes that information, e.g. one that follows the format
> <tree-ish>:<path>. If the mode information does not exist, an empty string
> is printed instead.

Overall, this looks good to me. I have a few small comments below,
though I'm not sure if they merit a re-roll or not.

> @@ -345,6 +347,9 @@ static int expand_atom(struct strbuf *sb, const char *atom, int len,
>  		else
>  			strbuf_addstr(sb,
>  				      oid_to_hex(&data->delta_base_oid));
> +	} else if (is_atom("objectmode", atom, len)) {
> +		if (!data->mark_query && !(S_IFINVALID == data->mode))
> +			strbuf_addf(sb, "%06o", data->mode);
>  	} else
>  		return 0;
>  	return 1;

Looking at this hunk raised a few questions. Fortunately with answers. ;)

First, in other parts of this if/else chain, when mark_query is set we
need to perform some action (usually setting up the object_info
pointers). But we _don't_ need to do that here, since we get the mode
info "for free" from get_oid_with_context(). Good.

Second, how do we reliably get S_IFINVALID? We can see that the
expand_data struct is now initialized with it:

> +#define EXPAND_DATA_INIT  { .mode = S_IFINVALID }

But that seems like it would be a bug, since we only initialize it once,
in batch_objects():

> @@ -866,7 +872,7 @@ static int batch_objects(struct batch_options *opt)
>  {
>  	struct strbuf input = STRBUF_INIT;
>  	struct strbuf output = STRBUF_INIT;
> -	struct expand_data data;
> +	struct expand_data data = EXPAND_DATA_INIT;
>  	int save_warning;
>  	int retval = 0;
>  
> @@ -875,7 +881,6 @@ static int batch_objects(struct batch_options *opt)
>  	 * object_info to be handed to oid_object_info_extended for each
>  	 * object.
>  	 */
> -	memset(&data, 0, sizeof(data));
>  	data.mark_query = 1;
>  	expand_format(&output,
>  		      opt->format ? opt->format : DEFAULT_FORMAT,
>  
>  static int is_atom(const char *atom, const char *s, int slen)
>  {

...and then call batch_one_object() over and over. So at first glance,
doing this:

  (echo HEAD:Makefile; echo HEAD) |
  git cat-file --batch-check='%(objectmode)'

would let the mode from the first object bleed over into the second. But
that doesn't happen, because we overwrite expand_data.mode for each
object unconditionally, here:

> @@ -613,6 +618,7 @@ static void batch_one_object(const char *obj_name,
>  		goto out;
>  	}
>  
> +	data->mode = ctx.mode;
>  	batch_object_write(obj_name, scratch, opt, data, NULL, 0);
>  
>  out:

And there we are relying on ctx.mode, which we get from
get_oid_with_context(), which always falls back to S_IFINVALID if no
mode is available. Good.

But I think that means that the value set in EXPAND_DATA_INIT is never
used, and we could continue to zero-initialize the struct with memset?

That said, it's probably OK to err on the side of over-initializing. The
worst case is probably somebody later reading the code being confused
about the importance of the line. And at best it may prevent a future
code path from unexpectedly reading a funny value.


And on to the third question. In the non-batch code path of
cat_one_file(), we do:

          if (obj_context.mode == S_IFINVALID)
                  obj_context.mode = 0100644;

which made me wonder if we should be harmonizing our behavior. But that
mode is used only for passing to filter_object() and textconv_object().
Neither of which really care about the mode, and this is mostly just
saying "eh, do your regular thing as if it were a blob we found at
--path". I suspect we could get the same effect by just passing a
hard-coded 100644 to those functions, but probably not worth changing
now (and certainly very orthogonal to your patch). But the important
thing is we do not really need to worry about being consistent with this
line. Good.

-Peff

The type of the object (the same as `cat-file -t` reports).

`objectmode`::
If the specified object has mode information (such as a tree or
index entry), the mode expressed as an octal integer. Otherwise,
empty string.

`objectsize`::
The size, in bytes, of the object (the same as `cat-file -s`
reports).
Expand Down
9 changes: 7 additions & 2 deletions builtin/cat-file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ struct expand_data {
struct object_id oid;
enum object_type type;
unsigned long size;
unsigned short mode;
off_t disk_size;
const char *rest;
struct object_id delta_base_oid;
Expand Down Expand Up @@ -306,6 +307,7 @@ struct expand_data {
*/
unsigned skip_object_info : 1;
};
#define EXPAND_DATA_INIT { .mode = S_IFINVALID }

static int is_atom(const char *atom, const char *s, int slen)
{
Expand Down Expand Up @@ -345,6 +347,9 @@ static int expand_atom(struct strbuf *sb, const char *atom, int len,
else
strbuf_addstr(sb,
oid_to_hex(&data->delta_base_oid));
} else if (is_atom("objectmode", atom, len)) {
if (!data->mark_query && !(S_IFINVALID == data->mode))
strbuf_addf(sb, "%06o", data->mode);
} else
return 0;
return 1;
Expand Down Expand Up @@ -613,6 +618,7 @@ static void batch_one_object(const char *obj_name,
goto out;
}

data->mode = ctx.mode;
batch_object_write(obj_name, scratch, opt, data, NULL, 0);

out:
Expand Down Expand Up @@ -866,7 +872,7 @@ static int batch_objects(struct batch_options *opt)
{
struct strbuf input = STRBUF_INIT;
struct strbuf output = STRBUF_INIT;
struct expand_data data;
struct expand_data data = EXPAND_DATA_INIT;
int save_warning;
int retval = 0;

Expand All @@ -875,7 +881,6 @@ static int batch_objects(struct batch_options *opt)
* object_info to be handed to oid_object_info_extended for each
* object.
*/
memset(&data, 0, sizeof(data));
data.mark_query = 1;
expand_format(&output,
opt->format ? opt->format : DEFAULT_FORMAT,
Expand Down
86 changes: 54 additions & 32 deletions t/t1006-cat-file.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,53 +113,55 @@ strlen () {

run_tests () {
type=$1
oid=$2
size=$3
content=$4
pretty_content=$5
object_name="$2"
mode=$3
size=$4
content=$5
pretty_content=$6
oid=${7:-"$object_name"}

batch_output="$oid $type $size
$content"

test_expect_success "$type exists" '
git cat-file -e $oid
git cat-file -e "$object_name"
'

test_expect_success "Type of $type is correct" '
echo $type >expect &&
git cat-file -t $oid >actual &&
git cat-file -t "$object_name" >actual &&
test_cmp expect actual
'

test_expect_success "Size of $type is correct" '
echo $size >expect &&
git cat-file -s $oid >actual &&
git cat-file -s "$object_name" >actual &&
test_cmp expect actual
'

test -z "$content" ||
test_expect_success "Content of $type is correct" '
echo_without_newline "$content" >expect &&
git cat-file $type $oid >actual &&
git cat-file $type "$object_name" >actual &&
test_cmp expect actual
'

test_expect_success "Pretty content of $type is correct" '
echo_without_newline "$pretty_content" >expect &&
git cat-file -p $oid >actual &&
git cat-file -p "$object_name" >actual &&
test_cmp expect actual
'

test -z "$content" ||
test_expect_success "--batch output of $type is correct" '
echo "$batch_output" >expect &&
echo $oid | git cat-file --batch >actual &&
echo "$object_name" | git cat-file --batch >actual &&
test_cmp expect actual
'

test_expect_success "--batch-check output of $type is correct" '
echo "$oid $type $size" >expect &&
echo_without_newline $oid | git cat-file --batch-check >actual &&
echo_without_newline "$object_name" | git cat-file --batch-check >actual &&
test_cmp expect actual
'

Expand All @@ -168,44 +170,59 @@ $content"
test -z "$content" ||
test_expect_success "--batch-command $opt output of $type content is correct" '
echo "$batch_output" >expect &&
test_write_lines "contents $oid" | git cat-file --batch-command $opt >actual &&
test_write_lines "contents $object_name" | git cat-file --batch-command $opt >actual &&
test_cmp expect actual
'

test_expect_success "--batch-command $opt output of $type info is correct" '
echo "$oid $type $size" >expect &&
test_write_lines "info $oid" |
test_write_lines "info $object_name" |
git cat-file --batch-command $opt >actual &&
test_cmp expect actual
'
done

test_expect_success "custom --batch-check format" '
echo "$type $oid" >expect &&
echo $oid | git cat-file --batch-check="%(objecttype) %(objectname)" >actual &&
echo "$object_name" | git cat-file --batch-check="%(objecttype) %(objectname)" >actual &&
test_cmp expect actual
'

test_expect_success "custom --batch-command format" '
echo "$type $oid" >expect &&
echo "info $oid" | git cat-file --batch-command="%(objecttype) %(objectname)" >actual &&
echo "info $object_name" | git cat-file --batch-command="%(objecttype) %(objectname)" >actual &&
test_cmp expect actual
'

test_expect_success '--batch-check with %(rest)' '
# FIXME: %(rest) is incompatible with object names that include whitespace,
# e.g. HEAD:path/to/a/file with spaces. Use the resolved OID as input to
# test this instead of the raw object name.
if echo "$object_name" | grep " "; then
test_rest=test_expect_failure
else
test_rest=test_expect_success
fi

$test_rest '--batch-check with %(rest)' '
echo "$type this is some extra content" >expect &&
echo "$oid this is some extra content" |
echo "$object_name this is some extra content" |
git cat-file --batch-check="%(objecttype) %(rest)" >actual &&
test_cmp expect actual
'

test_expect_success '--batch-check with %(objectmode)' '
echo "$mode $oid" >expect &&
echo $object_name | git cat-file --batch-check="%(objectmode) %(objectname)" >actual &&
test_cmp expect actual
'

test -z "$content" ||
test_expect_success "--batch without type ($type)" '
{
echo "$size" &&
echo "$content"
} >expect &&
echo $oid | git cat-file --batch="%(objectsize)" >actual &&
echo "$object_name" | git cat-file --batch="%(objectsize)" >actual &&
test_cmp expect actual
'

Expand All @@ -215,7 +232,7 @@ $content"
echo "$type" &&
echo "$content"
} >expect &&
echo $oid | git cat-file --batch="%(objecttype)" >actual &&
echo "$object_name" | git cat-file --batch="%(objecttype)" >actual &&
test_cmp expect actual
'
}
Expand All @@ -230,13 +247,14 @@ test_expect_success "setup" '
git config extensions.compatobjectformat $test_compat_hash_algo &&
echo_without_newline "$hello_content" > hello &&
git update-index --add hello &&
echo_without_newline "$hello_content" > "path with spaces" &&
git update-index --add --chmod=+x "path with spaces" &&
git commit -m "add hello file"
'

run_blob_tests () {
oid=$1

run_tests 'blob' $oid $hello_size "$hello_content" "$hello_content"
run_tests 'blob' $oid "" $hello_size "$hello_content" "$hello_content"

test_expect_success '--batch-command --buffer with flush for blob info' '
echo "$oid blob $hello_size" >expect &&
Expand Down Expand Up @@ -269,13 +287,17 @@ test_expect_success '--batch-check without %(rest) considers whole line' '

tree_oid=$(git write-tree)
tree_compat_oid=$(git rev-parse --output-object-format=$test_compat_hash_algo $tree_oid)
tree_size=$(($(test_oid rawsz) + 13))
tree_compat_size=$(($(test_oid --hash=compat rawsz) + 13))
tree_pretty_content="100644 blob $hello_oid hello${LF}"
tree_compat_pretty_content="100644 blob $hello_compat_oid hello${LF}"

run_tests 'tree' $tree_oid $tree_size "" "$tree_pretty_content"
run_tests 'tree' $tree_compat_oid $tree_compat_size "" "$tree_compat_pretty_content"
tree_size=$((2 * $(test_oid rawsz) + 13 + 24))
tree_compat_size=$((2 * $(test_oid --hash=compat rawsz) + 13 + 24))
tree_pretty_content="100644 blob $hello_oid hello${LF}100755 blob $hello_oid path with spaces${LF}"
tree_compat_pretty_content="100644 blob $hello_compat_oid hello${LF}100755 blob $hello_compat_oid path with spaces${LF}"

run_tests 'tree' $tree_oid "" $tree_size "" "$tree_pretty_content"
run_tests 'tree' $tree_compat_oid "" $tree_compat_size "" "$tree_compat_pretty_content"
run_tests 'blob' "$tree_oid:hello" "100644" $hello_size "" "$hello_content" $hello_oid
run_tests 'blob' "$tree_compat_oid:hello" "100644" $hello_size "" "$hello_content" $hello_compat_oid
run_tests 'blob' "$tree_oid:path with spaces" "100755" $hello_size "" "$hello_content" $hello_oid
run_tests 'blob' "$tree_compat_oid:path with spaces" "100755" $hello_size "" "$hello_content" $hello_compat_oid

commit_message="Initial commit"
commit_oid=$(echo_without_newline "$commit_message" | git commit-tree $tree_oid)
Expand All @@ -294,8 +316,8 @@ committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE

$commit_message"

run_tests 'commit' $commit_oid $commit_size "$commit_content" "$commit_content"
run_tests 'commit' $commit_compat_oid $commit_compat_size "$commit_compat_content" "$commit_compat_content"
run_tests 'commit' $commit_oid "" $commit_size "$commit_content" "$commit_content"
run_tests 'commit' $commit_compat_oid "" $commit_compat_size "$commit_compat_content" "$commit_compat_content"

tag_header_without_oid="type blob
tag hellotag
Expand All @@ -318,8 +340,8 @@ tag_size=$(strlen "$tag_content")
tag_compat_oid=$(git rev-parse --output-object-format=$test_compat_hash_algo $tag_oid)
tag_compat_size=$(strlen "$tag_compat_content")

run_tests 'tag' $tag_oid $tag_size "$tag_content" "$tag_content"
run_tests 'tag' $tag_compat_oid $tag_compat_size "$tag_compat_content" "$tag_compat_content"
run_tests 'tag' $tag_oid "" $tag_size "$tag_content" "$tag_content"
run_tests 'tag' $tag_compat_oid "" $tag_compat_size "$tag_compat_content" "$tag_compat_content"

test_expect_success "Reach a blob from a tag pointing to it" '
echo_without_newline "$hello_content" >expect &&
Expand Down