Git development
 help / color / mirror / Atom feed
* [PATCH v2 0/6] some "commit-graph verify" fixes for chains
From: Jeff King @ 2023-09-28  4:37 UTC (permalink / raw)
  To: git; +Cc: Taylor Blau, Derrick Stolee
In-Reply-To: <20230926055452.GA1341109@coredump.intra.peff.net>

On Tue, Sep 26, 2023 at 01:54:52AM -0400, Jeff King wrote:

>   [1/6]: commit-graph: factor out chain opening function
>   [2/6]: commit-graph: check mixed generation validation when loading chain file
>   [3/6]: t5324: harmonize sha1/sha256 graph chain corruption
>   [4/6]: commit-graph: detect read errors when verifying graph chain
>   [5/6]: commit-graph: tighten chain size check
>   [6/6]: commit-graph: report incomplete chains during verification

Here's a re-roll that fixes a missed case in patch 6 (I sent more
details elsewhere in the thread).

Range-diff is below, but the changes are:

  - patch 3 extends its tests to cover corrupting both the first and
    second lines of the file. The test and directory renaming spills
    over into the range-diff context for the other patches (and I
    renamed the directory used in the "too-short chain file" test in
    patch 5 to align better with the others).

  - patch 6 covers the case that get_oid_hex() fails (previously it only
    detected that a parsed hex failed to result in us loading a valid
    graph file). The range-diff is IMHO quite hard to read here, but the
    patch itself is quite simple.

1:  baa20e6391 = 1:  a4893f325f commit-graph: factor out chain opening function
2:  b77cc15584 = 2:  722181d8ed commit-graph: check mixed generation validation when loading chain file
3:  af350d1d41 < -:  ---------- t5324: harmonize sha1/sha256 graph chain corruption
-:  ---------- > 3:  7de47054c7 t5324: harmonize sha1/sha256 graph chain corruption
4:  54ce3863f8 ! 4:  313c051a38 commit-graph: detect read errors when verifying graph chain
    @@ builtin/commit-graph.c: static int graph_verify(int argc, const char **argv, con
      	free_commit_graph(graph);
     
      ## t/t5324-split-commit-graph.sh ##
    -@@ t/t5324-split-commit-graph.sh: test_expect_success 'verify after commit-graph-chain corruption' '
    +@@ t/t5324-split-commit-graph.sh: test_expect_success 'verify after commit-graph-chain corruption (base)' '
      	(
    - 		cd verify-chain &&
    + 		cd verify-chain-base &&
      		corrupt_file "$graphdir/commit-graph-chain" 30 "G" &&
     -		git commit-graph verify 2>test_err &&
     +		test_must_fail git commit-graph verify 2>test_err &&
5:  273f5e8b87 ! 5:  6d9efd761a commit-graph: tighten chain size check
    @@ commit-graph.c: int open_commit_graph_chain(const char *chain_file,
      	return 1;
     
      ## t/t5324-split-commit-graph.sh ##
    -@@ t/t5324-split-commit-graph.sh: test_expect_success 'verify after commit-graph-chain corruption' '
    +@@ t/t5324-split-commit-graph.sh: test_expect_success 'verify after commit-graph-chain corruption (tip)' '
      	)
      '
      
     +test_expect_success 'verify notices too-short chain file' '
    -+	git clone --no-hardlinks . verify-chain-garbage &&
    ++	git clone --no-hardlinks . verify-chain-short &&
     +	(
    -+		cd verify-chain-garbage &&
    ++		cd verify-chain-short &&
     +		git commit-graph verify &&
     +		echo "garbage" >$graphdir/commit-graph-chain &&
     +		test_must_fail git commit-graph verify 2>test_err &&
6:  2661efed03 ! 6:  8fc82e2254 commit-graph: report incomplete chains during verification
    @@ Commit message
         "tip" case is what is fixed by this patch (it complains to stderr but
         still returns the base slice).
     
    +    Likewise the existing tests for corruption of the commit-graph-chain
    +    file itself need to be updated. We already exited non-zero correctly for
    +    the "base" case, but the "tip" case can now do so, too.
    +
         Note that this also causes us to adjust a test later in the file that
         similarly corrupts a tip (though confusingly the test script calls this
         "base"). It checks stderr but erroneously expects the whole "verify"
    @@ commit-graph.c: int open_commit_graph_chain(const char *chain_file,
      {
      	struct commit_graph *graph_chain = NULL;
      	struct strbuf line = STRBUF_INIT;
    - 	struct object_id *oids;
    - 	int i = 0, valid = 1, count;
    - 	FILE *fp = xfdopen(fd, "r");
    - 
    -+	*incomplete_chain = 0;
    -+
    - 	count = st->st_size / (the_hash_algo->hexsz + 1);
    - 	CALLOC_ARRAY(oids, count);
    - 
     @@ commit-graph.c: struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r,
    + 	fclose(fp);
    + 	strbuf_release(&line);
    + 
    ++	*incomplete_chain = !valid;
    + 	return graph_chain;
    + }
      
    - 		if (!valid) {
    - 			warning(_("unable to find all commit-graph files"));
    -+			*incomplete_chain = 1;
    - 			break;
    - 		}
    - 	}
     @@ commit-graph.c: static struct commit_graph *load_commit_graph_chain(struct repository *r,
      	struct commit_graph *g = NULL;
      
    @@ t/t5324-split-commit-graph.sh: test_expect_success 'warn on base graph chunk inc
      		grep -v "^+" test_err >err &&
      		test_i18ngrep "commit-graph chain does not match" err
      	)
    +@@ t/t5324-split-commit-graph.sh: test_expect_success 'verify after commit-graph-chain corruption (tip)' '
    + 	(
    + 		cd verify-chain-tip &&
    + 		corrupt_file "$graphdir/commit-graph-chain" 70 "G" &&
    +-		git commit-graph verify 2>test_err &&
    ++		test_must_fail git commit-graph verify 2>test_err &&
    + 		grep -v "^+" test_err >err &&
    + 		test_i18ngrep "invalid commit-graph chain" err &&
    + 		corrupt_file "$graphdir/commit-graph-chain" 70 "A" &&
    +-		git commit-graph verify 2>test_err &&
    ++		test_must_fail git commit-graph verify 2>test_err &&
    + 		grep -v "^+" test_err >err &&
    + 		test_i18ngrep "unable to find all commit-graph files" err
    + 	)

  [1/6]: commit-graph: factor out chain opening function
  [2/6]: commit-graph: check mixed generation validation when loading chain file
  [3/6]: t5324: harmonize sha1/sha256 graph chain corruption
  [4/6]: commit-graph: detect read errors when verifying graph chain
  [5/6]: commit-graph: tighten chain size check
  [6/6]: commit-graph: report incomplete chains during verification

 builtin/commit-graph.c        |  31 +++++++---
 commit-graph.c                | 109 +++++++++++++++++++++-------------
 commit-graph.h                |   4 ++
 t/t5324-split-commit-graph.sh |  69 ++++++++++++++++++---
 4 files changed, 158 insertions(+), 55 deletions(-)

-Peff

^ permalink raw reply

* Re: [PATCH 6/6] commit-graph: report incomplete chains during verification
From: Jeff King @ 2023-09-28  4:30 UTC (permalink / raw)
  To: git; +Cc: Taylor Blau, Derrick Stolee
In-Reply-To: <20230926060430.GF1341418@coredump.intra.peff.net>

On Tue, Sep 26, 2023 at 02:04:30AM -0400, Jeff King wrote:

> @@ -608,6 +611,7 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r,
>  
>  		if (!valid) {
>  			warning(_("unable to find all commit-graph files"));
> +			*incomplete_chain = 1;
>  			break;
>  		}
>  	}

Reviewing my own patch since I noticed it misses a case. ;)

The whole function here looks like (abbreviated):

  int valid = 1;

  for (i = 0; i < count; i++) {
          if (get_oid_hex(line.buf, &oids[i])) {
                  warning(_("invalid commit-graph chain: line '%s' not a hash"),
                          line.buf);
                  valid = 0;
                  break;
          }
  
          valid = 0;
          for (odb = r->objects->odb; odb; odb = odb->next) {
                  struct commit_graph *g = load_commit_graph_one(r, graph_name, odb);
                  if (g) {
                          if (add_graph_to_chain(g, graph_chain, oids, i)) {
                                  graph_chain = g;
                                  valid = 1;
                          }
  
                          break;
                  }
          }
  
          if (!valid) {
                  warning(_("unable to find all commit-graph files"));
                  break;
          }
  }

My patch updates the final "if (!valid)" check, which covers anything
that happened in the loop over the odb (i.e., finding and opening the
file itself). But if we see a line which does not parse as an oid, we
break out of the outer loop earlier. We set "valid", but nobody looks at
it! So the caller would not be correctly notified that we had an error
in that case.

The fix is simple: we can just check "valid" after leaving the outer
loop, which covers both cases. And we'll want an extra test to check it.
This is actually covered by the test modified earlier in patch 3, where
sha1 and sha256 produced different results. I fixed it there by
consistently corrupting the first line of the file, but we really want
to check both cases.

I'll send a re-roll in a moment.

-Peff

^ permalink raw reply

* Re: [PATCH 03/30] object-names: Support input of oids in any supported hash
From: Eric Sunshine @ 2023-09-27 23:29 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Junio C Hamano, git, brian m. carlson, Eric W. Biederman
In-Reply-To: <20230927195537.1682-3-ebiederm@gmail.com>

On Wed, Sep 27, 2023 at 3:56 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
> Support short oids encoded in any algorithm, while ensuring enough of
> the oid is specified to disambiguate between all of the oids in the
> repository encoded in any algorithm.
>
> By default have the code continue to only accept oids specified in the
> storage hash algorithm of the repository, but when something is
> ambiguous display all of the possible oids from any oid encoding.
>
> A new flag is added GET_OID_HASH_ANY that when supplied causes the
> code to accept oids specified in any hash algorithm, and to return the
> oids that were resolved.
>
> This implements the functionality that allows both SHA-1 and SHA-256
> object names, from the "Object names on the command line" section of
> the hash function transition document.
>
> Care is taken in get_short_oid so that when the result is ambiguous
> the output remains the same of GIT_OID_HASH_ANY was not supplied.

s/of/as if/

> If GET_OID_HASH_ANY was supplied objects of any hash algorithm
> that match the prefix are displayed.
>
> This required updating repo_for_each_abbrev to give it a parameter
> so that it knows to look at all hash algorithms.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> diff --git a/object-name.c b/object-name.c
> @@ -49,6 +50,7 @@ struct disambiguate_state {
>  static void update_candidates(struct disambiguate_state *ds, const struct object_id *current)
>  {
> +       /* The hash algorithm of the current has already been filtered */

Is there a word missing after "current"?

> @@ -503,8 +516,13 @@ static int sort_ambiguous(const void *a, const void *b, void *ctx)
> -       if (a_type == b_type)
> -               return oidcmp(a, b);
> +       if (a_type == b_type) {
> +               /* Is the hash algorithm the same? */
> +               if (a->algo == b->algo)
> +                       return oidcmp(a, b);
> +               else
> +                       return a->algo > b->algo ? 1 : -1;
> +       }

Nit: unnecessary comment ("Is the hash algorithm...") is merely
repeating what the code itself already says clearly enough

> @@ -553,6 +575,7 @@ static enum get_oid_result get_short_oid(struct repository *r,
>         else
>                 ds.fn = default_disambiguate_hint;
>
> +
>         find_short_object_filename(&ds);

Nit: unnecessary new blank line

^ permalink raw reply

* Re: [PATCH 02/30] oid-array: Teach oid-array to handle multiple kinds of oids
From: Eric Sunshine @ 2023-09-27 23:20 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Junio C Hamano, git, brian m. carlson, Eric W. Biederman
In-Reply-To: <20230927195537.1682-2-ebiederm@gmail.com>

On Wed, Sep 27, 2023 at 3:55 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
> While looking at how to handle input of both SHA-1 and SHA-256 oids in
> get_oid_with_context, I realized that the oid_array in
> repo_for_each_abbrev might have more than one kind of oid stored in it
> simulataneously.

s/simulataneously/simultaneously/

> Update to oid_array_append to ensure that oids added to an oid array
> always have an algorithm set.
>
> Update void_hashcmp to first verify two oids use the same hash algorithm
> before comparing them to each other.
>
> With that oid-array should be safe to use with differnt kinds of
> oids simultaneously.

s/differnt/different/

> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

^ permalink raw reply

* Re: [PATCH 21/30] repository: Implement extensions.compatObjectFormat
From: Junio C Hamano @ 2023-09-27 21:39 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <20230927195537.1682-21-ebiederm@gmail.com>

"Eric W. Biederman" <ebiederm@gmail.com> writes:

> diff --git a/setup.c b/setup.c
> index deb5a33fe9e1..87b40472dbc5 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -598,6 +598,25 @@ static enum extension_result handle_extension(const char *var,
>  		}

This line in the pre-context needed fuzzing, but otherwise the
series applied cleanly on top of v2.42.0.

> Subject: Re: [PATCH 21/30] repository: Implement extensions.compatObjectFormat

"Implement" -> "implement" (many other patches share the same
problem, none of which I fixed while queueing).


^ permalink raw reply

* Re: [PATCH 00/30] Initial support for multiple hash functions
From: Junio C Hamano @ 2023-09-27 21:31 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: git, brian m. carlson
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

"Eric W. Biederman" <ebiederm@gmail.com> writes:

> I have been going over and over this patchset trying to figure
> out if it is ready to be merged.  I don't know of any deficiencies
> so it is at a point it could benefit from a set of eyes that
> are not mine.
>
> I had planned to wait a little bit longer but there are some on-going
> conversations that could benefit from people seeing what it means for a
> repository to support two hash functions at the same time.

Thanks.  On top of what commit are these patches expected to be applied?

^ permalink raw reply

* Re: [PATCH 01/30] object-file-convert: Stubs for converting from one object format to another
From: Eric Sunshine @ 2023-09-27 20:42 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Junio C Hamano, git, brian m. carlson, Eric W. Biederman
In-Reply-To: <20230927195537.1682-1-ebiederm@gmail.com>

On Wed, Sep 27, 2023 at 3:55 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
> Two basic functions are provided:
> - convert_object_file Takes an object file it's type and hash algorithm
>   and converts it into the equivalent object file that would
>   have been generated with hash algorithm "to".
>
>   For blob objects there is no converstion to be done and it is an
>   error to use this function on them.

s/converstion/conversion/

>   For commit, tree, and tag objects embedded oids are replaced by the
>   oids of the objects they refer to with those objects and their
>   object ids reencoded in with the hash algorithm "to".  Signatures
>   are rearranged so that they remain valid after the object has
>   been reencoded.
>
> - repo_oid_to_algop which takes an oid that refers to an object file
>   and returns the oid of the equavalent object file generated
>   with the target hash algorithm.

s/equavalent/equivalent/

> The pair of files object-file-convert.c and object-file-convert.h are
> introduced to hold as much of this logic as possible to keep this
> conversion logic cleanly separated from everything else and in the
> hopes that someday the code will be clean enough git can support
> compiling out support for sha1 and the various conversion functions.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Just some minor comments below, many of which are subjective
style-related observations, thus not necessarily actionable, but also
one or two legitimate questions.

> diff --git a/object-file-convert.c b/object-file-convert.c
> @@ -0,0 +1,57 @@
> +int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
> +                     const struct git_hash_algo *to, struct object_id *dest)
> +{
> +       /*
> +        * If the source alogirthm is not set, then we're using the
> +        * default hash algorithm for that object.
> +        */

s/alogirthm/algorithm/

> +       const struct git_hash_algo *from =
> +               src->algo ? &hash_algos[src->algo] : repo->hash_algo;
> +
> +       if (from == to) {
> +               if (src != dest)
> +                       oidcpy(dest, src);
> +               return 0;
> +       }
> +       return -1;
> +}

On this project, we usually get the simple cases out of the way first,
which often reduces the indentation level, making the code easier to
digest at a glance. So, it would be typical to write this as:

    if (from != to)
        return -1
    if (src != dest)
        oidcpy(dest, src);
    return 0;

or even:

    if (from != to)
        return -1
    if (src == dest)
        return 0;
    oidcpy(dest, src);
    return 0;

This way, for instance, the reader doesn't get to the end of the
function and then have to scan backward to understand the condition of
the `return -1`.

> +int convert_object_file(struct strbuf *outbuf,
> +                       const struct git_hash_algo *from,
> +                       const struct git_hash_algo *to,
> +                       const void *buf, size_t len,
> +                       enum object_type type,
> +                       int gentle)
> +{
> +       int ret;
> +
> +       /* Don't call this function when no conversion is necessary */
> +       if ((from == to) || (type == OBJ_BLOB))
> +               die("Refusing noop object file conversion");

Several comments...

Style: we usually reduce the noise level by dropping the extra parentheses:

    if (from == to || type == OBJ_BLOB)

Does this condition represent a programming error or a runtime error
triggerable by some input? If a programming error, then use BUG()
rather than die().

If a triggerable runtime error, then...

* start user-facing messages with lowercase rather than capitalized word

* make the user-facing message localizable so readers of other
languages can digest it

    die(_("refusing do-nothing object conversion"));

On the other hand, don't make BUG() messages localizable.

> +       switch (type) {
> +       case OBJ_COMMIT:
> +       case OBJ_TREE:
> +       case OBJ_TAG:
> +       default:
> +               /* Not implemented yet, so fail. */
> +               ret = -1;
> +               break;
> +       }
> +       if (!ret)
> +               return 0;
> +       if (gentle) {
> +               strbuf_release(outbuf);
> +               return ret;
> +       }

This function appears to be a mere skeleton at the moment, so it's
difficult to judge at this point whether you are using `outbuf` as a
bag of bytes or as a legitimate string container. If the latter, then
the API may be reasonable, but if you're using it as a bag-of-bytes,
then it feels like you're leaking an implementation detail into the
API.

> +       die(_("Failed to convert object from %s to %s"),
> +               from->name, to->name);

s/Failed/failed/

For people trying to diagnose this problem, would it be helpful to
present more information about the failed conversion, such as object
type and perhaps even its OID?

> diff --git a/object-file-convert.h b/object-file-convert.h
> @@ -0,0 +1,24 @@
> +int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
> +                     const struct git_hash_algo *to, struct object_id *dest);

I suppose the function name is pretty much self-explanatory to those
familiar with the underlying concepts, but it might still be helpful
to add a comment explaining what the function does.

> +/*
> + * Convert an object file from one hash algorithm to another algorithm.
> + * Return -1 on failure, 0 on success.
> + */
> +int convert_object_file(struct strbuf *outbuf,
> +                       const struct git_hash_algo *from,
> +                       const struct git_hash_algo *to,
> +                       const void *buf, size_t len,
> +                       enum object_type type,
> +                       int gentle);

^ permalink raw reply

* Re: [PATCH v3] bulk-checkin: only support blobs in index_bulk_checkin
From: Eric W. Biederman @ 2023-09-27 20:13 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Junio C Hamano, brian m. carlson, git
In-Reply-To: <ZROHrSmmZOIE6bl9@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

> On Tue, Sep 26, 2023 at 02:48:31PM -0700, Junio C Hamano wrote:
>> > Avoid all of those future complications by limiting index_bulk_checkin
>> > to only work on blobs.
>>
>> Thanks.  Will queue.
>
> Hmm. I wonder if retaining some flexibility in the bulk-checkin
> mechanism may be worthwhile. We discussed at the Contributor's
> Summit[^1] today that the bulk-checkin system may be a good fit for
> packing any blobs/trees created by `merge-tree` or `replay` instead of
> writing them out as loose objects.
>
> Being able to write trees in addition to blobs is definitely important
> there, so we may want to wait on merging this down until that direction
> solidifies a bit more. (FWIW, I started working on that today and hope
> to have patches on the list in the next day or two).
>
> Alternatively, if there is an urgency to merge these down, we can always
> come back to it in the future and revert it if need be. Either way
> :-).

There are two things that index_bulk_checkin does.
- Handle objects that are too large to fit into a memory
- Place objects immediately in a pack.

Do I read things correctly that you want to take an object that is small
enough to fit into memory, and to immediately into a pack?

If so you essentially want write_object_file that directly writes to a
pack?

A version of write_object_file that that directly writes to a pack is
much easier than the chunking that index_bulk_checkin does.

Perhaps your version could be called index_pack_checkin?

Eric

^ permalink raw reply

* Re: [PATCH v3] bulk-checkin: only support blobs in index_bulk_checkin
From: Eric W. Biederman @ 2023-09-27 20:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Taylor Blau, brian m. carlson, Elijah Newren, git
In-Reply-To: <xmqq7cobmvjt.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> Taylor Blau <me@ttaylorr.com> writes:
>
>> I am not sure that I follow. If we have an address in memory from which
>> we want to stream raw bytes directly to the packfile, that should work
>> for all objects regardless of type, no?
>
> For a single hash world, yes.  For keeping track of "the other hash"
> and correspondence, you need to (1) interpret the contents of the
> object (e.g., if you received a tree contents for SHA-1 repository,
> you'd need to split them into tree entries and know which parts of
> the bytestream are SHA-1 hashes of the tree contebnts), (2) come
> up with the corresponding tree contents in the SHA-256 world (you
> should be able to do that now you know SHA-1 names of the objects
> directly referred to by the tree) and hash that using SHA-256, and
> (3) remember the SHA-1 and the SHA-256 name correspondence of the
> tree object you just hashed, in addition to the usual (4) hashing
> the contents using SHA-1 hash algorithm without caring what the byte
> stream represents.

If it helps I just posted a patchset that implements what it takes
to deal with objects small enough to live in-core.

You can read object-file-convert.c to see what it takes to generate
an object in the other hash function world.

The exercise for the reader is how to apply this to objects that
are too large to fit in memory.

Eric


^ permalink raw reply

* Re: [PATCH] setup: Only allow extenions.objectFormat to be specified once
From: Junio C Hamano @ 2023-09-27 19:56 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: git, brian m. carlson
In-Reply-To: <87r0mjn4ly.fsf@gmail.froward.int.ebiederm.org>

"Eric W. Biederman" <ebiederm@gmail.com> writes:

> For me the fundamental question is if we allow multiples compatibility
> hashes or historical hashes how do we specify them?  Have the option
> appear more than once?  A comma separated list?

As you found out, we tend to use both, but the former does look more
natural to me.

The "usual" pros and cons [*] involve how easy it is to override the
settings given by more general low-priority configuration files with
more specific high-priority configuration files, and does not apply
to the extensions.* stuff that are by definition repository
specific.


[Footnote]

As I said, this does not apply to the topic of this discussion, but
just for completeness:

 * comma separated list allows overriding everything that was said
   earlier wholesale; there is no ambiguity, which is a plus, but
   there is no incremental updates, which may be a minus when
   flexibility is desired.

 * multi-valued configuration variable allows incremental additions,
   but ad-hoc syntax needs to be invented if incremental
   subtractions or clearing the slate to start from scratch is
   needed.


^ permalink raw reply

* [PATCH 30/30] t1016-compatObjectFormat: Add tests to verify the conversion between objects
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

For now my strategy is simple.  Create two identical repositories one
in each format.  Use fixed timestamps. Verify the dynamically computed
compatibility objects from one repository match the objects stored in
the other repository.

A general limitation of this strategy is that the git when generating
signed tags and commits with compatObjectFormat enabled will generate
a signature for both formats.  To overcome this limitation I have
added "test-tool delete-gpgsig" that when fed an signed commit or tag
with two signatures deletes one of the signatures.

With that in place I can have "git commit" and  "git tag" generate
signed objects, have my tool delete one, and feed the new object
into "git hash-object" to create the kinds of commits and tags
git without compatObjectFormat enabled will generate.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 Makefile                      |   1 +
 t/helper/test-delete-gpgsig.c |  62 ++++++++
 t/helper/test-tool.c          |   1 +
 t/helper/test-tool.h          |   1 +
 t/t1016-compatObjectFormat.sh | 280 ++++++++++++++++++++++++++++++++++
 t/t1016/gpg                   |   2 +
 6 files changed, 347 insertions(+)
 create mode 100644 t/helper/test-delete-gpgsig.c
 create mode 100755 t/t1016-compatObjectFormat.sh
 create mode 100755 t/t1016/gpg

diff --git a/Makefile b/Makefile
index 3c18664def9a..3e4444fb9ab2 100644
--- a/Makefile
+++ b/Makefile
@@ -790,6 +790,7 @@ TEST_BUILTINS_OBJS += test-crontab.o
 TEST_BUILTINS_OBJS += test-csprng.o
 TEST_BUILTINS_OBJS += test-ctype.o
 TEST_BUILTINS_OBJS += test-date.o
+TEST_BUILTINS_OBJS += test-delete-gpgsig.o
 TEST_BUILTINS_OBJS += test-delta.o
 TEST_BUILTINS_OBJS += test-dir-iterator.o
 TEST_BUILTINS_OBJS += test-drop-caches.o
diff --git a/t/helper/test-delete-gpgsig.c b/t/helper/test-delete-gpgsig.c
new file mode 100644
index 000000000000..e36831af03f6
--- /dev/null
+++ b/t/helper/test-delete-gpgsig.c
@@ -0,0 +1,62 @@
+#include "test-tool.h"
+#include "gpg-interface.h"
+#include "strbuf.h"
+
+
+int cmd__delete_gpgsig(int argc, const char **argv)
+{
+	struct strbuf buf = STRBUF_INIT;
+	const char *pattern = "gpgsig";
+	const char *bufptr, *tail, *eol;
+	int deleting = 0;
+	size_t plen;
+
+	if (argc >= 2) {
+		pattern = argv[1];
+		argv++;
+		argc--;
+	}
+
+	plen = strlen(pattern);
+	strbuf_read(&buf, 0, 0);
+
+	if (!strcmp(pattern, "trailer")) {
+		size_t payload_size = parse_signed_buffer(buf.buf, buf.len);
+		fwrite(buf.buf, 1, payload_size, stdout);
+		fflush(stdout);
+		return 0;
+	}
+
+	bufptr = buf.buf;
+	tail = bufptr + buf.len;
+
+	while (bufptr < tail) {
+		/* Find the end of the line */
+		eol = memchr(bufptr, '\n', tail - bufptr);
+		if (!eol)
+			eol = tail;
+
+		/* Drop continuation lines */
+		if (deleting && (bufptr < eol) && (bufptr[0] == ' ')) {
+			bufptr = eol + 1;
+			continue;
+		}
+		deleting = 0;
+
+		/* Does the line match the prefix? */
+		if (((bufptr + plen) < eol) &&
+		    !memcmp(bufptr, pattern, plen) &&
+		    (bufptr[plen] == ' ')) {
+			deleting = 1;
+			bufptr = eol + 1;
+			continue;
+		}
+
+		/* Print all other lines */
+		fwrite(bufptr, 1, (eol - bufptr) + 1, stdout);
+		bufptr = eol + 1;
+	}
+	fflush(stdout);
+
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index abe8a785eb65..8b6c84f202d6 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -21,6 +21,7 @@ static struct test_cmd cmds[] = {
 	{ "csprng", cmd__csprng },
 	{ "ctype", cmd__ctype },
 	{ "date", cmd__date },
+	{ "delete-gpgsig", cmd__delete_gpgsig },
 	{ "delta", cmd__delta },
 	{ "dir-iterator", cmd__dir_iterator },
 	{ "drop-caches", cmd__drop_caches },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index ea2672436c9a..76baaece35b9 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -15,6 +15,7 @@ int cmd__csprng(int argc, const char **argv);
 int cmd__ctype(int argc, const char **argv);
 int cmd__date(int argc, const char **argv);
 int cmd__delta(int argc, const char **argv);
+int cmd__delete_gpgsig(int argc, const char **argv);
 int cmd__dir_iterator(int argc, const char **argv);
 int cmd__drop_caches(int argc, const char **argv);
 int cmd__dump_cache_tree(int argc, const char **argv);
diff --git a/t/t1016-compatObjectFormat.sh b/t/t1016-compatObjectFormat.sh
new file mode 100755
index 000000000000..bb558a1d562a
--- /dev/null
+++ b/t/t1016-compatObjectFormat.sh
@@ -0,0 +1,280 @@
+#!/bin/sh
+#
+# Copyright (c) 2023 Eric Biederman
+#
+
+test_description='Test how well compatObjectFormat works'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
+
+# All of the follow variables must be defined in the environment:
+# GIT_AUTHOR_NAME
+# GIT_AUTHOR_EMAIL
+# GIT_AUTHOR_DATE
+# GIT_COMMITTER_NAME
+# GIT_COMMITTER_EMAIL
+# GIT_COMMITTER_DATE
+#
+# The test relies on these variables being set so that the two
+# different commits in two different repositories encoded with two
+# different hash functions result in the same content in the commits.
+# This means that when the commit is translated between hash functions
+# the commit is identical to the commit in the other repository.
+
+compat_hash () {
+    case "$1" in
+    "sha1")
+	echo "sha256"
+	;;
+    "sha256")
+	echo "sha1"
+	;;
+    esac
+}
+
+hello_oid () {
+    case "$1" in
+    "sha1")
+	echo "$hello_sha1_oid"
+	;;
+    "sha256")
+	echo "$hello_sha256_oid"
+	;;
+    esac
+}
+
+tree_oid () {
+    case "$1" in
+    "sha1")
+	echo "$tree_sha1_oid"
+	;;
+    "sha256")
+	echo "$tree_sha256_oid"
+	;;
+    esac
+}
+
+commit_oid () {
+    case "$1" in
+    "sha1")
+	echo "$commit_sha1_oid"
+	;;
+    "sha256")
+	echo "$commit_sha256_oid"
+	;;
+    esac
+}
+
+commit2_oid () {
+    case "$1" in
+    "sha1")
+	echo "$commit2_sha1_oid"
+	;;
+    "sha256")
+	echo "$commit2_sha256_oid"
+	;;
+    esac
+}
+
+del_sigcommit () {
+    local delete=$1
+
+    if test "$delete" = "sha256" ; then
+	local pattern="gpgsig-sha256"
+    else
+	local pattern="gpgsig"
+    fi
+    test-tool delete-gpgsig "$pattern"
+}
+
+
+del_sigtag () {
+    local storage=$1
+    local delete=$2
+
+    if test "$storage" = "$delete" ; then
+	local pattern="trailer"
+    elif test "$storage" = "sha256" ; then
+	local pattern="gpgsig"
+    else
+	local pattern="gpgsig-sha256"
+    fi
+    test-tool delete-gpgsig "$pattern"
+}
+
+base=$(pwd)
+for hash in sha1 sha256
+do
+	cd "$base"
+	mkdir -p repo-$hash
+	cd repo-$hash
+
+	test_expect_success "setup $hash repository" '
+		git init --object-format=$hash &&
+		git config core.repositoryformatversion 1 &&
+		git config extensions.objectformat $hash &&
+		git config extensions.compatobjectformat $(compat_hash $hash) &&
+		git config gpg.program $TEST_DIRECTORY/t1016/gpg &&
+		echo "Hellow World!" > hello &&
+		eval hello_${hash}_oid=$(git hash-object hello) &&
+		git update-index --add hello &&
+		git commit -m "Initial commit" &&
+		eval commit_${hash}_oid=$(git rev-parse HEAD) &&
+		eval tree_${hash}_oid=$(git rev-parse HEAD^{tree})
+	'
+	test_expect_success "create a $hash  tagged blob" '
+		git tag --no-sign -m "This is a tag" hellotag $(hello_oid $hash) &&
+		eval hellotag_${hash}_oid=$(git rev-parse hellotag)
+	'
+	test_expect_success "create a $hash tagged tree" '
+		git tag --no-sign -m "This is a tag" treetag $(tree_oid $hash) &&
+		eval treetag_${hash}_oid=$(git rev-parse treetag)
+	'
+	test_expect_success "create a $hash tagged commit" '
+		git tag --no-sign -m "This is a tag" committag $(commit_oid $hash) &&
+		eval committag_${hash}_oid=$(git rev-parse committag)
+	'
+	test_expect_success GPG2 "create a $hash signed commit" '
+		git commit --gpg-sign --allow-empty -m "This is a signed commit" &&
+		eval signedcommit_${hash}_oid=$(git rev-parse HEAD)
+	'
+	test_expect_success GPG2 "create a $hash signed tag" '
+		git tag -s -m "This is a signed tag" signedtag HEAD &&
+		eval signedtag_${hash}_oid=$(git rev-parse signedtag)
+	'
+	test_expect_success "create a $hash branch" '
+		git checkout -b branch $(commit_oid $hash) &&
+		echo "More more more give me more!" > more &&
+		eval more_${hash}_oid=$(git hash-object more) &&
+		echo "Another and another and another" > another &&
+		eval another_${hash}_oid=$(git hash-object another) &&
+		git update-index --add more another &&
+		git commit -m "Add more files!" &&
+		eval commit2_${hash}_oid=$(git rev-parse HEAD) &&
+		eval tree2_${hash}_oid=$(git rev-parse HEAD^{tree})
+	'
+	test_expect_success GPG2 "create another $hash signed tag" '
+		git tag -s -m "This is another signed tag" signedtag2 $(commit2_oid $hash) &&
+		eval signedtag2_${hash}_oid=$(git rev-parse signedtag2)
+	'
+	test_expect_success GPG2 "merge the $hash branches together" '
+		git merge -S -m "merge some signed tags together" signedtag signedtag2 &&
+		eval signedcommit2_${hash}_oid=$(git rev-parse HEAD)
+	'
+	test_expect_success GPG2 "create additional $hash signed commits" '
+		git commit --gpg-sign --allow-empty -m "This is an additional signed commit" &&
+		git cat-file commit HEAD | del_sigcommit sha256 > "../${hash}_signedcommit3" &&
+		git cat-file commit HEAD | del_sigcommit sha1 > "../${hash}_signedcommit4" &&
+		eval signedcommit3_${hash}_oid=$(git hash-object -t commit -w ../${hash}_signedcommit3) &&
+		eval signedcommit4_${hash}_oid=$(git hash-object -t commit -w ../${hash}_signedcommit4)
+	'
+	test_expect_success GPG2 "create additional $hash signed tags" '
+		git tag -s -m "This is an additional signed tag" signedtag34 HEAD &&
+		git cat-file tag signedtag34 | del_sigtag "${hash}" sha256 > ../${hash}_signedtag3 &&
+		git cat-file tag signedtag34 | del_sigtag "${hash}" sha1 > ../${hash}_signedtag4 &&
+		eval signedtag3_${hash}_oid=$(git hash-object -t tag -w ../${hash}_signedtag3) &&
+		eval signedtag4_${hash}_oid=$(git hash-object -t tag -w ../${hash}_signedtag4)
+	'
+done
+cd "$base"
+
+compare_oids () {
+    test "$#" = 5 && { local PREREQ=$1; shift; } || PREREQ=
+    local type="$1"
+    local name="$2"
+    local sha1_oid="$3"
+    local sha256_oid="$4"
+
+    echo ${sha1_oid} > ${name}_sha1_expected
+    echo ${sha256_oid} > ${name}_sha256_expected
+    echo ${type} > ${name}_type_expected
+
+    git --git-dir=repo-sha1/.git rev-parse --output-object-format=sha256 ${sha1_oid} > ${name}_sha1_sha256_found
+    git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha256_sha1_found
+    local sha1_sha256_oid=$(cat ${name}_sha1_sha256_found)
+    local sha256_sha1_oid=$(cat ${name}_sha256_sha1_found)
+
+    test_expect_success $PREREQ "Verify ${type} ${name}'s sha1 oid" '
+	git --git-dir=repo-sha256/.git rev-parse --output-object-format=sha1 ${sha256_oid} > ${name}_sha1 &&
+	test_cmp ${name}_sha1 ${name}_sha1_expected
+'
+
+    test_expect_success $PREREQ "Verify ${type} ${name}'s sha256 oid" '
+	git --git-dir=repo-sha1/.git rev-parse --output-object-format=sha256 ${sha1_oid} > ${name}_sha256 &&
+	test_cmp ${name}_sha256 ${name}_sha256_expected
+'
+
+    test_expect_success $PREREQ "Verify ${name}'s sha1 type" '
+	git --git-dir=repo-sha1/.git cat-file -t ${sha1_oid} > ${name}_type1 &&
+	git --git-dir=repo-sha256/.git cat-file -t ${sha256_sha1_oid} > ${name}_type2 &&
+	test_cmp ${name}_type1 ${name}_type2 &&
+	test_cmp ${name}_type1 ${name}_type_expected
+'
+
+    test_expect_success $PREREQ "Verify ${name}'s sha256 type" '
+	git --git-dir=repo-sha256/.git cat-file -t ${sha256_oid} > ${name}_type3 &&
+	git --git-dir=repo-sha1/.git cat-file -t ${sha1_sha256_oid} > ${name}_type4 &&
+	test_cmp ${name}_type3 ${name}_type4 &&
+	test_cmp ${name}_type3 ${name}_type_expected
+'
+
+    test_expect_success $PREREQ "Verify ${name}'s sha1 size" '
+	git --git-dir=repo-sha1/.git cat-file -s ${sha1_oid} > ${name}_size1 &&
+	git --git-dir=repo-sha256/.git cat-file -s ${sha256_sha1_oid} > ${name}_size2 &&
+	test_cmp ${name}_size1 ${name}_size2
+'
+
+    test_expect_success $PREREQ "Verify ${name}'s sha256 size" '
+	git --git-dir=repo-sha256/.git cat-file -s ${sha256_oid} > ${name}_size3 &&
+	git --git-dir=repo-sha1/.git cat-file -s ${sha1_sha256_oid} > ${name}_size4 &&
+	test_cmp ${name}_size3 ${name}_size4
+'
+
+    test_expect_success $PREREQ "Verify ${name}'s sha1 pretty content" '
+	git --git-dir=repo-sha1/.git cat-file -p ${sha1_oid} > ${name}_content1 &&
+	git --git-dir=repo-sha256/.git cat-file -p ${sha256_sha1_oid} > ${name}_content2 &&
+	test_cmp ${name}_content1 ${name}_content2
+'
+
+    test_expect_success $PREREQ "Verify ${name}'s sha256 pretty content" '
+	git --git-dir=repo-sha256/.git cat-file -p ${sha256_oid} > ${name}_content3 &&
+	git --git-dir=repo-sha1/.git cat-file -p ${sha1_sha256_oid} > ${name}_content4 &&
+	test_cmp ${name}_content3 ${name}_content4
+'
+
+    test_expect_success $PREREQ "Verify ${name}'s sha1 content" '
+	git --git-dir=repo-sha1/.git cat-file ${type} ${sha1_oid} > ${name}_content5 &&
+	git --git-dir=repo-sha256/.git cat-file ${type} ${sha256_sha1_oid} > ${name}_content6 &&
+	test_cmp ${name}_content5 ${name}_content6
+'
+
+    test_expect_success $PREREQ "Verify ${name}'s sha256 content" '
+	git --git-dir=repo-sha256/.git cat-file ${type} ${sha256_oid} > ${name}_content7 &&
+	git --git-dir=repo-sha1/.git cat-file ${type} ${sha1_sha256_oid} > ${name}_content8 &&
+	test_cmp ${name}_content7 ${name}_content8
+'
+
+}
+
+compare_oids 'blob' hello "$hello_sha1_oid" "$hello_sha256_oid"
+compare_oids 'tree' tree "$tree_sha1_oid" "$tree_sha256_oid"
+compare_oids 'commit' commit "$commit_sha1_oid" "$commit_sha256_oid"
+compare_oids GPG2 'commit' signedcommit "$signedcommit_sha1_oid" "$signedcommit_sha256_oid"
+compare_oids 'tag' hellotag "$hellotag_sha1_oid" "$hellotag_sha256_oid"
+compare_oids 'tag' treetag "$treetag_sha1_oid" "$treetag_sha256_oid"
+compare_oids 'tag' committag "$committag_sha1_oid" "$committag_sha256_oid"
+compare_oids GPG2 'tag' signedtag "$signedtag_sha1_oid" "$signedtag_sha256_oid"
+
+compare_oids 'blob' more "$more_sha1_oid" "$more_sha256_oid"
+compare_oids 'blob' another "$another_sha1_oid" "$another_sha256_oid"
+compare_oids 'tree' tree2 "$tree2_sha1_oid" "$tree2_sha256_oid"
+compare_oids 'commit' commit2 "$commit2_sha1_oid" "$commit2_sha256_oid"
+compare_oids GPG2 'tag' signedtag2 "$signedtag2_sha1_oid" "$signedtag2_sha256_oid"
+compare_oids GPG2 'commit' signedcommit2 "$signedcommit2_sha1_oid" "$signedcommit2_sha256_oid"
+compare_oids GPG2 'commit' signedcommit3 "$signedcommit3_sha1_oid" "$signedcommit3_sha256_oid"
+compare_oids GPG2 'commit' signedcommit4 "$signedcommit4_sha1_oid" "$signedcommit4_sha256_oid"
+compare_oids GPG2 'tag' signedtag3 "$signedtag3_sha1_oid" "$signedtag3_sha256_oid"
+compare_oids GPG2 'tag' signedtag4 "$signedtag4_sha1_oid" "$signedtag4_sha256_oid"
+
+test_done
diff --git a/t/t1016/gpg b/t/t1016/gpg
new file mode 100755
index 000000000000..2601cb18a5b3
--- /dev/null
+++ b/t/t1016/gpg
@@ -0,0 +1,2 @@
+#!/bin/sh
+exec gpg --faked-system-time "20230918T154812" "$@"
-- 
2.41.0


^ permalink raw reply related

* [PATCH 28/30] t1006: Rename sha1 to oid
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

Before I extend this test, changing the naming of the relevant
hash from sha1 to oid.  Calling the hash sha1 is incorrect today
as it can be either sha1 or sha256 depending on the value of
GIT_DEFAULT_HASH_FUNCTION when the test is called.

I plan to test sha1 and sha256 simultaneously in the same repository.
Having a name like sha1 will be even more confusing.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 t/t1006-cat-file.sh | 220 ++++++++++++++++++++++----------------------
 1 file changed, 110 insertions(+), 110 deletions(-)

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index d73a0be1b9d1..9b018b538950 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -112,65 +112,65 @@ strlen () {
 
 run_tests () {
     type=$1
-    sha1=$2
+    oid=$2
     size=$3
     content=$4
     pretty_content=$5
 
-    batch_output="$sha1 $type $size
+    batch_output="$oid $type $size
 $content"
 
     test_expect_success "$type exists" '
-	git cat-file -e $sha1
+	git cat-file -e $oid
     '
 
     test_expect_success "Type of $type is correct" '
 	echo $type >expect &&
-	git cat-file -t $sha1 >actual &&
+	git cat-file -t $oid >actual &&
 	test_cmp expect actual
     '
 
     test_expect_success "Size of $type is correct" '
 	echo $size >expect &&
-	git cat-file -s $sha1 >actual &&
+	git cat-file -s $oid >actual &&
 	test_cmp expect actual
     '
 
     test_expect_success "Type of $type is correct using --allow-unknown-type" '
 	echo $type >expect &&
-	git cat-file -t --allow-unknown-type $sha1 >actual &&
+	git cat-file -t --allow-unknown-type $oid >actual &&
 	test_cmp expect actual
     '
 
     test_expect_success "Size of $type is correct using --allow-unknown-type" '
 	echo $size >expect &&
-	git cat-file -s --allow-unknown-type $sha1 >actual &&
+	git cat-file -s --allow-unknown-type $oid >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 $sha1 >actual &&
+	git cat-file $type $oid >actual &&
 	test_cmp expect actual
     '
 
     test_expect_success "Pretty content of $type is correct" '
 	echo_without_newline "$pretty_content" >expect &&
-	git cat-file -p $sha1 >actual &&
+	git cat-file -p $oid >actual &&
 	test_cmp expect actual
     '
 
     test -z "$content" ||
     test_expect_success "--batch output of $type is correct" '
 	echo "$batch_output" >expect &&
-	echo $sha1 | git cat-file --batch >actual &&
+	echo $oid | git cat-file --batch >actual &&
 	test_cmp expect actual
     '
 
     test_expect_success "--batch-check output of $type is correct" '
-	echo "$sha1 $type $size" >expect &&
-	echo_without_newline $sha1 | git cat-file --batch-check >actual &&
+	echo "$oid $type $size" >expect &&
+	echo_without_newline $oid | git cat-file --batch-check >actual &&
 	test_cmp expect actual
     '
 
@@ -179,33 +179,33 @@ $content"
 	test -z "$content" ||
 		test_expect_success "--batch-command $opt output of $type content is correct" '
 		echo "$batch_output" >expect &&
-		test_write_lines "contents $sha1" | git cat-file --batch-command $opt >actual &&
+		test_write_lines "contents $oid" | git cat-file --batch-command $opt >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" |
+		echo "$oid $type $size" >expect &&
+		test_write_lines "info $oid" |
 		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 &&
+	echo "$type $oid" >expect &&
+	echo $oid | git cat-file --batch-check="%(objecttype) %(objectname)" >actual &&
 	test_cmp expect actual
     '
 
     test_expect_success "custom --batch-command format" '
-	echo "$type $sha1" >expect &&
-	echo "info $sha1" | git cat-file --batch-command="%(objecttype) %(objectname)" >actual &&
+	echo "$type $oid" >expect &&
+	echo "info $oid" | git cat-file --batch-command="%(objecttype) %(objectname)" >actual &&
 	test_cmp expect actual
     '
 
     test_expect_success '--batch-check with %(rest)' '
 	echo "$type this is some extra content" >expect &&
-	echo "$sha1    this is some extra content" |
+	echo "$oid    this is some extra content" |
 		git cat-file --batch-check="%(objecttype) %(rest)" >actual &&
 	test_cmp expect actual
     '
@@ -216,7 +216,7 @@ $content"
 		echo "$size" &&
 		echo "$content"
 	} >expect &&
-	echo $sha1 | git cat-file --batch="%(objectsize)" >actual &&
+	echo $oid | git cat-file --batch="%(objectsize)" >actual &&
 	test_cmp expect actual
     '
 
@@ -226,25 +226,25 @@ $content"
 		echo "$type" &&
 		echo "$content"
 	} >expect &&
-	echo $sha1 | git cat-file --batch="%(objecttype)" >actual &&
+	echo $oid | git cat-file --batch="%(objecttype)" >actual &&
 	test_cmp expect actual
     '
 }
 
 hello_content="Hello World"
 hello_size=$(strlen "$hello_content")
-hello_sha1=$(echo_without_newline "$hello_content" | git hash-object --stdin)
+hello_oid=$(echo_without_newline "$hello_content" | git hash-object --stdin)
 
 test_expect_success "setup" '
 	echo_without_newline "$hello_content" > hello &&
 	git update-index --add hello
 '
 
-run_tests 'blob' $hello_sha1 $hello_size "$hello_content" "$hello_content"
+run_tests 'blob' $hello_oid $hello_size "$hello_content" "$hello_content"
 
 test_expect_success '--batch-command --buffer with flush for blob info' '
-	echo "$hello_sha1 blob $hello_size" >expect &&
-	test_write_lines "info $hello_sha1" "flush" |
+	echo "$hello_oid blob $hello_size" >expect &&
+	test_write_lines "info $hello_oid" "flush" |
 	GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT=1 \
 	git cat-file --batch-command --buffer >actual &&
 	test_cmp expect actual
@@ -252,38 +252,38 @@ test_expect_success '--batch-command --buffer with flush for blob info' '
 
 test_expect_success '--batch-command --buffer without flush for blob info' '
 	touch output &&
-	test_write_lines "info $hello_sha1" |
+	test_write_lines "info $hello_oid" |
 	GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT=1 \
 	git cat-file --batch-command --buffer >>output &&
 	test_must_be_empty output
 '
 
 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" &&
+	echo "$hello_oid blob $hello_size" >expect &&
+	git update-index --add --cacheinfo 100644 $hello_oid "white space" &&
 	test_when_finished "git update-index --remove \"white space\"" &&
 	echo ":white space" | git cat-file --batch-check >actual &&
 	test_cmp expect actual
 '
 
-tree_sha1=$(git write-tree)
+tree_oid=$(git write-tree)
 tree_size=$(($(test_oid rawsz) + 13))
-tree_pretty_content="100644 blob $hello_sha1	hello${LF}"
+tree_pretty_content="100644 blob $hello_oid	hello${LF}"
 
-run_tests 'tree' $tree_sha1 $tree_size "" "$tree_pretty_content"
+run_tests 'tree' $tree_oid $tree_size "" "$tree_pretty_content"
 
 commit_message="Initial commit"
-commit_sha1=$(echo_without_newline "$commit_message" | git commit-tree $tree_sha1)
+commit_oid=$(echo_without_newline "$commit_message" | git commit-tree $tree_oid)
 commit_size=$(($(test_oid hexsz) + 137))
-commit_content="tree $tree_sha1
+commit_content="tree $tree_oid
 author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE
 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
 
 $commit_message"
 
-run_tests 'commit' $commit_sha1 $commit_size "$commit_content" "$commit_content"
+run_tests 'commit' $commit_oid $commit_size "$commit_content" "$commit_content"
 
-tag_header_without_timestamp="object $hello_sha1
+tag_header_without_timestamp="object $hello_oid
 type blob
 tag hellotag
 tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
@@ -292,14 +292,14 @@ tag_content="$tag_header_without_timestamp 0 +0000
 
 $tag_description"
 
-tag_sha1=$(echo_without_newline "$tag_content" | git hash-object -t tag --stdin -w)
+tag_oid=$(echo_without_newline "$tag_content" | git hash-object -t tag --stdin -w)
 tag_size=$(strlen "$tag_content")
 
-run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_content"
+run_tests 'tag' $tag_oid $tag_size "$tag_content" "$tag_content"
 
 test_expect_success "Reach a blob from a tag pointing to it" '
 	echo_without_newline "$hello_content" >expect &&
-	git cat-file blob $tag_sha1 >actual &&
+	git cat-file blob $tag_oid >actual &&
 	test_cmp expect actual
 '
 
@@ -308,31 +308,31 @@ do
     for opt in t s e p
     do
 	test_expect_success "Passing -$opt with --$batch fails" '
-	    test_must_fail git cat-file --$batch -$opt $hello_sha1
+	    test_must_fail git cat-file --$batch -$opt $hello_oid
 	'
 
 	test_expect_success "Passing --$batch with -$opt fails" '
-	    test_must_fail git cat-file -$opt --$batch $hello_sha1
+	    test_must_fail git cat-file -$opt --$batch $hello_oid
 	'
     done
 
     test_expect_success "Passing <type> with --$batch fails" '
-	test_must_fail git cat-file --$batch blob $hello_sha1
+	test_must_fail git cat-file --$batch blob $hello_oid
     '
 
     test_expect_success "Passing --$batch with <type> fails" '
-	test_must_fail git cat-file blob --$batch $hello_sha1
+	test_must_fail git cat-file blob --$batch $hello_oid
     '
 
-    test_expect_success "Passing sha1 with --$batch fails" '
-	test_must_fail git cat-file --$batch $hello_sha1
+    test_expect_success "Passing oid with --$batch fails" '
+	test_must_fail git cat-file --$batch $hello_oid
     '
 done
 
 for opt in t s e p
 do
     test_expect_success "Passing -$opt with --follow-symlinks fails" '
-	    test_must_fail git cat-file --follow-symlinks -$opt $hello_sha1
+	    test_must_fail git cat-file --follow-symlinks -$opt $hello_oid
 	'
 done
 
@@ -360,12 +360,12 @@ test_expect_success "--batch-check for a non-existent hash" '
 
 test_expect_success "--batch for an existent and a non-existent hash" '
 	cat >expect <<-EOF &&
-	$tag_sha1 tag $tag_size
+	$tag_oid tag $tag_size
 	$tag_content
 	0000000000000000000000000000000000000000 missing
 	EOF
 
-	printf "$tag_sha1\n0000000000000000000000000000000000000000" >in &&
+	printf "$tag_oid\n0000000000000000000000000000000000000000" >in &&
 	git cat-file --batch <in >actual &&
 	test_cmp expect actual
 '
@@ -386,74 +386,74 @@ test_expect_success 'empty --batch-check notices missing object' '
 	test_cmp expect actual
 '
 
-batch_input="$hello_sha1
-$commit_sha1
-$tag_sha1
+batch_input="$hello_oid
+$commit_oid
+$tag_oid
 deadbeef
 
 "
 
 printf "%s\0" \
-	"$hello_sha1 blob $hello_size" \
+	"$hello_oid blob $hello_size" \
 	"$hello_content" \
-	"$commit_sha1 commit $commit_size" \
+	"$commit_oid commit $commit_size" \
 	"$commit_content" \
-	"$tag_sha1 tag $tag_size" \
+	"$tag_oid tag $tag_size" \
 	"$tag_content" \
 	"deadbeef missing" \
 	" missing" >batch_output
 
-test_expect_success '--batch with multiple sha1s gives correct format' '
+test_expect_success '--batch with multiple oids gives correct format' '
 	tr "\0" "\n" <batch_output >expect &&
 	echo_without_newline "$batch_input" >in &&
 	git cat-file --batch <in >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success '--batch, -z with multiple sha1s gives correct format' '
+test_expect_success '--batch, -z with multiple oids gives correct format' '
 	echo_without_newline_nul "$batch_input" >in &&
 	tr "\0" "\n" <batch_output >expect &&
 	git cat-file --batch -z <in >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success '--batch, -Z with multiple sha1s gives correct format' '
+test_expect_success '--batch, -Z with multiple oids gives correct format' '
 	echo_without_newline_nul "$batch_input" >in &&
 	git cat-file --batch -Z <in >actual &&
 	test_cmp batch_output actual
 '
 
-batch_check_input="$hello_sha1
-$tree_sha1
-$commit_sha1
-$tag_sha1
+batch_check_input="$hello_oid
+$tree_oid
+$commit_oid
+$tag_oid
 deadbeef
 
 "
 
 printf "%s\0" \
-	"$hello_sha1 blob $hello_size" \
-	"$tree_sha1 tree $tree_size" \
-	"$commit_sha1 commit $commit_size" \
-	"$tag_sha1 tag $tag_size" \
+	"$hello_oid blob $hello_size" \
+	"$tree_oid tree $tree_size" \
+	"$commit_oid commit $commit_size" \
+	"$tag_oid tag $tag_size" \
 	"deadbeef missing" \
 	" missing" >batch_check_output
 
-test_expect_success "--batch-check with multiple sha1s gives correct format" '
+test_expect_success "--batch-check with multiple oids gives correct format" '
 	tr "\0" "\n" <batch_check_output >expect &&
 	echo_without_newline "$batch_check_input" >in &&
 	git cat-file --batch-check <in >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success "--batch-check, -z with multiple sha1s gives correct format" '
+test_expect_success "--batch-check, -z with multiple oids gives correct format" '
 	tr "\0" "\n" <batch_check_output >expect &&
 	echo_without_newline_nul "$batch_check_input" >in &&
 	git cat-file --batch-check -z <in >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success "--batch-check, -Z with multiple sha1s gives correct format" '
+test_expect_success "--batch-check, -Z with multiple oids gives correct format" '
 	echo_without_newline_nul "$batch_check_input" >in &&
 	git cat-file --batch-check -Z <in >actual &&
 	test_cmp batch_check_output actual
@@ -480,18 +480,18 @@ test_expect_success FUNNYNAMES '--batch-check, -Z with newline in input' '
 	test_cmp expect actual
 '
 
-batch_command_multiple_info="info $hello_sha1
-info $tree_sha1
-info $commit_sha1
-info $tag_sha1
+batch_command_multiple_info="info $hello_oid
+info $tree_oid
+info $commit_oid
+info $tag_oid
 info deadbeef"
 
 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
+	$hello_oid blob $hello_size
+	$tree_oid tree $tree_size
+	$commit_oid commit $commit_size
+	$tag_oid tag $tag_size
 	deadbeef missing
 	EOF
 
@@ -512,19 +512,19 @@ test_expect_success '--batch-command with multiple info calls gives correct form
 	test_cmp expect_nul actual
 '
 
-batch_command_multiple_contents="contents $hello_sha1
-contents $commit_sha1
-contents $tag_sha1
+batch_command_multiple_contents="contents $hello_oid
+contents $commit_oid
+contents $tag_oid
 contents deadbeef
 flush"
 
 test_expect_success '--batch-command with multiple command calls gives correct format' '
 	printf "%s\0" \
-		"$hello_sha1 blob $hello_size" \
+		"$hello_oid blob $hello_size" \
 		"$hello_content" \
-		"$commit_sha1 commit $commit_size" \
+		"$commit_oid commit $commit_size" \
 		"$commit_content" \
-		"$tag_sha1 tag $tag_size" \
+		"$tag_oid tag $tag_size" \
 		"$tag_content" \
 		"deadbeef missing" >expect_nul &&
 	tr "\0" "\n" <expect_nul >expect &&
@@ -569,7 +569,7 @@ test_expect_success 'confirm that neither loose blob is a delta' '
 # we will check only that one of the two objects is a delta
 # against the other, but not the order. We can do so by just
 # asking for the base of both, and checking whether either
-# sha1 appears in the output.
+# oid appears in the output.
 test_expect_success '%(deltabase) reports packed delta bases' '
 	git repack -ad &&
 	git cat-file --batch-check="%(deltabase)" <blobs >actual &&
@@ -583,12 +583,12 @@ test_expect_success 'setup bogus data' '
 	bogus_short_type="bogus" &&
 	bogus_short_content="bogus" &&
 	bogus_short_size=$(strlen "$bogus_short_content") &&
-	bogus_short_sha1=$(echo_without_newline "$bogus_short_content" | git hash-object -t $bogus_short_type --literally -w --stdin) &&
+	bogus_short_oid=$(echo_without_newline "$bogus_short_content" | git hash-object -t $bogus_short_type --literally -w --stdin) &&
 
 	bogus_long_type="abcdefghijklmnopqrstuvwxyz1234679" &&
 	bogus_long_content="bogus" &&
 	bogus_long_size=$(strlen "$bogus_long_content") &&
-	bogus_long_sha1=$(echo_without_newline "$bogus_long_content" | git hash-object -t $bogus_long_type --literally -w --stdin)
+	bogus_long_oid=$(echo_without_newline "$bogus_long_content" | git hash-object -t $bogus_long_type --literally -w --stdin)
 '
 
 for arg1 in '' --allow-unknown-type
@@ -608,9 +608,9 @@ do
 
 			if test "$arg1" = "--allow-unknown-type"
 			then
-				git cat-file $arg1 $arg2 $bogus_short_sha1
+				git cat-file $arg1 $arg2 $bogus_short_oid
 			else
-				test_must_fail git cat-file $arg1 $arg2 $bogus_short_sha1 >out 2>actual &&
+				test_must_fail git cat-file $arg1 $arg2 $bogus_short_oid >out 2>actual &&
 				test_must_be_empty out &&
 				test_cmp expect actual
 			fi
@@ -620,21 +620,21 @@ do
 			if test "$arg2" = "-p"
 			then
 				cat >expect <<-EOF
-				error: header for $bogus_long_sha1 too long, exceeds 32 bytes
-				fatal: Not a valid object name $bogus_long_sha1
+				error: header for $bogus_long_oid too long, exceeds 32 bytes
+				fatal: Not a valid object name $bogus_long_oid
 				EOF
 			else
 				cat >expect <<-EOF
-				error: header for $bogus_long_sha1 too long, exceeds 32 bytes
+				error: header for $bogus_long_oid too long, exceeds 32 bytes
 				fatal: git cat-file: could not get object info
 				EOF
 			fi &&
 
 			if test "$arg1" = "--allow-unknown-type"
 			then
-				git cat-file $arg1 $arg2 $bogus_short_sha1
+				git cat-file $arg1 $arg2 $bogus_short_oid
 			else
-				test_must_fail git cat-file $arg1 $arg2 $bogus_long_sha1 >out 2>actual &&
+				test_must_fail git cat-file $arg1 $arg2 $bogus_long_oid >out 2>actual &&
 				test_must_be_empty out &&
 				test_cmp expect actual
 			fi
@@ -668,28 +668,28 @@ do
 done
 
 test_expect_success '-e is OK with a broken object without --allow-unknown-type' '
-	git cat-file -e $bogus_short_sha1
+	git cat-file -e $bogus_short_oid
 '
 
 test_expect_success '-e can not be combined with --allow-unknown-type' '
-	test_expect_code 128 git cat-file -e --allow-unknown-type $bogus_short_sha1
+	test_expect_code 128 git cat-file -e --allow-unknown-type $bogus_short_oid
 '
 
 test_expect_success '-p cannot print a broken object even with --allow-unknown-type' '
-	test_must_fail git cat-file -p $bogus_short_sha1 &&
-	test_expect_code 128 git cat-file -p --allow-unknown-type $bogus_short_sha1
+	test_must_fail git cat-file -p $bogus_short_oid &&
+	test_expect_code 128 git cat-file -p --allow-unknown-type $bogus_short_oid
 '
 
 test_expect_success '<type> <hash> does not work with objects of broken types' '
 	cat >err.expect <<-\EOF &&
 	fatal: invalid object type "bogus"
 	EOF
-	test_must_fail git cat-file $bogus_short_type $bogus_short_sha1 2>err.actual &&
+	test_must_fail git cat-file $bogus_short_type $bogus_short_oid 2>err.actual &&
 	test_cmp err.expect err.actual
 '
 
 test_expect_success 'broken types combined with --batch and --batch-check' '
-	echo $bogus_short_sha1 >bogus-oid &&
+	echo $bogus_short_oid >bogus-oid &&
 
 	cat >err.expect <<-\EOF &&
 	fatal: invalid object type
@@ -711,52 +711,52 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme
 	cat >expect <<-EOF &&
 	$bogus_short_type
 	EOF
-	git cat-file -t --allow-unknown-type $bogus_short_sha1 >actual &&
+	git cat-file -t --allow-unknown-type $bogus_short_oid >actual &&
 	test_cmp expect actual &&
 
 	# Create it manually, as "git replace" will die on bogus
 	# types.
 	head=$(git rev-parse --verify HEAD) &&
-	test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" &&
-	test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID REF_SKIP_OID_VERIFICATION &&
+	test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_oid" &&
+	test-tool ref-store main update-ref msg "refs/replace/$bogus_short_oid" $head $ZERO_OID REF_SKIP_OID_VERIFICATION &&
 
 	cat >expect <<-EOF &&
 	commit
 	EOF
-	git cat-file -t --allow-unknown-type $bogus_short_sha1 >actual &&
+	git cat-file -t --allow-unknown-type $bogus_short_oid >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success "Type of broken object is correct" '
 	echo $bogus_short_type >expect &&
-	git cat-file -t --allow-unknown-type $bogus_short_sha1 >actual &&
+	git cat-file -t --allow-unknown-type $bogus_short_oid >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success "Size of broken object is correct" '
 	echo $bogus_short_size >expect &&
-	git cat-file -s --allow-unknown-type $bogus_short_sha1 >actual &&
+	git cat-file -s --allow-unknown-type $bogus_short_oid >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'clean up broken object' '
-	rm .git/objects/$(test_oid_to_path $bogus_short_sha1)
+	rm .git/objects/$(test_oid_to_path $bogus_short_oid)
 '
 
 test_expect_success "Type of broken object is correct when type is large" '
 	echo $bogus_long_type >expect &&
-	git cat-file -t --allow-unknown-type $bogus_long_sha1 >actual &&
+	git cat-file -t --allow-unknown-type $bogus_long_oid >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success "Size of large broken object is correct when type is large" '
 	echo $bogus_long_size >expect &&
-	git cat-file -s --allow-unknown-type $bogus_long_sha1 >actual &&
+	git cat-file -s --allow-unknown-type $bogus_long_oid >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'clean up broken object' '
-	rm .git/objects/$(test_oid_to_path $bogus_long_sha1)
+	rm .git/objects/$(test_oid_to_path $bogus_long_oid)
 '
 
 test_expect_success 'cat-file -t and -s on corrupt loose object' '
@@ -853,7 +853,7 @@ test_expect_success 'prep for symlink tests' '
 	test_ln_s_add loop2 loop1 &&
 	git add morx dir/subdir/ind2 dir/ind1 &&
 	git commit -am "test" &&
-	echo $hello_sha1 blob $hello_size >found
+	echo $hello_oid blob $hello_size >found
 '
 
 test_expect_success 'git cat-file --batch-check --follow-symlinks works for non-links' '
@@ -941,7 +941,7 @@ test_expect_success 'git cat-file --batch-check --follow-symlinks works for dir/
 	echo HEAD:dirlink/morx >>expect &&
 	echo HEAD:dirlink/morx | git cat-file --batch-check --follow-symlinks >actual &&
 	test_cmp expect actual &&
-	echo $hello_sha1 blob $hello_size >expect &&
+	echo $hello_oid blob $hello_size >expect &&
 	echo HEAD:dirlink/ind1 | git cat-file --batch-check --follow-symlinks >actual &&
 	test_cmp expect actual
 '
-- 
2.41.0


^ permalink raw reply related

* [PATCH 29/30] t1006: Test oid compatibility with cat-file
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

Update the existing tests that are oid based to test that cat-file
works correctly with the normal oid and the compat_oid.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 t/t1006-cat-file.sh | 251 +++++++++++++++++++++++++++-----------------
 1 file changed, 154 insertions(+), 97 deletions(-)

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 9b018b538950..23d3d37283bb 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -236,27 +236,38 @@ hello_size=$(strlen "$hello_content")
 hello_oid=$(echo_without_newline "$hello_content" | git hash-object --stdin)
 
 test_expect_success "setup" '
+	git config core.repositoryformatversion 1 &&
+	git config extensions.objectformat $test_hash_algo &&
+	git config extensions.compatobjectformat $test_compat_hash_algo &&
 	echo_without_newline "$hello_content" > hello &&
 	git update-index --add hello
 '
 
-run_tests 'blob' $hello_oid $hello_size "$hello_content" "$hello_content"
+run_blob_tests () {
+    oid=$1
 
-test_expect_success '--batch-command --buffer with flush for blob info' '
-	echo "$hello_oid blob $hello_size" >expect &&
-	test_write_lines "info $hello_oid" "flush" |
+    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 &&
+	test_write_lines "info $oid" "flush" |
 	GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT=1 \
 	git cat-file --batch-command --buffer >actual &&
 	test_cmp expect actual
-'
+    '
 
-test_expect_success '--batch-command --buffer without flush for blob info' '
+    test_expect_success '--batch-command --buffer without flush for blob info' '
 	touch output &&
-	test_write_lines "info $hello_oid" |
+	test_write_lines "info $oid" |
 	GIT_TEST_CAT_FILE_NO_FLUSH_ON_EXIT=1 \
 	git cat-file --batch-command --buffer >>output &&
 	test_must_be_empty output
-'
+    '
+}
+
+hello_compat_oid=$(git rev-parse --output-object-format=$test_compat_hash_algo $hello_oid)
+run_blob_tests $hello_oid
+run_blob_tests $hello_compat_oid
 
 test_expect_success '--batch-check without %(rest) considers whole line' '
 	echo "$hello_oid blob $hello_size" >expect &&
@@ -267,35 +278,58 @@ 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"
 
 commit_message="Initial commit"
 commit_oid=$(echo_without_newline "$commit_message" | git commit-tree $tree_oid)
+commit_compat_oid=$(git rev-parse --output-object-format=$test_compat_hash_algo $commit_oid)
 commit_size=$(($(test_oid hexsz) + 137))
+commit_compat_size=$(($(test_oid --hash=compat hexsz) + 137))
 commit_content="tree $tree_oid
 author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE
 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
 
 $commit_message"
 
+commit_compat_content="tree $tree_compat_oid
+author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE
+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"
 
-tag_header_without_timestamp="object $hello_oid
-type blob
+tag_header_without_oid="type blob
 tag hellotag
 tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+tag_header_without_timestamp="object $hello_oid
+$tag_header_without_oid"
+tag_compat_header_without_timestamp="object $hello_compat_oid
+$tag_header_without_oid"
 tag_description="This is a tag"
 tag_content="$tag_header_without_timestamp 0 +0000
 
+$tag_description"
+tag_compat_content="$tag_compat_header_without_timestamp 0 +0000
+
 $tag_description"
 
 tag_oid=$(echo_without_newline "$tag_content" | git hash-object -t tag --stdin -w)
 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"
 
 test_expect_success "Reach a blob from a tag pointing to it" '
 	echo_without_newline "$hello_content" >expect &&
@@ -303,37 +337,43 @@ test_expect_success "Reach a blob from a tag pointing to it" '
 	test_cmp expect actual
 '
 
-for batch in batch batch-check batch-command
+for oid in $hello_oid $hello_compat_oid
 do
-    for opt in t s e p
+    for batch in batch batch-check batch-command
     do
+	for opt in t s e p
+	do
 	test_expect_success "Passing -$opt with --$batch fails" '
-	    test_must_fail git cat-file --$batch -$opt $hello_oid
+	    test_must_fail git cat-file --$batch -$opt $oid
 	'
 
 	test_expect_success "Passing --$batch with -$opt fails" '
-	    test_must_fail git cat-file -$opt --$batch $hello_oid
+	    test_must_fail git cat-file -$opt --$batch $oid
 	'
-    done
+	done
 
-    test_expect_success "Passing <type> with --$batch fails" '
-	test_must_fail git cat-file --$batch blob $hello_oid
-    '
+	test_expect_success "Passing <type> with --$batch fails" '
+	test_must_fail git cat-file --$batch blob $oid
+	'
 
-    test_expect_success "Passing --$batch with <type> fails" '
-	test_must_fail git cat-file blob --$batch $hello_oid
-    '
+	test_expect_success "Passing --$batch with <type> fails" '
+	test_must_fail git cat-file blob --$batch $oid
+	'
 
-    test_expect_success "Passing oid with --$batch fails" '
-	test_must_fail git cat-file --$batch $hello_oid
-    '
+	test_expect_success "Passing oid with --$batch fails" '
+	test_must_fail git cat-file --$batch $oid
+	'
+    done
 done
 
-for opt in t s e p
+for oid in $hello_oid $hello_compat_oid
 do
-    test_expect_success "Passing -$opt with --follow-symlinks fails" '
-	    test_must_fail git cat-file --follow-symlinks -$opt $hello_oid
+    for opt in t s e p
+    do
+	test_expect_success "Passing -$opt with --follow-symlinks fails" '
+	    test_must_fail git cat-file --follow-symlinks -$opt $oid
 	'
+    done
 done
 
 test_expect_success "--batch-check for a non-existent named object" '
@@ -386,112 +426,102 @@ test_expect_success 'empty --batch-check notices missing object' '
 	test_cmp expect actual
 '
 
-batch_input="$hello_oid
-$commit_oid
-$tag_oid
+batch_tests () {
+    boid=$1
+    loid=$2
+    lsize=$3
+    coid=$4
+    csize=$5
+    ccontent=$6
+    toid=$7
+    tsize=$8
+    tcontent=$9
+
+    batch_input="$boid
+$coid
+$toid
 deadbeef
 
 "
 
-printf "%s\0" \
-	"$hello_oid blob $hello_size" \
+    printf "%s\0" \
+	"$boid blob $hello_size" \
 	"$hello_content" \
-	"$commit_oid commit $commit_size" \
-	"$commit_content" \
-	"$tag_oid tag $tag_size" \
-	"$tag_content" \
+	"$coid commit $csize" \
+	"$ccontent" \
+	"$toid tag $tsize" \
+	"$tcontent" \
 	"deadbeef missing" \
 	" missing" >batch_output
 
-test_expect_success '--batch with multiple oids gives correct format' '
+    test_expect_success '--batch with multiple oids gives correct format' '
 	tr "\0" "\n" <batch_output >expect &&
 	echo_without_newline "$batch_input" >in &&
 	git cat-file --batch <in >actual &&
 	test_cmp expect actual
-'
+    '
 
-test_expect_success '--batch, -z with multiple oids gives correct format' '
+    test_expect_success '--batch, -z with multiple oids gives correct format' '
 	echo_without_newline_nul "$batch_input" >in &&
 	tr "\0" "\n" <batch_output >expect &&
 	git cat-file --batch -z <in >actual &&
 	test_cmp expect actual
-'
+    '
 
-test_expect_success '--batch, -Z with multiple oids gives correct format' '
+    test_expect_success '--batch, -Z with multiple oids gives correct format' '
 	echo_without_newline_nul "$batch_input" >in &&
 	git cat-file --batch -Z <in >actual &&
 	test_cmp batch_output actual
-'
+    '
 
-batch_check_input="$hello_oid
-$tree_oid
-$commit_oid
-$tag_oid
+batch_check_input="$boid
+$loid
+$coid
+$toid
 deadbeef
 
 "
 
-printf "%s\0" \
-	"$hello_oid blob $hello_size" \
-	"$tree_oid tree $tree_size" \
-	"$commit_oid commit $commit_size" \
-	"$tag_oid tag $tag_size" \
+    printf "%s\0" \
+	"$boid blob $hello_size" \
+	"$loid tree $lsize" \
+	"$coid commit $csize" \
+	"$toid tag $tsize" \
 	"deadbeef missing" \
 	" missing" >batch_check_output
 
-test_expect_success "--batch-check with multiple oids gives correct format" '
+    test_expect_success "--batch-check with multiple oids gives correct format" '
 	tr "\0" "\n" <batch_check_output >expect &&
 	echo_without_newline "$batch_check_input" >in &&
 	git cat-file --batch-check <in >actual &&
 	test_cmp expect actual
-'
+    '
 
-test_expect_success "--batch-check, -z with multiple oids gives correct format" '
+    test_expect_success "--batch-check, -z with multiple oids gives correct format" '
 	tr "\0" "\n" <batch_check_output >expect &&
 	echo_without_newline_nul "$batch_check_input" >in &&
 	git cat-file --batch-check -z <in >actual &&
 	test_cmp expect actual
-'
+    '
 
-test_expect_success "--batch-check, -Z with multiple oids gives correct format" '
+    test_expect_success "--batch-check, -Z with multiple oids gives correct format" '
 	echo_without_newline_nul "$batch_check_input" >in &&
 	git cat-file --batch-check -Z <in >actual &&
 	test_cmp batch_check_output actual
-'
-
-test_expect_success FUNNYNAMES 'setup with newline in input' '
-	touch -- "newline${LF}embedded" &&
-	git add -- "newline${LF}embedded" &&
-	git commit -m "file with newline embedded" &&
-	test_tick &&
-
-	printf "HEAD:newline${LF}embedded" >in
-'
-
-test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '
-	git cat-file --batch-check -z <in >actual &&
-	echo "$(git rev-parse "HEAD:newline${LF}embedded") blob 0" >expect &&
-	test_cmp expect actual
-'
-
-test_expect_success FUNNYNAMES '--batch-check, -Z with newline in input' '
-	git cat-file --batch-check -Z <in >actual &&
-	printf "%s\0" "$(git rev-parse "HEAD:newline${LF}embedded") blob 0" >expect &&
-	test_cmp expect actual
-'
+    '
 
-batch_command_multiple_info="info $hello_oid
-info $tree_oid
-info $commit_oid
-info $tag_oid
+batch_command_multiple_info="info $boid
+info $loid
+info $coid
+info $toid
 info deadbeef"
 
-test_expect_success '--batch-command with multiple info calls gives correct format' '
+    test_expect_success '--batch-command with multiple info calls gives correct format' '
 	cat >expect <<-EOF &&
-	$hello_oid blob $hello_size
-	$tree_oid tree $tree_size
-	$commit_oid commit $commit_size
-	$tag_oid tag $tag_size
+	$boid blob $hello_size
+	$loid tree $lsize
+	$coid commit $csize
+	$toid tag $tsize
 	deadbeef missing
 	EOF
 
@@ -510,22 +540,22 @@ test_expect_success '--batch-command with multiple info calls gives correct form
 	git cat-file --batch-command --buffer -Z <in >actual &&
 
 	test_cmp expect_nul actual
-'
+    '
 
-batch_command_multiple_contents="contents $hello_oid
-contents $commit_oid
-contents $tag_oid
+batch_command_multiple_contents="contents $boid
+contents $coid
+contents $toid
 contents deadbeef
 flush"
 
-test_expect_success '--batch-command with multiple command calls gives correct format' '
+    test_expect_success '--batch-command with multiple command calls gives correct format' '
 	printf "%s\0" \
-		"$hello_oid blob $hello_size" \
+		"$boid blob $hello_size" \
 		"$hello_content" \
-		"$commit_oid commit $commit_size" \
-		"$commit_content" \
-		"$tag_oid tag $tag_size" \
-		"$tag_content" \
+		"$coid commit $csize" \
+		"$ccontent" \
+		"$toid tag $tsize" \
+		"$tcontent" \
 		"deadbeef missing" >expect_nul &&
 	tr "\0" "\n" <expect_nul >expect &&
 
@@ -543,6 +573,33 @@ test_expect_success '--batch-command with multiple command calls gives correct f
 	git cat-file --batch-command --buffer -Z <in >actual &&
 
 	test_cmp expect_nul actual
+    '
+
+}
+
+batch_tests $hello_oid $tree_oid $tree_size $commit_oid $commit_size "$commit_content" $tag_oid $tag_size "$tag_content"
+batch_tests $hello_compat_oid $tree_compat_oid $tree_compat_size $commit_compat_oid $commit_compat_size "$commit_compat_content" $tag_compat_oid $tag_compat_size "$tag_compat_content"
+
+
+test_expect_success FUNNYNAMES 'setup with newline in input' '
+	touch -- "newline${LF}embedded" &&
+	git add -- "newline${LF}embedded" &&
+	git commit -m "file with newline embedded" &&
+	test_tick &&
+
+	printf "HEAD:newline${LF}embedded" >in
+'
+
+test_expect_success FUNNYNAMES '--batch-check, -z with newline in input' '
+	git cat-file --batch-check -z <in >actual &&
+	echo "$(git rev-parse "HEAD:newline${LF}embedded") blob 0" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success FUNNYNAMES '--batch-check, -Z with newline in input' '
+	git cat-file --batch-check -Z <in >actual &&
+	printf "%s\0" "$(git rev-parse "HEAD:newline${LF}embedded") blob 0" >expect &&
+	test_cmp expect actual
 '
 
 test_expect_success 'setup blobs which are likely to delta' '
-- 
2.41.0


^ permalink raw reply related

* [PATCH 27/30] test-lib: Compute the compatibility hash so tests may use it
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

Inspired-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 t/test-lib-functions.sh | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 2f8868caa171..92b462e2e711 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1599,7 +1599,16 @@ test_set_hash () {
 
 # Detect the hash algorithm in use.
 test_detect_hash () {
-	test_hash_algo="${GIT_TEST_DEFAULT_HASH:-sha1}"
+	case "$GIT_TEST_DEFAULT_HASH" in
+	"sha256")
+	    test_hash_algo=sha256
+	    test_compat_hash_algo=sha1
+	    ;;
+	*)
+	    test_hash_algo=sha1
+	    test_compat_hash_algo=sha256
+	    ;;
+	esac
 }
 
 # Load common hash metadata and common placeholder object IDs for use with
@@ -1651,6 +1660,12 @@ test_oid () {
 	local algo="${test_hash_algo}" &&
 
 	case "$1" in
+	--hash=storage)
+		algo="$test_hash_algo" &&
+		shift;;
+	--hash=compat)
+		algo="$test_compat_hash_algo" &&
+		shift;;
 	--hash=*)
 		algo="${1#--hash=}" &&
 		shift;;
-- 
2.41.0


^ permalink raw reply related

* [PATCH 24/30] tree-walk: init_tree_desc take an oid to get the hash algorithm
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

To make it possible for git ls-tree to display the tree encoded
in the hash algorithm of the oid specified to git ls-tree, update
init_tree_desc to take as a parameter the oid of the tree object.

Update all callers of init_tree_desc and init_tree_desc_gently
to pass the oid of the tree object.

Use the oid of the tree object to discover the hash algorithm
of the oid and store that hash algorithm in struct tree_desc.

Use the hash algorithm in decode_tree_entry and
update_tree_entry_internal to handle reading a tree object encoded in
a hash algorithm that differs from the repositories hash algorithm.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 archive.c              |  3 ++-
 builtin/am.c           |  6 +++---
 builtin/checkout.c     |  8 +++++---
 builtin/clone.c        |  2 +-
 builtin/commit.c       |  2 +-
 builtin/grep.c         |  8 ++++----
 builtin/merge.c        |  3 ++-
 builtin/pack-objects.c |  6 ++++--
 builtin/read-tree.c    |  2 +-
 builtin/stash.c        |  5 +++--
 cache-tree.c           |  2 +-
 delta-islands.c        |  2 +-
 diff-lib.c             |  2 +-
 fsck.c                 |  6 ++++--
 http-push.c            |  2 +-
 list-objects.c         |  2 +-
 match-trees.c          |  4 ++--
 merge-ort.c            | 11 ++++++-----
 merge-recursive.c      |  2 +-
 merge.c                |  3 ++-
 pack-bitmap-write.c    |  2 +-
 packfile.c             |  3 ++-
 reflog.c               |  2 +-
 revision.c             |  4 ++--
 tree-walk.c            | 36 +++++++++++++++++++++---------------
 tree-walk.h            |  7 +++++--
 tree.c                 |  2 +-
 walker.c               |  2 +-
 28 files changed, 80 insertions(+), 59 deletions(-)

diff --git a/archive.c b/archive.c
index ca11db185b15..b10269aee7be 100644
--- a/archive.c
+++ b/archive.c
@@ -339,7 +339,8 @@ int write_archive_entries(struct archiver_args *args,
 		opts.src_index = args->repo->index;
 		opts.dst_index = args->repo->index;
 		opts.fn = oneway_merge;
-		init_tree_desc(&t, args->tree->buffer, args->tree->size);
+		init_tree_desc(&t, &args->tree->object.oid,
+			       args->tree->buffer, args->tree->size);
 		if (unpack_trees(1, &t, &opts))
 			return -1;
 		git_attr_set_direction(GIT_ATTR_INDEX);
diff --git a/builtin/am.c b/builtin/am.c
index 8bde034fae68..4dfd714b910e 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1991,8 +1991,8 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
 	opts.reset = reset ? UNPACK_RESET_PROTECT_UNTRACKED : 0;
 	opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
 	opts.fn = twoway_merge;
-	init_tree_desc(&t[0], head->buffer, head->size);
-	init_tree_desc(&t[1], remote->buffer, remote->size);
+	init_tree_desc(&t[0], &head->object.oid, head->buffer, head->size);
+	init_tree_desc(&t[1], &remote->object.oid, remote->buffer, remote->size);
 
 	if (unpack_trees(2, t, &opts)) {
 		rollback_lock_file(&lock_file);
@@ -2026,7 +2026,7 @@ static int merge_tree(struct tree *tree)
 	opts.dst_index = &the_index;
 	opts.merge = 1;
 	opts.fn = oneway_merge;
-	init_tree_desc(&t[0], tree->buffer, tree->size);
+	init_tree_desc(&t[0], &tree->object.oid, tree->buffer, tree->size);
 
 	if (unpack_trees(1, t, &opts)) {
 		rollback_lock_file(&lock_file);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f53612f46870..03eff73fd031 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -701,7 +701,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
 			       info->commit ? &info->commit->object.oid : null_oid(),
 			       NULL);
 	parse_tree(tree);
-	init_tree_desc(&tree_desc, tree->buffer, tree->size);
+	init_tree_desc(&tree_desc, &tree->object.oid, tree->buffer, tree->size);
 	switch (unpack_trees(1, &tree_desc, &opts)) {
 	case -2:
 		*writeout_error = 1;
@@ -815,10 +815,12 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			die(_("unable to parse commit %s"),
 				oid_to_hex(old_commit_oid));
 
-		init_tree_desc(&trees[0], tree->buffer, tree->size);
+		init_tree_desc(&trees[0], &tree->object.oid,
+			       tree->buffer, tree->size);
 		parse_tree(new_tree);
 		tree = new_tree;
-		init_tree_desc(&trees[1], tree->buffer, tree->size);
+		init_tree_desc(&trees[1], &tree->object.oid,
+			       tree->buffer, tree->size);
 
 		ret = unpack_trees(2, trees, &topts);
 		clear_unpack_trees_porcelain(&topts);
diff --git a/builtin/clone.c b/builtin/clone.c
index c6357af94989..79ceefb93995 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -737,7 +737,7 @@ static int checkout(int submodule_progress, int filter_submodules)
 	if (!tree)
 		die(_("unable to parse commit %s"), oid_to_hex(&oid));
 	parse_tree(tree);
-	init_tree_desc(&t, tree->buffer, tree->size);
+	init_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);
 	if (unpack_trees(1, &t, &opts) < 0)
 		die(_("unable to checkout working tree"));
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 7da5f924484d..537319932b65 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -340,7 +340,7 @@ static void create_base_index(const struct commit *current_head)
 	if (!tree)
 		die(_("failed to unpack HEAD tree object"));
 	parse_tree(tree);
-	init_tree_desc(&t, tree->buffer, tree->size);
+	init_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);
 	if (unpack_trees(1, &t, &opts))
 		exit(128); /* We've already reported the error, finish dying */
 }
diff --git a/builtin/grep.c b/builtin/grep.c
index 50e712a18479..0c2b8a376f8e 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -530,7 +530,7 @@ static int grep_submodule(struct grep_opt *opt,
 		strbuf_addstr(&base, filename);
 		strbuf_addch(&base, '/');
 
-		init_tree_desc(&tree, data, size);
+		init_tree_desc(&tree, oid, data, size);
 		hit = grep_tree(&subopt, pathspec, &tree, &base, base.len,
 				object_type == OBJ_COMMIT);
 		strbuf_release(&base);
@@ -574,7 +574,7 @@ static int grep_cache(struct grep_opt *opt,
 
 			data = repo_read_object_file(the_repository, &ce->oid,
 						     &type, &size);
-			init_tree_desc(&tree, data, size);
+			init_tree_desc(&tree, &ce->oid, data, size);
 
 			hit |= grep_tree(opt, pathspec, &tree, &name, 0, 0);
 			strbuf_setlen(&name, name_base_len);
@@ -670,7 +670,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 				    oid_to_hex(&entry.oid));
 
 			strbuf_addch(base, '/');
-			init_tree_desc(&sub, data, size);
+			init_tree_desc(&sub, &entry.oid, data, size);
 			hit |= grep_tree(opt, pathspec, &sub, base, tn_len,
 					 check_attr);
 			free(data);
@@ -714,7 +714,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
 			strbuf_add(&base, name, len);
 			strbuf_addch(&base, ':');
 		}
-		init_tree_desc(&tree, data, size);
+		init_tree_desc(&tree, &obj->oid, data, size);
 		hit = grep_tree(opt, pathspec, &tree, &base, base.len,
 				obj->type == OBJ_COMMIT);
 		strbuf_release(&base);
diff --git a/builtin/merge.c b/builtin/merge.c
index de68910177fb..718165d45917 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -704,7 +704,8 @@ static int read_tree_trivial(struct object_id *common, struct object_id *head,
 	cache_tree_free(&the_index.cache_tree);
 	for (i = 0; i < nr_trees; i++) {
 		parse_tree(trees[i]);
-		init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
+		init_tree_desc(t+i, &trees[i]->object.oid,
+			       trees[i]->buffer, trees[i]->size);
 	}
 	if (unpack_trees(nr_trees, t, &opts))
 		return -1;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d2a162d52804..d34902002656 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1756,7 +1756,8 @@ static void add_pbase_object(struct tree_desc *tree,
 			tree = pbase_tree_get(&entry.oid);
 			if (!tree)
 				return;
-			init_tree_desc(&sub, tree->tree_data, tree->tree_size);
+			init_tree_desc(&sub, &tree->oid,
+				       tree->tree_data, tree->tree_size);
 
 			add_pbase_object(&sub, down, downlen, fullname);
 			pbase_tree_put(tree);
@@ -1816,7 +1817,8 @@ static void add_preferred_base_object(const char *name)
 		}
 		else {
 			struct tree_desc tree;
-			init_tree_desc(&tree, it->pcache.tree_data, it->pcache.tree_size);
+			init_tree_desc(&tree, &it->pcache.oid,
+				       it->pcache.tree_data, it->pcache.tree_size);
 			add_pbase_object(&tree, name, cmplen, name);
 		}
 	}
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 1fec702a04fa..24d6d156d3a2 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -264,7 +264,7 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 	for (i = 0; i < nr_trees; i++) {
 		struct tree *tree = trees[i];
 		parse_tree(tree);
-		init_tree_desc(t+i, tree->buffer, tree->size);
+		init_tree_desc(t+i, &tree->object.oid, tree->buffer, tree->size);
 	}
 	if (unpack_trees(nr_trees, t, &opts))
 		return 128;
diff --git a/builtin/stash.c b/builtin/stash.c
index fe64cde9ce30..9ee52af4d28e 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -285,7 +285,7 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
 	if (parse_tree(tree))
 		return -1;
 
-	init_tree_desc(t, tree->buffer, tree->size);
+	init_tree_desc(t, &tree->object.oid, tree->buffer, tree->size);
 
 	opts.head_idx = 1;
 	opts.src_index = &the_index;
@@ -871,7 +871,8 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op
 		tree[i] = parse_tree_indirect(oid[i]);
 		if (parse_tree(tree[i]) < 0)
 			die(_("failed to parse tree"));
-		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
+		init_tree_desc(&tree_desc[i], &tree[i]->object.oid,
+			       tree[i]->buffer, tree[i]->size);
 	}
 
 	unpack_tree_opt.head_idx = -1;
diff --git a/cache-tree.c b/cache-tree.c
index ddc7d3d86959..334973a01cee 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -770,7 +770,7 @@ static void prime_cache_tree_rec(struct repository *r,
 
 	oidcpy(&it->oid, &tree->object.oid);
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	cnt = 0;
 	while (tree_entry(&desc, &entry)) {
 		if (!S_ISDIR(entry.mode))
diff --git a/delta-islands.c b/delta-islands.c
index 5de5759f3f13..1ff3506b10f2 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -289,7 +289,7 @@ void resolve_tree_islands(struct repository *r,
 		if (!tree || parse_tree(tree) < 0)
 			die(_("bad tree object %s"), oid_to_hex(&ent->idx.oid));
 
-		init_tree_desc(&desc, tree->buffer, tree->size);
+		init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 		while (tree_entry(&desc, &entry)) {
 			struct object *obj;
 
diff --git a/diff-lib.c b/diff-lib.c
index 6b0c6a7180cc..add323f5628d 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -558,7 +558,7 @@ static int diff_cache(struct rev_info *revs,
 	opts.pathspec = &revs->diffopt.pathspec;
 	opts.pathspec->recursive = 1;
 
-	init_tree_desc(&t, tree->buffer, tree->size);
+	init_tree_desc(&t, &tree->object.oid, tree->buffer, tree->size);
 	return unpack_trees(1, &t, &opts);
 }
 
diff --git a/fsck.c b/fsck.c
index 2b1e348005b7..6b492a48da82 100644
--- a/fsck.c
+++ b/fsck.c
@@ -313,7 +313,8 @@ static int fsck_walk_tree(struct tree *tree, void *data, struct fsck_options *op
 		return -1;
 
 	name = fsck_get_object_name(options, &tree->object.oid);
-	if (init_tree_desc_gently(&desc, tree->buffer, tree->size, 0))
+	if (init_tree_desc_gently(&desc, &tree->object.oid,
+				  tree->buffer, tree->size, 0))
 		return -1;
 	while (tree_entry_gently(&desc, &entry)) {
 		struct object *obj;
@@ -583,7 +584,8 @@ static int fsck_tree(const struct object_id *tree_oid,
 	const char *o_name;
 	struct name_stack df_dup_candidates = { NULL };
 
-	if (init_tree_desc_gently(&desc, buffer, size, TREE_DESC_RAW_MODES)) {
+	if (init_tree_desc_gently(&desc, tree_oid, buffer, size,
+				  TREE_DESC_RAW_MODES)) {
 		retval += report(options, tree_oid, OBJ_TREE,
 				 FSCK_MSG_BAD_TREE,
 				 "cannot be parsed as a tree");
diff --git a/http-push.c b/http-push.c
index a704f490fdb2..81c35b5e96f7 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1308,7 +1308,7 @@ static struct object_list **process_tree(struct tree *tree,
 	obj->flags |= SEEN;
 	p = add_one_object(obj, p);
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 
 	while (tree_entry(&desc, &entry))
 		switch (object_type(entry.mode)) {
diff --git a/list-objects.c b/list-objects.c
index e60a6cd5b46e..312335c8a7f2 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -97,7 +97,7 @@ static void process_tree_contents(struct traversal_context *ctx,
 	enum interesting match = ctx->revs->diffopt.pathspec.nr == 0 ?
 		all_entries_interesting : entry_not_interesting;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 
 	while (tree_entry(&desc, &entry)) {
 		if (match != all_entries_interesting) {
diff --git a/match-trees.c b/match-trees.c
index 0885ac681cd5..3412b6a1401d 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -63,7 +63,7 @@ static void *fill_tree_desc_strict(struct tree_desc *desc,
 		die("unable to read tree (%s)", oid_to_hex(hash));
 	if (type != OBJ_TREE)
 		die("%s is not a tree", oid_to_hex(hash));
-	init_tree_desc(desc, buffer, size);
+	init_tree_desc(desc, hash, buffer, size);
 	return buffer;
 }
 
@@ -194,7 +194,7 @@ static int splice_tree(const struct object_id *oid1, const char *prefix,
 	buf = repo_read_object_file(the_repository, oid1, &type, &sz);
 	if (!buf)
 		die("cannot read tree %s", oid_to_hex(oid1));
-	init_tree_desc(&desc, buf, sz);
+	init_tree_desc(&desc, oid1, buf, sz);
 
 	rewrite_here = NULL;
 	while (desc.size) {
diff --git a/merge-ort.c b/merge-ort.c
index 8631c997002d..3a5729c91e48 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -1679,9 +1679,10 @@ static int collect_merge_info(struct merge_options *opt,
 	parse_tree(merge_base);
 	parse_tree(side1);
 	parse_tree(side2);
-	init_tree_desc(t + 0, merge_base->buffer, merge_base->size);
-	init_tree_desc(t + 1, side1->buffer, side1->size);
-	init_tree_desc(t + 2, side2->buffer, side2->size);
+	init_tree_desc(t + 0, &merge_base->object.oid,
+		       merge_base->buffer, merge_base->size);
+	init_tree_desc(t + 1, &side1->object.oid, side1->buffer, side1->size);
+	init_tree_desc(t + 2, &side2->object.oid, side2->buffer, side2->size);
 
 	trace2_region_enter("merge", "traverse_trees", opt->repo);
 	ret = traverse_trees(NULL, 3, t, &info);
@@ -4400,9 +4401,9 @@ static int checkout(struct merge_options *opt,
 	unpack_opts.fn = twoway_merge;
 	unpack_opts.preserve_ignored = 0; /* FIXME: !opts->overwrite_ignore */
 	parse_tree(prev);
-	init_tree_desc(&trees[0], prev->buffer, prev->size);
+	init_tree_desc(&trees[0], &prev->object.oid, prev->buffer, prev->size);
 	parse_tree(next);
-	init_tree_desc(&trees[1], next->buffer, next->size);
+	init_tree_desc(&trees[1], &next->object.oid, next->buffer, next->size);
 
 	ret = unpack_trees(2, trees, &unpack_opts);
 	clear_unpack_trees_porcelain(&unpack_opts);
diff --git a/merge-recursive.c b/merge-recursive.c
index 6a4081bb0f52..93df9eecdd95 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -411,7 +411,7 @@ static inline int merge_detect_rename(struct merge_options *opt)
 static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
 {
 	parse_tree(tree);
-	init_tree_desc(desc, tree->buffer, tree->size);
+	init_tree_desc(desc, &tree->object.oid, tree->buffer, tree->size);
 }
 
 static int unpack_trees_start(struct merge_options *opt,
diff --git a/merge.c b/merge.c
index b60925459c29..86179c34102d 100644
--- a/merge.c
+++ b/merge.c
@@ -81,7 +81,8 @@ int checkout_fast_forward(struct repository *r,
 	}
 	for (i = 0; i < nr_trees; i++) {
 		parse_tree(trees[i]);
-		init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
+		init_tree_desc(t+i, &trees[i]->object.oid,
+			       trees[i]->buffer, trees[i]->size);
 	}
 
 	memset(&opts, 0, sizeof(opts));
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index f6757c3cbf20..9211e08f0127 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -366,7 +366,7 @@ static int fill_bitmap_tree(struct bitmap *bitmap,
 	if (parse_tree(tree) < 0)
 		die("unable to load tree object %s",
 		    oid_to_hex(&tree->object.oid));
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 
 	while (tree_entry(&desc, &entry)) {
 		switch (object_type(entry.mode)) {
diff --git a/packfile.c b/packfile.c
index 9cc0a2e37a83..1fae0fcdd9e7 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2250,7 +2250,8 @@ static int add_promisor_object(const struct object_id *oid,
 		struct tree *tree = (struct tree *)obj;
 		struct tree_desc desc;
 		struct name_entry entry;
-		if (init_tree_desc_gently(&desc, tree->buffer, tree->size, 0))
+		if (init_tree_desc_gently(&desc, &tree->object.oid,
+					  tree->buffer, tree->size, 0))
 			/*
 			 * Error messages are given when packs are
 			 * verified, so do not print any here.
diff --git a/reflog.c b/reflog.c
index 9ad50e7d93e4..c6992a19268f 100644
--- a/reflog.c
+++ b/reflog.c
@@ -40,7 +40,7 @@ static int tree_is_complete(const struct object_id *oid)
 		tree->buffer = data;
 		tree->size = size;
 	}
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	complete = 1;
 	while (tree_entry(&desc, &entry)) {
 		if (!repo_has_object_file(the_repository, &entry.oid) ||
diff --git a/revision.c b/revision.c
index 2f4c53ea207b..a60dfc23a2a5 100644
--- a/revision.c
+++ b/revision.c
@@ -82,7 +82,7 @@ static void mark_tree_contents_uninteresting(struct repository *r,
 	if (parse_tree_gently(tree, 1) < 0)
 		return;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	while (tree_entry(&desc, &entry)) {
 		switch (object_type(entry.mode)) {
 		case OBJ_TREE:
@@ -189,7 +189,7 @@ static void add_children_by_path(struct repository *r,
 	if (parse_tree_gently(tree, 1) < 0)
 		return;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	while (tree_entry(&desc, &entry)) {
 		switch (object_type(entry.mode)) {
 		case OBJ_TREE:
diff --git a/tree-walk.c b/tree-walk.c
index 3af50a01c2c7..0b44ec7c75ff 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -15,7 +15,7 @@ static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned l
 	const char *path;
 	unsigned int len;
 	uint16_t mode;
-	const unsigned hashsz = the_hash_algo->rawsz;
+	const unsigned hashsz = desc->algo->rawsz;
 
 	if (size < hashsz + 3 || buf[size - (hashsz + 1)]) {
 		strbuf_addstr(err, _("too-short tree object"));
@@ -37,15 +37,19 @@ static int decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned l
 	desc->entry.path = path;
 	desc->entry.mode = (desc->flags & TREE_DESC_RAW_MODES) ? mode : canon_mode(mode);
 	desc->entry.pathlen = len - 1;
-	oidread(&desc->entry.oid, (const unsigned char *)path + len);
+	oidread_algop(&desc->entry.oid, (const unsigned char *)path + len,
+		      desc->algo);
 
 	return 0;
 }
 
-static int init_tree_desc_internal(struct tree_desc *desc, const void *buffer,
-				   unsigned long size, struct strbuf *err,
+static int init_tree_desc_internal(struct tree_desc *desc,
+				   const struct object_id *oid,
+				   const void *buffer, unsigned long size,
+				   struct strbuf *err,
 				   enum tree_desc_flags flags)
 {
+	desc->algo = (oid && oid->algo) ? &hash_algos[oid->algo] : the_hash_algo;
 	desc->buffer = buffer;
 	desc->size = size;
 	desc->flags = flags;
@@ -54,19 +58,21 @@ static int init_tree_desc_internal(struct tree_desc *desc, const void *buffer,
 	return 0;
 }
 
-void init_tree_desc(struct tree_desc *desc, const void *buffer, unsigned long size)
+void init_tree_desc(struct tree_desc *desc, const struct object_id *tree_oid,
+		    const void *buffer, unsigned long size)
 {
 	struct strbuf err = STRBUF_INIT;
-	if (init_tree_desc_internal(desc, buffer, size, &err, 0))
+	if (init_tree_desc_internal(desc, tree_oid, buffer, size, &err, 0))
 		die("%s", err.buf);
 	strbuf_release(&err);
 }
 
-int init_tree_desc_gently(struct tree_desc *desc, const void *buffer, unsigned long size,
+int init_tree_desc_gently(struct tree_desc *desc, const struct object_id *oid,
+			  const void *buffer, unsigned long size,
 			  enum tree_desc_flags flags)
 {
 	struct strbuf err = STRBUF_INIT;
-	int result = init_tree_desc_internal(desc, buffer, size, &err, flags);
+	int result = init_tree_desc_internal(desc, oid, buffer, size, &err, flags);
 	if (result)
 		error("%s", err.buf);
 	strbuf_release(&err);
@@ -85,7 +91,7 @@ void *fill_tree_descriptor(struct repository *r,
 		if (!buf)
 			die("unable to read tree %s", oid_to_hex(oid));
 	}
-	init_tree_desc(desc, buf, size);
+	init_tree_desc(desc, oid, buf, size);
 	return buf;
 }
 
@@ -102,7 +108,7 @@ static void entry_extract(struct tree_desc *t, struct name_entry *a)
 static int update_tree_entry_internal(struct tree_desc *desc, struct strbuf *err)
 {
 	const void *buf = desc->buffer;
-	const unsigned char *end = (const unsigned char *)desc->entry.path + desc->entry.pathlen + 1 + the_hash_algo->rawsz;
+	const unsigned char *end = (const unsigned char *)desc->entry.path + desc->entry.pathlen + 1 + desc->algo->rawsz;
 	unsigned long size = desc->size;
 	unsigned long len = end - (const unsigned char *)buf;
 
@@ -611,7 +617,7 @@ int get_tree_entry(struct repository *r,
 		retval = -1;
 	} else {
 		struct tree_desc t;
-		init_tree_desc(&t, tree, size);
+		init_tree_desc(&t, tree_oid, tree, size);
 		retval = find_tree_entry(r, &t, name, oid, mode);
 	}
 	free(tree);
@@ -654,7 +660,7 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct repository *r,
 	struct tree_desc t;
 	int follows_remaining = GET_TREE_ENTRY_FOLLOW_SYMLINKS_MAX_LINKS;
 
-	init_tree_desc(&t, NULL, 0UL);
+	init_tree_desc(&t, NULL, NULL, 0UL);
 	strbuf_addstr(&namebuf, name);
 	oidcpy(&current_tree_oid, tree_oid);
 
@@ -690,7 +696,7 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct repository *r,
 				goto done;
 
 			/* descend */
-			init_tree_desc(&t, tree, size);
+			init_tree_desc(&t, &current_tree_oid, tree, size);
 		}
 
 		/* Handle symlinks to e.g. a//b by removing leading slashes */
@@ -724,7 +730,7 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct repository *r,
 			free(parent->tree);
 			parents_nr--;
 			parent = &parents[parents_nr - 1];
-			init_tree_desc(&t, parent->tree, parent->size);
+			init_tree_desc(&t, &parent->oid, parent->tree, parent->size);
 			strbuf_remove(&namebuf, 0, remainder ? 3 : 2);
 			continue;
 		}
@@ -804,7 +810,7 @@ enum get_oid_result get_tree_entry_follow_symlinks(struct repository *r,
 			contents_start = contents;
 
 			parent = &parents[parents_nr - 1];
-			init_tree_desc(&t, parent->tree, parent->size);
+			init_tree_desc(&t, &parent->oid, parent->tree, parent->size);
 			strbuf_splice(&namebuf, 0, len,
 				      contents_start, link_len);
 			if (remainder)
diff --git a/tree-walk.h b/tree-walk.h
index 74cdceb3fed2..cf54d01019e9 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -26,6 +26,7 @@ struct name_entry {
  * A semi-opaque data structure used to maintain the current state of the walk.
  */
 struct tree_desc {
+	const struct git_hash_algo *algo;
 	/*
 	 * pointer into the memory representation of the tree. It always
 	 * points at the current entry being visited.
@@ -85,9 +86,11 @@ int update_tree_entry_gently(struct tree_desc *);
  * size parameters are assumed to be the same as the buffer and size
  * members of `struct tree`.
  */
-void init_tree_desc(struct tree_desc *desc, const void *buf, unsigned long size);
+void init_tree_desc(struct tree_desc *desc, const struct object_id *tree_oid,
+		    const void *buf, unsigned long size);
 
-int init_tree_desc_gently(struct tree_desc *desc, const void *buf, unsigned long size,
+int init_tree_desc_gently(struct tree_desc *desc, const struct object_id *oid,
+			  const void *buf, unsigned long size,
 			  enum tree_desc_flags flags);
 
 /*
diff --git a/tree.c b/tree.c
index c745462f968e..44bcf728f10a 100644
--- a/tree.c
+++ b/tree.c
@@ -27,7 +27,7 @@ int read_tree_at(struct repository *r,
 	if (parse_tree(tree))
 		return -1;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 
 	while (tree_entry(&desc, &entry)) {
 		if (retval != all_entries_interesting) {
diff --git a/walker.c b/walker.c
index 65002a7220ad..c0fd632d921c 100644
--- a/walker.c
+++ b/walker.c
@@ -45,7 +45,7 @@ static int process_tree(struct walker *walker, struct tree *tree)
 	if (parse_tree(tree))
 		return -1;
 
-	init_tree_desc(&desc, tree->buffer, tree->size);
+	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
 	while (tree_entry(&desc, &entry)) {
 		struct object *obj = NULL;
 
-- 
2.41.0


^ permalink raw reply related

* [PATCH 26/30] builtin/ls-tree: Let the oid determine the output algorithm
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

Update cmd_ls_tree to call get_oid_with_context and pass
GET_OID_HASH_ANY instead of calling the simpler repo_get_oid.

This implments in ls-tree the behavior that asking to display a sha1
hash displays the corrresponding sha1 encoded object and asking to
display a sha256 hash displayes the corresponding sha256 encoded
object.

This is useful for testing the conversion of an object to an
equivlanet object encoded with a different hash function.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 builtin/ls-tree.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index f558db5f3b80..71281ab705b6 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -376,6 +376,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 	struct ls_tree_cmdmode_to_fmt *m2f = ls_tree_cmdmode_format;
+	struct object_context obj_context;
 	int ret;
 
 	git_config(git_default_config, NULL);
@@ -407,7 +408,9 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 			ls_tree_usage, ls_tree_options);
 	if (argc < 1)
 		usage_with_options(ls_tree_usage, ls_tree_options);
-	if (repo_get_oid(the_repository, argv[0], &oid))
+	if (get_oid_with_context(the_repository, argv[0],
+				 GET_OID_HASH_ANY, &oid,
+				 &obj_context))
 		die("Not a valid object name %s", argv[0]);
 
 	/*
-- 
2.41.0


^ permalink raw reply related

* [PATCH 25/30] object-file: Handle compat objects in check_object_signature
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

Update check_object_signature to find the hash algorithm the exising
signature uses, and to use the same hash algorithm when recomputing it
to check the signature is valid.

This will be useful when teaching git ls-tree to display objects
encoded with the compat hash algorithm.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 object-file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/object-file.c b/object-file.c
index 1601d624c9fd..df49d2239f24 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1094,9 +1094,11 @@ int check_object_signature(struct repository *r, const struct object_id *oid,
 			   void *buf, unsigned long size,
 			   enum object_type type)
 {
+	const struct git_hash_algo *algo =
+		oid->algo ? &hash_algos[oid->algo] : r->hash_algo;
 	struct object_id real_oid;
 
-	hash_object_file(r->hash_algo, buf, size, type, &real_oid);
+	hash_object_file(algo, buf, size, type, &real_oid);
 
 	return !oideq(oid, &real_oid) ? -1 : 0;
 }
-- 
2.41.0


^ permalink raw reply related

* [PATCH 23/30] builtin/cat-file:  Let the oid determine the output algorithm
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

Use GET_OID_HASH_ANY when calling get_oid_with_context.  This
implements the semi-obvious behaviour that specifying a sha1 oid shows
the output for a sha1 encoded object, and specifying a sha256 oid
shows the output for a sha256 encoded object.

This is useful for testing the the conversion of an object to an
equivalent object encoded with a different hash function.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 builtin/cat-file.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 694c8538df2f..e615d1f8e0da 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -107,7 +107,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	struct object_info oi = OBJECT_INFO_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;
-	unsigned get_oid_flags = GET_OID_RECORD_PATH | GET_OID_ONLY_TO_DIE;
+	unsigned get_oid_flags =
+		GET_OID_RECORD_PATH |
+		GET_OID_ONLY_TO_DIE |
+		GET_OID_HASH_ANY;
 	const char *path = force_path;
 	const int opt_cw = (opt == 'c' || opt == 'w');
 	if (!path && opt_cw)
@@ -223,7 +226,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 								     &size);
 				const char *target;
 				if (!skip_prefix(buffer, "object ", &target) ||
-				    get_oid_hex(target, &blob_oid))
+				    get_oid_hex_algop(target, &blob_oid,
+						      &hash_algos[oid.algo]))
 					die("%s not a valid tag", oid_to_hex(&oid));
 				free(buffer);
 			} else
@@ -512,7 +516,9 @@ static void batch_one_object(const char *obj_name,
 			     struct expand_data *data)
 {
 	struct object_context ctx;
-	int flags = opt->follow_symlinks ? GET_OID_FOLLOW_SYMLINKS : 0;
+	int flags =
+		GET_OID_HASH_ANY |
+		(opt->follow_symlinks ? GET_OID_FOLLOW_SYMLINKS : 0);
 	enum get_oid_result result;
 
 	result = get_oid_with_context(the_repository, obj_name,
-- 
2.41.0


^ permalink raw reply related

* [PATCH 21/30] repository: Implement extensions.compatObjectFormat
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "brian m. carlson" <sandals@crustytoothpaste.net>

Add a configuration option to enable updating and reading from
compatibility hash maps when git accesses the reposotiry.

Call the helper function repo_set_compat_hash_algo with the value
that compatObjectFormat is set to.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 Documentation/config/extensions.txt | 12 ++++++++++++
 repository.c                        |  2 +-
 setup.c                             | 23 +++++++++++++++++++++--
 setup.h                             |  1 +
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/extensions.txt b/Documentation/config/extensions.txt
index bccaec7a9636..9f72e6d9f4f1 100644
--- a/Documentation/config/extensions.txt
+++ b/Documentation/config/extensions.txt
@@ -7,6 +7,18 @@ Note that this setting should only be set by linkgit:git-init[1] or
 linkgit:git-clone[1].  Trying to change it after initialization will not
 work and will produce hard-to-diagnose issues.
 
+extensions.compatObjectFormat::
+
+	Specify a compatitbility hash algorithm to use.  The acceptable values
+	are `sha1` and `sha256`.  The value specified must be different from the
+	value of extensions.objectFormat.  This allows client level
+	interoperability between git repositories whose objectFormat matches
+	this compatObjectFormat.  In particular when fully implemented the
+	pushes and pulls from a repository in whose objectFormat matches
+	compatObjectFormat.  As well as being able to use oids encoded in
+	compatObjectFormat in addition to oids encoded with objectFormat to
+	locally specify objects.
+
 extensions.worktreeConfig::
 	If enabled, then worktrees will load config settings from the
 	`$GIT_DIR/config.worktree` file in addition to the
diff --git a/repository.c b/repository.c
index 6214f61cf4e7..9d91536b613b 100644
--- a/repository.c
+++ b/repository.c
@@ -194,7 +194,7 @@ int repo_init(struct repository *repo,
 		goto error;
 
 	repo_set_hash_algo(repo, format.hash_algo);
-	repo_set_compat_hash_algo(repo, GIT_HASH_UNKNOWN);
+	repo_set_compat_hash_algo(repo, format.compat_hash_algo);
 	repo->repository_format_worktree_config = format.worktree_config;
 
 	/* take ownership of format.partial_clone */
diff --git a/setup.c b/setup.c
index deb5a33fe9e1..87b40472dbc5 100644
--- a/setup.c
+++ b/setup.c
@@ -598,6 +598,25 @@ static enum extension_result handle_extension(const char *var,
 		}
 		data->hash_algo = format;
 		return EXTENSION_OK;
+	} else if (!strcmp(ext, "compatobjectformat")) {
+		struct string_list_item *item;
+		int format;
+
+		if (!value)
+			return config_error_nonbool(var);
+		format = hash_algo_by_name(value);
+		if (format == GIT_HASH_UNKNOWN)
+			return error(_("invalid value for '%s': '%s'"),
+				     "extensions.compatobjectformat", value);
+		/* For now only support compatObjectFormat being specified once. */
+		for_each_string_list_item(item, &data->v1_only_extensions) {
+			if (!strcmp(item->string, "compatobjectformat"))
+				return error(_("'%s' already specified as '%s'"),
+					"extensions.compatobjectformat",
+					hash_algos[data->compat_hash_algo].name);
+		}
+		data->compat_hash_algo = format;
+		return EXTENSION_OK;
 	}
 	return EXTENSION_UNKNOWN;
 }
@@ -1573,7 +1592,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		if (startup_info->have_repository) {
 			repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
 			repo_set_compat_hash_algo(the_repository,
-						  GIT_HASH_UNKNOWN);
+						  repo_fmt.compat_hash_algo);
 			the_repository->repository_format_worktree_config =
 				repo_fmt.worktree_config;
 			/* take ownership of repo_fmt.partial_clone */
@@ -1667,7 +1686,7 @@ void check_repository_format(struct repository_format *fmt)
 	check_repository_format_gently(get_git_dir(), fmt, NULL);
 	startup_info->have_repository = 1;
 	repo_set_hash_algo(the_repository, fmt->hash_algo);
-	repo_set_compat_hash_algo(the_repository, GIT_HASH_UNKNOWN);
+	repo_set_compat_hash_algo(the_repository, fmt->compat_hash_algo);
 	the_repository->repository_format_worktree_config =
 		fmt->worktree_config;
 	the_repository->repository_format_partial_clone =
diff --git a/setup.h b/setup.h
index 58fd2605dd26..5d678ceb8caa 100644
--- a/setup.h
+++ b/setup.h
@@ -86,6 +86,7 @@ struct repository_format {
 	int worktree_config;
 	int is_bare;
 	int hash_algo;
+	int compat_hash_algo;
 	int sparse_index;
 	char *work_tree;
 	struct string_list unknown_extensions;
-- 
2.41.0


^ permalink raw reply related

* [PATCH 22/30] rev-parse: Add an --output-object-format parameter
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

The new --output-object-format parameter returns the oid in the
specified format.

This is a generally useful plumbing facility.  It is useful for writing
test cases and for directly querying the translation maps.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 Documentation/git-rev-parse.txt | 12 ++++++++++++
 builtin/rev-parse.c             | 23 +++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index f26a7591e373..f0f9021f2a5a 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -159,6 +159,18 @@ for another option.
 	unfortunately named tag "master"), and show them as full
 	refnames (e.g. "refs/heads/master").
 
+--output-object-format=(sha1|sha256|storage)::
+
+	Allow oids to be input from any object format that the current
+	repository supports.
+
+	Specifying "sha1" translates if necessary and returns a sha1 oid.
+
+	Specifying "sha256" translates if necessary and returns a sha256 oid.
+
+	Specifying "storage" translates if necessary and returns an oid in
+	encoded in the storage hash algorithm.
+
 Options for Objects
 ~~~~~~~~~~~~~~~~~~~
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 43e96765400c..0ef3e658cc5b 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -25,6 +25,7 @@
 #include "submodule.h"
 #include "commit-reach.h"
 #include "shallow.h"
+#include "object-file-convert.h"
 
 #define DO_REVS		1
 #define DO_NOREV	2
@@ -675,6 +676,8 @@ static void print_path(const char *path, const char *prefix, enum format_type fo
 int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 {
 	int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
+	const struct git_hash_algo *output_algo = NULL;
+	const struct git_hash_algo *compat = NULL;
 	int did_repo_setup = 0;
 	int has_dashdash = 0;
 	int output_prefix = 0;
@@ -746,6 +749,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 
 			prepare_repo_settings(the_repository);
 			the_repository->settings.command_requires_full_index = 0;
+			compat = the_repository->compat_hash_algo;
 		}
 
 		if (!strcmp(arg, "--")) {
@@ -833,6 +837,22 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				flags |= GET_OID_QUIETLY;
 				continue;
 			}
+			if (opt_with_value(arg, "--output-object-format", &arg)) {
+				if (!arg)
+					die(_("no object format specified"));
+				if (!strcmp(arg, the_hash_algo->name) ||
+				    !strcmp(arg, "storage")) {
+					flags |= GET_OID_HASH_ANY;
+					output_algo = the_hash_algo;
+					continue;
+				}
+				else if (compat && !strcmp(arg, compat->name)) {
+					flags |= GET_OID_HASH_ANY;
+					output_algo = compat;
+					continue;
+				}
+				else die(_("unsupported object format: %s"), arg);
+			}
 			if (opt_with_value(arg, "--short", &arg)) {
 				filter &= ~(DO_FLAGS|DO_NOREV);
 				verify = 1;
@@ -1083,6 +1103,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 		}
 		if (!get_oid_with_context(the_repository, name,
 					  flags, &oid, &unused)) {
+			if (output_algo)
+				repo_oid_to_algop(the_repository, &oid,
+						  output_algo, &oid);
 			if (verify)
 				revs_count++;
 			else
-- 
2.41.0


^ permalink raw reply related

* [PATCH 20/30] object-file: Update object_info_extended to reencode objects
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

oid_object_info_extended is updated to detect an oid encoding that
does not match the current repository, use repo_oid_to_algop to find
the correspoding oid in the current repository and to return the data
for the oid.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 object-file.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/object-file.c b/object-file.c
index d66d11890696..1601d624c9fd 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1662,10 +1662,101 @@ static int do_oid_object_info_extended(struct repository *r,
 	return 0;
 }
 
+static int oid_object_info_convert(struct repository *r,
+				   const struct object_id *input_oid,
+				   struct object_info *input_oi, unsigned flags)
+{
+	const struct git_hash_algo *input_algo = &hash_algos[input_oid->algo];
+	int do_die = flags & OBJECT_INFO_DIE_IF_CORRUPT;
+	struct strbuf type_name = STRBUF_INIT;
+	struct object_id oid, delta_base_oid;
+	struct object_info new_oi, *oi;
+	unsigned long size;
+	void *content;
+	int ret;
+
+	if (repo_oid_to_algop(r, input_oid, the_hash_algo, &oid)) {
+		if (do_die)
+			die(_("missing mapping of %s to %s"),
+			    oid_to_hex(input_oid), the_hash_algo->name);
+		return -1;
+	}
+
+	/* Is new_oi needed? */
+	oi = input_oi;
+	if (input_oi && (input_oi->delta_base_oid || input_oi->sizep ||
+			 input_oi->contentp)) {
+		new_oi = *input_oi;
+		/* Does delta_base_oid need to be converted? */
+		if (input_oi->delta_base_oid)
+			new_oi.delta_base_oid = &delta_base_oid;
+		/* Will the attributes differ when converted? */
+		if (input_oi->sizep || input_oi->contentp) {
+			new_oi.contentp = &content;
+			new_oi.sizep = &size;
+			new_oi.type_name = &type_name;
+		}
+		oi = &new_oi;
+	}
+
+	ret = oid_object_info_extended(r, &oid, oi, flags);
+	if (ret)
+		return -1;
+	if (oi == input_oi)
+		return ret;
+
+	if (new_oi.contentp) {
+		struct strbuf outbuf = STRBUF_INIT;
+		enum object_type type;
+
+		type = type_from_string_gently(type_name.buf, type_name.len,
+					       !do_die);
+		if (type == -1)
+			return -1;
+		if (type != OBJ_BLOB) {
+			ret = convert_object_file(&outbuf,
+						  the_hash_algo, input_algo,
+						  content, size, type, !do_die);
+			if (ret == -1)
+				return -1;
+			free(content);
+			size = outbuf.len;
+			content = strbuf_detach(&outbuf, NULL);
+		}
+		if (input_oi->sizep)
+			*input_oi->sizep = size;
+		if (input_oi->contentp)
+			*input_oi->contentp = content;
+		else
+			free(content);
+		if (input_oi->type_name)
+			*input_oi->type_name = type_name;
+		else
+			strbuf_release(&type_name);
+	}
+	if (new_oi.delta_base_oid == &delta_base_oid) {
+		if (repo_oid_to_algop(r, &delta_base_oid, input_algo,
+				 input_oi->delta_base_oid)) {
+			if (do_die)
+				die(_("missing mapping of %s to %s"),
+				    oid_to_hex(&delta_base_oid),
+				    input_algo->name);
+			return -1;
+		}
+	}
+	input_oi->whence = new_oi.whence;
+	input_oi->u = new_oi.u;
+	return ret;
+}
+
 int oid_object_info_extended(struct repository *r, const struct object_id *oid,
 			     struct object_info *oi, unsigned flags)
 {
 	int ret;
+
+	if (oid->algo && (hash_algo_by_ptr(r->hash_algo) != oid->algo))
+		return oid_object_info_convert(r, oid, oi, flags);
+
 	obj_read_lock();
 	ret = do_oid_object_info_extended(r, oid, oi, flags);
 	obj_read_unlock();
-- 
2.41.0


^ permalink raw reply related

* [PATCH 19/30] object-file-convert: Convert commits that embed signed tags
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

As mentioned in the hash function transition plan commit mergetag
lines need to be handled.  The commit mergetag lines embed an entire
tag object in a commit object.

Keep the implementation sane if not fast by unembedding the tag
object, converting the tag object, and embedding the new tag object,
in the new commit object.

In the long run I don't expect any other approach is maintainable, as
tag objects may be extended in ways that require additional
translation.

To keep the implementation of convert_commit_object maintainable I
have modified convert_commit_object to process the lines in any order,
and to fail on unknown lines.  We can't know ahead of time if a new
line might embed something that needs translation or not so it is
better to fail and require the code to be updated instead of silently
mistranslating objects.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 object-file-convert.c | 104 +++++++++++++++++++++++++++++++++---------
 1 file changed, 82 insertions(+), 22 deletions(-)

diff --git a/object-file-convert.c b/object-file-convert.c
index f53e14e5a170..8ede9889a7ab 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -146,35 +146,95 @@ static int convert_commit_object(struct strbuf *out,
 	const int tree_entry_len = from->hexsz + 5;
 	const int parent_entry_len = from->hexsz + 7;
 	struct object_id oid, mapped_oid;
-	const char *p;
+	const char *p, *eol;
 
 	tail += size;
-	if (tail <= bufptr + tree_entry_len + 1 || memcmp(bufptr, "tree ", 5) ||
-			bufptr[tree_entry_len] != '\n')
-		return error("bogus commit object");
-	if (parse_oid_hex_algop(bufptr + 5, &oid, &p, from) < 0)
-		return error("bad tree pointer");
 
-	if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
-		return error("unable to map tree %s in commit object",
-			     oid_to_hex(&oid));
-	strbuf_addf(out, "tree %s\n", oid_to_hex(&mapped_oid));
-	bufptr = p + 1;
+	while ((bufptr < tail) && (*bufptr != '\n')) {
+		eol = memchr(bufptr, '\n', tail - bufptr);
+		if (!eol)
+			return error(_("bad %s in commit"), "line");
+
+		if (((bufptr + 5) < eol) && !memcmp(bufptr, "tree ", 5))
+		{
+			if (((bufptr + tree_entry_len) != eol) ||
+			    parse_oid_hex_algop(bufptr + 5, &oid, &p, from) ||
+			    (p != eol))
+				return error(_("bad %s in commit"), "tree");
+
+			if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
+				return error(_("unable to map %s %s in commit object"),
+					     "tree", oid_to_hex(&oid));
+			strbuf_addf(out, "tree %s\n", oid_to_hex(&mapped_oid));
+		}
+		else if (((bufptr + 7) < eol) && !memcmp(bufptr, "parent ", 7))
+		{
+			if (((bufptr + parent_entry_len) != eol) ||
+			    parse_oid_hex_algop(bufptr + 7, &oid, &p, from) ||
+			    (p != eol))
+				return error(_("bad %s in commit"), "parent");
 
-	while (bufptr + parent_entry_len < tail && !memcmp(bufptr, "parent ", 7)) {
-		if (tail <= bufptr + parent_entry_len + 1 ||
-		    parse_oid_hex_algop(bufptr + 7, &oid, &p, from) ||
-		    *p != '\n')
-			return error("bad parents in commit");
+			if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
+				return error(_("unable to map %s %s in commit object"),
+					     "parent", oid_to_hex(&oid));
 
-		if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
-			return error("unable to map parent %s in commit object",
-				     oid_to_hex(&oid));
+			strbuf_addf(out, "parent %s\n", oid_to_hex(&mapped_oid));
+		}
+		else if (((bufptr + 9) < eol) && !memcmp(bufptr, "mergetag ", 9))
+		{
+			struct strbuf tag = STRBUF_INIT, new_tag = STRBUF_INIT;
 
-		strbuf_addf(out, "parent %s\n", oid_to_hex(&mapped_oid));
-		bufptr = p + 1;
+			/* Recover the tag object from the mergetag */
+			strbuf_add(&tag, bufptr + 9, (eol - (bufptr + 9)) + 1);
+
+			bufptr = eol + 1;
+			while ((bufptr < tail) && (*bufptr == ' ')) {
+				eol = memchr(bufptr, '\n', tail - bufptr);
+				if (!eol) {
+					strbuf_release(&tag);
+					return error(_("bad %s in commit"), "mergetag continuation");
+				}
+				strbuf_add(&tag, bufptr + 1, (eol - (bufptr + 1)) + 1);
+				bufptr = eol + 1;
+			}
+
+			/* Compute the new tag object */
+			if (convert_tag_object(&new_tag, from, to, tag.buf, tag.len)) {
+				strbuf_release(&tag);
+				strbuf_release(&new_tag);
+				return -1;
+			}
+
+			/* Write the new mergetag */
+			strbuf_addstr(out, "mergetag");
+			strbuf_add_lines(out, " ", new_tag.buf, new_tag.len);
+			strbuf_release(&tag);
+			strbuf_release(&new_tag);
+		}
+		else if (((bufptr + 7) < tail) && !memcmp(bufptr, "author ", 7))
+			strbuf_add(out, bufptr, (eol - bufptr) + 1);
+		else if (((bufptr + 10) < tail) && !memcmp(bufptr, "committer ", 10))
+			strbuf_add(out, bufptr, (eol - bufptr) + 1);
+		else if (((bufptr + 9) < tail) && !memcmp(bufptr, "encoding ", 9))
+			strbuf_add(out, bufptr, (eol - bufptr) + 1);
+		else if (((bufptr + 6) < tail) && !memcmp(bufptr, "gpgsig", 6))
+			strbuf_add(out, bufptr, (eol - bufptr) + 1);
+		else {
+			/* Unknown line fail it might embed an oid */
+			return -1;
+		}
+		/* Consume any trailing continuation lines */
+		bufptr = eol + 1;
+		while ((bufptr < tail) && (*bufptr == ' ')) {
+			eol = memchr(bufptr, '\n', tail - bufptr);
+			if (!eol)
+				return error(_("bad %s in commit"), "continuation");
+			strbuf_add(out, bufptr, (eol - bufptr) + 1);
+			bufptr = eol + 1;
+		}
 	}
-	strbuf_add(out, bufptr, tail - bufptr);
+	if (bufptr < tail)
+		strbuf_add(out, bufptr, tail - bufptr);
 	return 0;
 }
 
-- 
2.41.0


^ permalink raw reply related

* [PATCH 18/30] object-file-convert: convert commit objects when writing
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W . Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "brian m. carlson" <sandals@crustytoothpaste.net>

When writing a commit object in a repository with both SHA-1 and
SHA-256, we'll need to convert our commit objects so that we can write
the hash values for both into the repository.  To do so, let's add a
function to convert commit objects.

Read the commit object and map the tree value and any of the parent
values, and copy the rest of the commit through unmodified.  Note that
we don't need to modify the signature headers, because they are the
same under both algorithms.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 object-file-convert.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/object-file-convert.c b/object-file-convert.c
index 822be9d0fdb8..f53e14e5a170 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -136,6 +136,48 @@ static int convert_tag_object(struct strbuf *out,
 	return 0;
 }
 
+static int convert_commit_object(struct strbuf *out,
+				 const struct git_hash_algo *from,
+				 const struct git_hash_algo *to,
+				 const char *buffer, size_t size)
+{
+	const char *tail = buffer;
+	const char *bufptr = buffer;
+	const int tree_entry_len = from->hexsz + 5;
+	const int parent_entry_len = from->hexsz + 7;
+	struct object_id oid, mapped_oid;
+	const char *p;
+
+	tail += size;
+	if (tail <= bufptr + tree_entry_len + 1 || memcmp(bufptr, "tree ", 5) ||
+			bufptr[tree_entry_len] != '\n')
+		return error("bogus commit object");
+	if (parse_oid_hex_algop(bufptr + 5, &oid, &p, from) < 0)
+		return error("bad tree pointer");
+
+	if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
+		return error("unable to map tree %s in commit object",
+			     oid_to_hex(&oid));
+	strbuf_addf(out, "tree %s\n", oid_to_hex(&mapped_oid));
+	bufptr = p + 1;
+
+	while (bufptr + parent_entry_len < tail && !memcmp(bufptr, "parent ", 7)) {
+		if (tail <= bufptr + parent_entry_len + 1 ||
+		    parse_oid_hex_algop(bufptr + 7, &oid, &p, from) ||
+		    *p != '\n')
+			return error("bad parents in commit");
+
+		if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
+			return error("unable to map parent %s in commit object",
+				     oid_to_hex(&oid));
+
+		strbuf_addf(out, "parent %s\n", oid_to_hex(&mapped_oid));
+		bufptr = p + 1;
+	}
+	strbuf_add(out, bufptr, tail - bufptr);
+	return 0;
+}
+
 int convert_object_file(struct strbuf *outbuf,
 			const struct git_hash_algo *from,
 			const struct git_hash_algo *to,
@@ -150,13 +192,15 @@ int convert_object_file(struct strbuf *outbuf,
 		die("Refusing noop object file conversion");
 
 	switch (type) {
+	case OBJ_COMMIT:
+		ret = convert_commit_object(outbuf, from, to, buf, len);
+		break;
 	case OBJ_TREE:
 		ret = convert_tree_object(outbuf, from, to, buf, len);
 		break;
 	case OBJ_TAG:
 		ret = convert_tag_object(outbuf, from, to, buf, len);
 		break;
-	case OBJ_COMMIT:
 	default:
 		/* Not implemented yet, so fail. */
 		ret = -1;
-- 
2.41.0


^ permalink raw reply related

* [PATCH 17/30] object-file-convert: Don't leak when converting tag objects
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "Eric W. Biederman" <ebiederm@xmission.com>

Upon close examination I discovered that while brian's code to convert
tag objects was functionally correct, it leaked memory.

Rearrange the code so that all error checking happens before any
memory is allocated.

Add code to release the temporary strbufs the code uses.

The code pretty much assumes the tag object ends with a newline,
so add an explict test to verify that is the case.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 object-file-convert.c | 45 ++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/object-file-convert.c b/object-file-convert.c
index 777ae5b58036..822be9d0fdb8 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -90,44 +90,49 @@ static int convert_tag_object(struct strbuf *out,
 			      const struct git_hash_algo *to,
 			      const char *buffer, size_t size)
 {
-	struct strbuf payload = STRBUF_INIT, temp = STRBUF_INIT, oursig = STRBUF_INIT, othersig = STRBUF_INIT;
+	struct strbuf payload = STRBUF_INIT, oursig = STRBUF_INIT, othersig = STRBUF_INIT;
+	const int entry_len = from->hexsz + 7;
 	size_t payload_size;
 	struct object_id oid, mapped_oid;
 	const char *p;
 
-	/* Add some slop for longer signature header in the new algorithm. */
-	strbuf_grow(out, size + 7);
+	/* Consume the object line */
+	if ((entry_len >= size) ||
+	    memcmp(buffer, "object ", 7) || buffer[entry_len] != '\n')
+		return error("bogus tag object");
+	if (parse_oid_hex_algop(buffer + 7, &oid, &p, from) < 0)
+		return error("bad tag object ID");
+	if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
+		return error("unable to map tree %s in tag object",
+			     oid_to_hex(&oid));
+	size -= ((p + 1) - buffer);
+	buffer = p + 1;
 
 	/* Is there a signature for our algorithm? */
 	payload_size = parse_signed_buffer(buffer, size);
-	strbuf_add(&payload, buffer, payload_size);
 	if (payload_size != size) {
 		/* Yes, there is. */
 		strbuf_add(&oursig, buffer + payload_size, size - payload_size);
 	}
-	/* Now, is there a signature for the other algorithm? */
-	if (parse_buffer_signed_by_header(payload.buf, payload.len, &temp, &othersig, to)) {
-		/* Yes, there is. */
-		strbuf_swap(&payload, &temp);
-		strbuf_release(&temp);
-	}
 
+	/* Now, is there a signature for the other algorithm? */
+	parse_buffer_signed_by_header(buffer, payload_size, &payload, &othersig, to);
 	/*
 	 * Our payload is now in payload and we may have up to two signatrures
 	 * in oursig and othersig.
 	 */
-	if (strncmp(payload.buf, "object ", 7) || payload.buf[from->hexsz + 7] != '\n')
-		return error("bogus tag object");
-	if (parse_oid_hex_algop(payload.buf + 7, &oid, &p, from) < 0)
-		return error("bad tag object ID");
-	if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
-		return error("unable to map tree %s in tag object",
-			     oid_to_hex(&oid));
-	strbuf_addf(out, "object %s", oid_to_hex(&mapped_oid));
-	strbuf_add(out, p, payload.len - (p - payload.buf));
-	strbuf_addbuf(out, &othersig);
+
+	/* Add some slop for longer signature header in the new algorithm. */
+	strbuf_grow(out, (7 + to->hexsz + 1) + size + 7);
+	strbuf_addf(out, "object %s\n", oid_to_hex(&mapped_oid));
+	strbuf_addbuf(out, &payload);
 	if (oursig.len)
 		add_header_signature(out, &oursig, from);
+	strbuf_addbuf(out, &othersig);
+
+	strbuf_release(&payload);
+	strbuf_release(&othersig);
+	strbuf_release(&oursig);
 	return 0;
 }
 
-- 
2.41.0


^ permalink raw reply related

* [PATCH 16/30] object-file-convert: convert tag objects when writing
From: Eric W. Biederman @ 2023-09-27 19:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W . Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>

From: "brian m. carlson" <sandals@crustytoothpaste.net>

When writing a tag object in a repository with both SHA-1 and SHA-256,
we'll need to convert our commit objects so that we can write the hash
values for both into the repository.  To do so, let's add a function to
convert tag objects.

Note that signatures for tag objects in the current algorithm trail the
message, and those for the alternate algorithm are in headers.
Therefore, we parse the tag object for both a trailing signature and a
header and then, when writing the other format, swap the two around.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 object-file-convert.c | 52 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/object-file-convert.c b/object-file-convert.c
index a9e7a208a707..777ae5b58036 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -7,6 +7,8 @@
 #include "hash.h"
 #include "object.h"
 #include "loose.h"
+#include "commit.h"
+#include "gpg-interface.h"
 #include "object-file-convert.h"
 
 int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
@@ -83,6 +85,52 @@ static int convert_tree_object(struct strbuf *out,
 	return 0;
 }
 
+static int convert_tag_object(struct strbuf *out,
+			      const struct git_hash_algo *from,
+			      const struct git_hash_algo *to,
+			      const char *buffer, size_t size)
+{
+	struct strbuf payload = STRBUF_INIT, temp = STRBUF_INIT, oursig = STRBUF_INIT, othersig = STRBUF_INIT;
+	size_t payload_size;
+	struct object_id oid, mapped_oid;
+	const char *p;
+
+	/* Add some slop for longer signature header in the new algorithm. */
+	strbuf_grow(out, size + 7);
+
+	/* Is there a signature for our algorithm? */
+	payload_size = parse_signed_buffer(buffer, size);
+	strbuf_add(&payload, buffer, payload_size);
+	if (payload_size != size) {
+		/* Yes, there is. */
+		strbuf_add(&oursig, buffer + payload_size, size - payload_size);
+	}
+	/* Now, is there a signature for the other algorithm? */
+	if (parse_buffer_signed_by_header(payload.buf, payload.len, &temp, &othersig, to)) {
+		/* Yes, there is. */
+		strbuf_swap(&payload, &temp);
+		strbuf_release(&temp);
+	}
+
+	/*
+	 * Our payload is now in payload and we may have up to two signatrures
+	 * in oursig and othersig.
+	 */
+	if (strncmp(payload.buf, "object ", 7) || payload.buf[from->hexsz + 7] != '\n')
+		return error("bogus tag object");
+	if (parse_oid_hex_algop(payload.buf + 7, &oid, &p, from) < 0)
+		return error("bad tag object ID");
+	if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid))
+		return error("unable to map tree %s in tag object",
+			     oid_to_hex(&oid));
+	strbuf_addf(out, "object %s", oid_to_hex(&mapped_oid));
+	strbuf_add(out, p, payload.len - (p - payload.buf));
+	strbuf_addbuf(out, &othersig);
+	if (oursig.len)
+		add_header_signature(out, &oursig, from);
+	return 0;
+}
+
 int convert_object_file(struct strbuf *outbuf,
 			const struct git_hash_algo *from,
 			const struct git_hash_algo *to,
@@ -100,8 +148,10 @@ int convert_object_file(struct strbuf *outbuf,
 	case OBJ_TREE:
 		ret = convert_tree_object(outbuf, from, to, buf, len);
 		break;
-	case OBJ_COMMIT:
 	case OBJ_TAG:
+		ret = convert_tag_object(outbuf, from, to, buf, len);
+		break;
+	case OBJ_COMMIT:
 	default:
 		/* Not implemented yet, so fail. */
 		ret = -1;
-- 
2.41.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox