* Re: bug - Inconsistency of git status and branch, ./git/branches and ./git/HEAD
From: Chris Torek @ 2023-09-30 11:32 UTC (permalink / raw)
To: Irina Gulina; +Cc: git, Tomas Tomecek
In-Reply-To: <CAPx1GvcjvctrQ-w1MekBs7S+R_7=KhCLUQ915-JAnxEq=BE3iQ@mail.gmail.com>
I should note a slight correction here:
On Sat, Sep 30, 2023 at 4:28 AM Chris Torek <chris.torek@gmail.com> wrote:
> A new, empty repository cannot have any branches, because a branch
> is defined as "the latest commit that is on the branch" (this definition is
> unsatisfyingly circular, but that too is what it is).
This is just one of multiple definitions of "branch" in Git. A branch is also
a *set* of commits, and/or a *name*. The fact that the word "branch" has
so many meanings in Git makes it almost meaningless: it's like being at
a party where everyone is named Chris. Yet somehow we humans still
manage to extract meaning from a sentence like "Hey Chris, Chris said
to tell you that Chris can't get Chris's car started, so Chris and Chris
won't be here."
Chris
^ permalink raw reply
* Possible to update-ref remote repository?
From: Jesse Hopkins @ 2023-09-30 16:34 UTC (permalink / raw)
To: git
Hello -
Wondering if it's possible to do the equivalent of the update-ref
command remotely. Or I guess another way of putting it would be to
git-push to a remote repository without needing a local clone of the
repo.
Trying do something like:
git push <remote-repo-url> <sha1>:refs/heads/mybranchtoupdate
where I know that <sha1> already exists on the remote. I'd like to
avoid the need to clone a local copy of the repo. Wondering if there
might be some plumbing command(s) that could accomplish this?
Regards,
Jesse
P.S. I think think a minimal way to do this using git push would be:
git init
git remote add origin <remote-repo-url>
git fetch origin <sha1>:refs/heads/mybranchtoupdate
git push origin refs/heads/mybranchtoupdate:refs/heads/mybranchtoupdate
Seems that if it's known that <sha1> already exists on the remote, the
fetch is unnecessary network overhead?
^ permalink raw reply
* Re: Possible to update-ref remote repository?
From: Junio C Hamano @ 2023-09-30 17:17 UTC (permalink / raw)
To: Jesse Hopkins; +Cc: git
In-Reply-To: <CAL3By--HowOL1ffKBPfmwnfUdJd4KXcnkpS2BgkbO=9E2WnHKw@mail.gmail.com>
Jesse Hopkins <jesse.hops@gmail.com> writes:
> Wondering if it's possible to do the equivalent of the update-ref
> command remotely. Or I guess another way of putting it would be to
> git-push to a remote repository without needing a local clone of the
> repo.
>
> Trying do something like:
>
> git push <remote-repo-url> <sha1>:refs/heads/mybranchtoupdate
>
> where I know that <sha1> already exists on the remote. I'd like to
> avoid the need to clone a local copy of the repo. Wondering if there
> might be some plumbing command(s) that could accomplish this?
There is no such command shipped with Git. I do not think anybody
ever proposed to add such a feature, as it won't be generally useful
in contexts other than one-shot surgery.
But that is different from that it is impossible to do. At the
protocol level, as long as the receiving end is convinced that such
a push, which does not transfer any object but proposes to update a
ref to a new value, does not corrupt the resulting repository state
after acceptingit, the receiving end does not care if the sending
end has what object, or even any repository---the receiving end
cannot even know what the sending end actually has.
The devil however is in the details of ensuring the "as long as the
receiving end is convinced" part. If you know that an object whose
object name is X is sitting at the tip of branch A at the remote,
and you try to update the tip of branch B to the same object, it is
likely [*] that the remote would notice that such an update after
receiving no new objects is safe. But if the object X is not
sitting at the tip of any branch or ref (it may be a few commits
behind an existing ref, or it may be dangling ahead of all refs), it
would depend on how the receiving end determines that it has object
chains necessary to complete the new DAG that has X. The official
versions of Git client historically have done a thorough job in
check_connected() to even discover fully connected object chains
that allows us to resurrect such a dangling tip of a DAG, but we
cannot complain if third-party Git implementations misses less
obvious cases.
Having said all that, writing a specialized "push lookalike" that
sends the required protocol message that would have been spewed by a
real "git push" client operating in the right environment (i.e., it
has a repository that is a good clone of the receiving repository at
the <remote-repo-url>, the receiving repository has <sha1> and all
the objects that are reachable from it, refs/heads/mybranchtoupdate
points at a commit that is an ancestor of <sha1> right now) should
not be brain surgery, and with such a program you can
$ git-push-empty <remote-repo-url> <sha1>:refs/heads/mybranchtoupdate
and fool the receiving end to do what you want it to do, as it
cannot tell if it is talking to a real "git push" client or to your
"push-empty" program from what is coming over the wire.
^ permalink raw reply
* Re: bug - Inconsistency of git status and branch, ./git/branches and ./git/HEAD
From: Junio C Hamano @ 2023-09-30 17:25 UTC (permalink / raw)
To: Irina Gulina; +Cc: git, Tomas Tomecek
In-Reply-To: <CAAcHogUEtcsFcDYp16g=154CMPh_a5fmY6ABEfp9zeqEJBNb6w@mail.gmail.com>
Irina Gulina <igulina@redhat.com> writes:
>> What did you do before the bug happened? (Steps to reproduce your issue)
>> 1. create an empty directory in CLI: mkdir test_repo
>> 2. convert it to a git directory: cd test_repo & git init
>> 3. 'git status' will say you are "On branch master"
>> 4. 'git branch', 'git branch --all', or 'git branch --list' - nothing
>> will say the master branch exists. And per man pages definition the
>> branch command displays existing branches. So the question is why 'git
>> status' says "On branch master", but 'git branches' don't?
Chris gave a good answer to this most relevant question; let me
fill in one gap with others.
>> 5. check on 'ls -la ./git/branches' - it's empty
The directory is where you would configure how individual local
branches you have would behave, and it is expected to be empty if
you did not do anything special.
>> 6. check on 'ls -la ./git/refs/heads' - it's empty
You can never expect the contents of .git/refs/* hierarchy to
correspond to any state of the refs. If you have a history behind
your local branch 'master', i.e., after running something like
$ git init empty && cd empty && git commit --allow-empty -m init
you can "pack" the references, i.e.,
$ git pack-refs --all
and all the necessary data for Git to know what your branches are
and what commits they point at will be removed from .git/refs/* and
stored in different place in .git/ hierarchy. So "there is no file
at .git/refs/heads/master" does not mean "there is no master branch",
even when we are not talking about a branch in the unborn state.
>> 7. check on 'cat ./git/HEAD' - it shows 'ref: refs/heads/master' -
>> why? if we didn't create any commit, and no branch exists?
The same comment applies to the above. If you want to know the
state of HEAD, there are commands to do so ("git branch --list" being
one of them). Do not expect that .git/HEAD would stay forever be
the implementation detail of what the HEAD ref points at.
^ permalink raw reply
* Please I want to discuess something with you reply me Urgent,
From: M X chris colin @ 2023-10-01 13:35 UTC (permalink / raw)
To: git
Holle,
Please I want to discuess something with you reply me for more informations.
COLIN
^ permalink raw reply
* [PATCH] diff: fix --merge-base with annotated tags
From: Alyssa Ross @ 2023-10-01 15:18 UTC (permalink / raw)
To: git; +Cc: Denton Liu
Checking early for OBJ_COMMIT excludes other objects that can be
resolved to commits, like annotated tags. If we remove it, annotated
tags will be resolved and handled just fine by
lookup_commit_reference(), and if we are given something that can't be
resolved to a commit, we'll still get a useful error message, e.g.:
> error: object 21ab162211ac3ef13c37603ca88b27e9c7e0d40b is a tree, not a commit
> fatal: no merge base found
Signed-off-by: Alyssa Ross <hi@alyssa.is>
---
diff-lib.c | 2 --
t/t4068-diff-symmetric-merge-base.sh | 13 ++++++++++---
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index 6b0c6a7180..543398b4d8 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -572,8 +572,6 @@ void diff_get_merge_base(const struct rev_info *revs, struct object_id *mb)
struct object *obj = revs->pending.objects[i].item;
if (obj->flags)
die(_("--merge-base does not work with ranges"));
- if (obj->type != OBJ_COMMIT)
- die(_("--merge-base only works with commits"));
}
/*
diff --git a/t/t4068-diff-symmetric-merge-base.sh b/t/t4068-diff-symmetric-merge-base.sh
index 2d650d8f10..541642650f 100755
--- a/t/t4068-diff-symmetric-merge-base.sh
+++ b/t/t4068-diff-symmetric-merge-base.sh
@@ -34,7 +34,7 @@ test_expect_success setup '
echo c >c &&
git add c &&
git commit -m C &&
- git tag commit-C &&
+ git tag -m commit-C commit-C &&
git merge -m D main &&
git tag commit-D &&
git checkout main &&
@@ -109,6 +109,13 @@ do
test_cmp expect actual
'
+ test_expect_success "$cmd --merge-base with annotated tag" '
+ git checkout main &&
+ git $cmd commit-C >expect &&
+ git $cmd --merge-base commit-C >actual &&
+ test_cmp expect actual
+ '
+
test_expect_success "$cmd --merge-base with one commit and unstaged changes" '
git checkout main &&
test_when_finished git reset --hard &&
@@ -143,7 +150,7 @@ do
test_expect_success "$cmd --merge-base with non-commit" '
git checkout main &&
test_must_fail git $cmd --merge-base main^{tree} 2>err &&
- test_i18ngrep "fatal: --merge-base only works with commits" err
+ test_i18ngrep "is a tree, not a commit" err
'
test_expect_success "$cmd --merge-base with no merge bases and one commit" '
@@ -169,7 +176,7 @@ do
test_expect_success "$cmd --merge-base commit and non-commit" '
test_must_fail git $cmd --merge-base br2 main^{tree} 2>err &&
- test_i18ngrep "fatal: --merge-base only works with commits" err
+ test_i18ngrep "is a tree, not a commit" err
'
test_expect_success "$cmd --merge-base with no merge bases and two commits" '
base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40
--
2.42.0
^ permalink raw reply related
* [ANNOUNCE] Git Rev News edition 103
From: Christian Couder @ 2023-10-01 15:32 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jakub Narebski, Markus Jansen, Kaartic Sivaraam,
Taylor Blau, Johannes Schindelin,
Ævar Arnfjörð Bjarmason, Ronan Pigott,
Adam Johnson, Bruno Brito, M Hickford,
Štěpán Němec, lwn
Hi everyone,
The 103rd edition of Git Rev News is now published:
https://git.github.io/rev_news/2023/09/30/edition-103/
Thanks a lot to Adam Johnson, Bruno Brito, Mirth Hickford and Štěpán
Němec who helped this month!
Enjoy,
Christian, Jakub, Markus and Kaartic.
PS: An issue for the next edition is already opened and contributions
are welcome:
https://github.com/git/git.github.io/issues/666
^ permalink raw reply
* [PATCH] git-status.txt: fix minor asciidoc format issue
From: cousteau via GitGitGadget @ 2023-10-01 20:52 UTC (permalink / raw)
To: git; +Cc: cousteau, Javier Mora
From: Javier Mora <cousteaulecommandant@gmail.com>
The paragraph below the list of short option combinations
isn't correctly formatted, making the result hard to read.
Signed-off-by: Javier Mora <cousteaulecommandant@gmail.com>
---
git-status.txt: minor asciidoc format correction
The paragraph below the list of short option combinations was hard to
read; turns out it wasn't correctly formatted in asciidoc.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1591%2Fcousteaulecommandant%2Fman-git-status-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1591/cousteaulecommandant/man-git-status-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1591
Documentation/git-status.txt | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index a051b1e8f38..48f46eb2047 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -245,10 +245,11 @@ U U unmerged, both modified
....
Submodules have more state and instead report
- M the submodule has a different HEAD than
- recorded in the index
- m the submodule has modified content
- ? the submodule has untracked files
+
+* 'M' = the submodule has a different HEAD than recorded in the index
+* 'm' = the submodule has modified content
+* '?' = the submodule has untracked files
+
since modified content or untracked files in a submodule cannot be added
via `git add` in the superproject to prepare a commit.
base-commit: bcb6cae2966cc407ca1afc77413b3ef11103c175
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] git-status.txt: fix minor asciidoc format issue
From: Junio C Hamano @ 2023-10-01 21:57 UTC (permalink / raw)
To: cousteau via GitGitGadget, Josh Soref; +Cc: git, cousteau
In-Reply-To: <pull.1591.git.1696193527923.gitgitgadget@gmail.com>
"cousteau via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Javier Mora <cousteaulecommandant@gmail.com>
>
> The paragraph below the list of short option combinations
> isn't correctly formatted, making the result hard to read.
In <pull.1590.git.1695392082207.gitgitgadget@gmail.com>, Josh fixed
this issue a bit differently but in the same spirit, and the fix in
38a15f47 (Documentation/git-status: add missing line breaks,
2023-09-22) is about to hit the 'master' branch.
Thanks.
[Reference]
* https://lore.kernel.org/git/pull.1590.git.1695392082207.gitgitgadget@gmail.com/
^ permalink raw reply
* Re: Possible to update-ref remote repository?
From: Jesse Hopkins @ 2023-10-01 22:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqo7hjfumj.fsf@gitster.g>
Thanks for the reply. Would you be able to point me to some
breadcrumbs for the "required protocol messages"? I might try to
tinker in some spare time.
FWIW, I had put something like this together a while back using
Gitolite hosting (a dedicated SSH command similar to Gitolite's
symbolic-ref command:
https://github.com/sitaramc/gitolite/blob/master/src/commands/symbolic-ref).
Our org is using Gitlab now, and I have been able to put some
functionality together using the Gitlab API's, but it's quite ugly,
and was hoping that maybe there was an intrinsic git protocol
solution, which seems there could be, but somewhere past trivial and
before impossible.
-Jesse
On Sat, Sep 30, 2023 at 11:17 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jesse Hopkins <jesse.hops@gmail.com> writes:
>
> > Wondering if it's possible to do the equivalent of the update-ref
> > command remotely. Or I guess another way of putting it would be to
> > git-push to a remote repository without needing a local clone of the
> > repo.
> >
> > Trying do something like:
> >
> > git push <remote-repo-url> <sha1>:refs/heads/mybranchtoupdate
> >
> > where I know that <sha1> already exists on the remote. I'd like to
> > avoid the need to clone a local copy of the repo. Wondering if there
> > might be some plumbing command(s) that could accomplish this?
>
> There is no such command shipped with Git. I do not think anybody
> ever proposed to add such a feature, as it won't be generally useful
> in contexts other than one-shot surgery.
>
> But that is different from that it is impossible to do. At the
> protocol level, as long as the receiving end is convinced that such
> a push, which does not transfer any object but proposes to update a
> ref to a new value, does not corrupt the resulting repository state
> after acceptingit, the receiving end does not care if the sending
> end has what object, or even any repository---the receiving end
> cannot even know what the sending end actually has.
>
> The devil however is in the details of ensuring the "as long as the
> receiving end is convinced" part. If you know that an object whose
> object name is X is sitting at the tip of branch A at the remote,
> and you try to update the tip of branch B to the same object, it is
> likely [*] that the remote would notice that such an update after
> receiving no new objects is safe. But if the object X is not
> sitting at the tip of any branch or ref (it may be a few commits
> behind an existing ref, or it may be dangling ahead of all refs), it
> would depend on how the receiving end determines that it has object
> chains necessary to complete the new DAG that has X. The official
> versions of Git client historically have done a thorough job in
> check_connected() to even discover fully connected object chains
> that allows us to resurrect such a dangling tip of a DAG, but we
> cannot complain if third-party Git implementations misses less
> obvious cases.
>
> Having said all that, writing a specialized "push lookalike" that
> sends the required protocol message that would have been spewed by a
> real "git push" client operating in the right environment (i.e., it
> has a repository that is a good clone of the receiving repository at
> the <remote-repo-url>, the receiving repository has <sha1> and all
> the objects that are reachable from it, refs/heads/mybranchtoupdate
> points at a commit that is an ancestor of <sha1> right now) should
> not be brain surgery, and with such a program you can
>
> $ git-push-empty <remote-repo-url> <sha1>:refs/heads/mybranchtoupdate
>
> and fool the receiving end to do what you want it to do, as it
> cannot tell if it is talking to a real "git push" client or to your
> "push-empty" program from what is coming over the wire.
>
>
>
^ permalink raw reply
* Re: [PATCH] git-status.txt: fix minor asciidoc format issue
From: Javier Mora @ 2023-10-01 23:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: cousteau via GitGitGadget, Josh Soref, git
In-Reply-To: <xmqq7co6f1le.fsf@gitster.g>
Yeah, I saw that right after I submitted my patch :(
Only problem is that that patch doesn't use the same formatting as the
rest of the document (uses a code block / preformatted text instead of
a list as other options in the document do) so my version of the patch
is just a minor cosmetic improvement now. (In any case, the result is
quite similar in the plain text manpage version; the change is more
noticeable in the HTML version though.)
If you're still interested in the "format as a list" issue and it
makes things easier to you, I can submit a merge of both commits
(fixing the merge conflict).
El dom, 1 oct 2023 a las 22:57, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> "cousteau via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Javier Mora <cousteaulecommandant@gmail.com>
> >
> > The paragraph below the list of short option combinations
> > isn't correctly formatted, making the result hard to read.
>
> In <pull.1590.git.1695392082207.gitgitgadget@gmail.com>, Josh fixed
> this issue a bit differently but in the same spirit, and the fix in
> 38a15f47 (Documentation/git-status: add missing line breaks,
> 2023-09-22) is about to hit the 'master' branch.
>
> Thanks.
>
> [Reference]
>
> * https://lore.kernel.org/git/pull.1590.git.1695392082207.gitgitgadget@gmail.com/
>
^ permalink raw reply
* Re: [PATCH 21/30] repository: Implement extensions.compatObjectFormat
From: Eric W. Biederman @ 2023-10-02 0:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <xmqqedigizms.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Eric W. Biederman" <ebiederm@gmail.com> writes:
>
>> Did you have any manual merge conflicts you had to resolve?
>> If so it is possible to see the merge result you had?
>
> The only merge-fix I had to apply to make everything compile was
> this:
>
> diff --git a/bloom.c b/bloom.c
> index ff131893cd..59eb0a0481 100644
> --- a/bloom.c
> +++ b/bloom.c
> @@ -278,7 +278,7 @@ static int has_entries_with_high_bit(struct repository *r, struct tree *t)
> struct tree_desc desc;
> struct name_entry entry;
>
> - init_tree_desc(&desc, t->buffer, t->size);
> + init_tree_desc(&desc, &t->object.oid, t->buffer, t->size);
> while (tree_entry(&desc, &entry)) {
> size_t i;
> for (i = 0; i < entry.pathlen; i++) {
>
> as one topic changed the function signature while the other topic
> added a new callsite.
>
> Everything else was pretty-much auto resolved, I think.
>
> Output from "git show --cc seen" matches my recollection. The above
> does appear as an evil merge.
Thanks, and I found all of this on your seen branch.
After tracking all of these down it appears all of the errors
came from my branch, I will be resending the patches as soon
as I finish going through the review comments.
Looking at the build errors pretty much all of the all of the
automatic test failures came from commit_tree_extended.
There was a strbuf that did
strbuf_init(&buf, 8192);
strbuf_init(&buf, 8192);
twice.
Plus there was another buffer that was allocated and not freed,
in commit_tree_extended.
The leaks were a bit tricky to track down as building with SANITIZE=leak
causes tests to fail somewhat randomly for me with "gcc (Debian
12.2.0-14) 12.2.0".
There was one smatch static-analysis error that suggested using
CALLOC_ARRAY instead of xcalloc.
The "win" build and "linux-gcc-default (ubuntu-lastest)" build failed
because of an over eager gcc warning -Werror=array-bounds.
Claiming:
In file included from /usr/include/string.h:535,
from git-compat-util.h:228,
from commit.c:1:
In function ‘memcpy’,
inlined from ‘oidcpy’ at hash-ll.h:272:2,
inlined from ‘commit_tree_extended’ at commit.c:1705:3:
##[error]/usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:10: ‘__builtin_memcpy’ offset [0, 31] is out of the bounds [0, 0] [-Werror=array-bounds]
29 | return __builtin___memcpy_chk (__dest, __src, __len,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
30 | __glibc_objsize0 (__dest));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
Previously in commit_tree_extended the structure was:
while (parents) {
struct commit *parent = pop_commit(&parents);
strbuf_addf(&buffer, "parent %s\n",
oid_to_hex(&parent->object.oid));
}
brian had changed it to:
nparents = commit_list_count(parents);
parent_buf = xcalloc(nparents, sizeof(*parent_buf));
for (i = 0; i < nparents; i++) {
struct commit *parent = pop_commit(&parents);
oidcpy(&parent_buf[i], &parent->object.oid);
}
Which is perfectly sound code.
I changed the structure of the loop to:
nparents = commit_list_count(parents);
parent_buf = xcalloc(nparents, sizeof(*parent_buf));
i = 0;
while (parents) {
struct commit *parent = pop_commit(&parents);
oidcpy(&parent_buf[i++], &parent->object.oid);
}
And the "array-bounds" warning had no problems with the code.
So it looks like the error was actually that array-bounds thought
there was a potential NULL pointer dereference at which point
it would not have array bounds, and then it complained about
the array bounds, instead of the NULL pointer dereference.
I am going to fix the patch. If array-bounds causes further
problems you may want to think about disabling it.
Eric
^ permalink raw reply
* Re: [PATCH 01/30] object-file-convert: Stubs for converting from one object format to another
From: Eric W. Biederman @ 2023-10-02 1:22 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Junio C Hamano, git, brian m. carlson, Eric W. Biederman
In-Reply-To: <CAPig+cRshiUNXfU=ZY4nZXgBgTJ_wF0WVDxWpqkEKPAT9pjX_w@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> 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`.
The "return -1" is there only because it is a stub, and it is there
where the rest of the code needs to go.
As for simple cases the "if (from == to)" case is a simple case I am
getting out of the way. It unfortunately is cluttered by the fact
that "oidcpy(&oid, &oid)" is not valid so it has to guard the copy.
if (from == to) {
if (src == dest)
return 0;
oidcpy(dest, src);
return 0;
}
Could be used but is wordier. And duplicates the return code
for the same case so I am not enthusiastic about it.
>> +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)
I honestly can not be confident of C code that does that.
The precedence of the operators in C has been wrong for longer than I
have been programming, and I can never remember exactly how the
precedence is wrong. So for the last 30 years I have been adding enough
parenthesis that I don't have to remember.
> 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().
Agreed BUG would be better there.
> 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.
It is a string that represents the entire object.
It might be arguable if tree objects are text given that trees represent
oids in binary, but for tag and commit objects they are definitely one
big text string.
This is an implementation detail that makes the code simpler, and less
error prone.
I have not encountered anything where a string buffer would not be
a reasonable fit.
>> + die(_("Failed to convert object from %s to %s"),
>> + from->name, to->name);
>
> s/Failed/failed/
I don't understand wanting to start a sentence with a lower case letter.
Can you explain?
> 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?
I expect some of that context will come from the conversion functions
themselves. Those messages are almost certain to give the type
information one way or another because they are type specific.
That said it requires some version of corrupt repository to reach this
error. Either missing mapping tables, or a corrupt object.
So I don't know how much it matters to get this perfect the first
time. We can improve the error message we gain experience.
I would agree that including the OID makes sense. Unfortunately the
code does not have the OID at this point.
>> 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.
I could use words that repeat what is in the function signature.
But I don't think I could add anything.
I would have to say something like:
Look up the oid that an equivalent object would have in a repository
whose object format is "to".
Is that helpful?
Eric
^ permalink raw reply
* Re: [PATCH 21/30] repository: Implement extensions.compatObjectFormat
From: Eric W. Biederman @ 2023-10-02 1:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric W. Biederman
In-Reply-To: <xmqqfs2zl2iy.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Eric W. Biederman" <ebiederm@gmail.com> writes:
>
>> 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).
Why shouldn't sentences begin with a capital letter?
I agree it isn't a very extensive sentence just two words but it is a
complete thought, and thus is a sentence.
There is great value in uniformity in a project so I will make the
change, but it seems very weird to me.
Eric
^ permalink raw reply
* Re: [PATCH 03/30] object-names: Support input of oids in any supported hash
From: Eric W. Biederman @ 2023-10-02 1:54 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Junio C Hamano, git, brian m. carlson, Eric W. Biederman
In-Reply-To: <CAPig+cRHBxPZYQ5XYA5Un7LeS21NgqxZGg=Q8D+aQckrw9Ymtg@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> 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/
Actually s/of/if/
Thank you for catching that. When reviewing this to understand what I
was trying to say I found a bug. The function repo_for_each_abbrev was
passing algo to init_object_disambiguation to properly initialize
ds.bin_pfx.algo, and then was resetting ds.bin_pfx.algo to GIT_HASH_ANY.
Oops!
>> 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"?
No. I forgot to remove "the" before 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
Fair 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
Naughty thing how did that sneak in there ;)
Eric
^ permalink raw reply
* Re: [PATCH 05/30] loose: add a mapping between SHA-1 and SHA-256 for loose objects
From: Eric W. Biederman @ 2023-10-02 2:11 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Junio C Hamano, git, brian m. carlson, Eric W . Biederman
In-Reply-To: <CAPig+cS02ushqgw+u39Tmnoy3rgp8BzqT4T9D=-01m5fsLxC6Q@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Wed, Sep 27, 2023 at 3:56 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
>> As part of the transition plan, we'd like to add a file in the .git
>> directory that maps loose objects between SHA-1 and SHA-256. Let's
>> implement the specification in the transition plan and store this data
>> on a per-repository basis in struct repository.
>>
>> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
>> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
>> ---
>> diff --git a/loose.c b/loose.c
>> @@ -0,0 +1,245 @@
>> +static int load_one_loose_object_map(struct repository *repo, struct object_directory *dir)
>> +{
>> + struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
>> + FILE *fp;
>> +
>> + if (!dir->loose_map)
>> + loose_object_map_init(&dir->loose_map);
>> +
>> + insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->empty_tree, repo->compat_hash_algo->empty_tree);
>> + insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->empty_tree, repo->hash_algo->empty_tree);
>> +
>> + insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);
>> + insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->empty_blob, repo->hash_algo->empty_blob);
>> +
>> + insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);
>> + insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->null_oid, repo->hash_algo->null_oid);
>> +
>> + strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
>> + fp = fopen(path.buf, "rb");
>> + if (!fp)
>> + return 0;
>
> This early return leaks `path`. At minimum, call
> `strbuf_release(&path)` before returning.
yep. Fixed.
>> + errno = 0;
>> + if (strbuf_getwholeline(&buf, fp, '\n') || strcmp(buf.buf, loose_object_header))
>> + goto err;
>> + while (!strbuf_getline_lf(&buf, fp)) {
>> + const char *p;
>> + struct object_id oid, compat_oid;
>> + if (parse_oid_hex_algop(buf.buf, &oid, &p, repo->hash_algo) ||
>> + *p++ != ' ' ||
>> + parse_oid_hex_algop(p, &compat_oid, &p, repo->compat_hash_algo) ||
>> + p != buf.buf + buf.len)
>> + goto err;
>> + insert_oid_pair(dir->loose_map->to_compat, &oid, &compat_oid);
>> + insert_oid_pair(dir->loose_map->to_storage, &compat_oid, &oid);
>> + }
>> +
>> + strbuf_release(&buf);
>> + strbuf_release(&path);
>> + return errno ? -1 : 0;
>> +err:
>> + strbuf_release(&buf);
>> + strbuf_release(&path);
>> + return -1;
>> +}
>> +
>> +int repo_write_loose_object_map(struct repository *repo)
>> +{
>> + kh_oid_map_t *map = repo->objects->odb->loose_map->to_compat;
>> + struct lock_file lock;
>> + int fd;
>> + khiter_t iter;
>> + struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
>> +
>> + if (!should_use_loose_object_map(repo))
>> + return 0;
>> +
>> + strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
>> + fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1);
>> + iter = kh_begin(map);
>> + if (write_in_full(fd, loose_object_header, strlen(loose_object_header)) < 0)
>> + goto errout;
>> +
>> + for (; iter != kh_end(map); iter++) {
>> + if (kh_exist(map, iter)) {
>> + if (oideq(&kh_key(map, iter), the_hash_algo->empty_tree) ||
>> + oideq(&kh_key(map, iter), the_hash_algo->empty_blob))
>> + continue;
>> + strbuf_addf(&buf, "%s %s\n", oid_to_hex(&kh_key(map, iter)), oid_to_hex(kh_value(map, iter)));
>> + if (write_in_full(fd, buf.buf, buf.len) < 0)
>> + goto errout;
>> + strbuf_reset(&buf);
>> + }
>> + }
>
> Nit: If you call strbuf_reset() immediately before strbuf_addf(), then
> you save the reader of the code the effort of having to scan backward
> through the function to verify that `buf` is empty the first time
> through the loop.
I am actually perplexed why the code works this way.
My gut says we should create the entire buffer in memory and then
write it to disk all in a single system call, and not perform
this line buffering.
Doing that would remove the need for the strbuf_reset entirely,
and would just require the buffer to be primed with the
loose_object_header.
But what is there works and I will leave it for now.
It isn't a bug, and it can be improved with a follow up patch.
>> + strbuf_release(&buf);
>> + if (commit_lock_file(&lock) < 0) {
>> + error_errno(_("could not write loose object index %s"), path.buf);
>> + strbuf_release(&path);
>> + return -1;
>> + }
>> + strbuf_release(&path);
>> + return 0;
>> +errout:
>> + rollback_lock_file(&lock);
>> + strbuf_release(&buf);
>> + error_errno(_("failed to write loose object index %s\n"), path.buf);
>> + strbuf_release(&path);
>> + return -1;
>> +}
>> +
>> +int repo_loose_object_map_oid(struct repository *repo,
>> + const struct object_id *src,
>> + const struct git_hash_algo *to,
>> + struct object_id *dest)
>> +
>> +{
>
> Style: unnecessary blank line before opening `{`
Yep. Fixed.
>
>> + struct object_directory *dir;
>> + kh_oid_map_t *map;
>> + khiter_t pos;
>> +
>> + for (dir = repo->objects->odb; dir; dir = dir->next) {
>> + struct loose_object_map *loose_map = dir->loose_map;
>> + if (!loose_map)
>> + continue;
>> + map = (to == repo->compat_hash_algo) ?
>> + loose_map->to_compat :
>> + loose_map->to_storage;
>> + pos = kh_get_oid_map(map, *src);
>> + if (pos < kh_end(map)) {
>> + oidcpy(dest, kh_value(map, pos));
>> + return 0;
>> + }
>> + }
>> + return -1;
>> +}
Eric
^ permalink raw reply
* Re: [PATCH 01/30] object-file-convert: Stubs for converting from one object format to another
From: Eric Sunshine @ 2023-10-02 2:27 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Junio C Hamano, git, brian m. carlson, Eric W. Biederman
In-Reply-To: <87o7hhbyxi.fsf@gmail.froward.int.ebiederm.org>
On Sun, Oct 1, 2023 at 9:22 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > On Wed, Sep 27, 2023 at 3:55 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
> >> + die(_("Failed to convert object from %s to %s"),
> >> + from->name, to->name);
> >
> > s/Failed/failed/
>
> I don't understand wanting to start a sentence with a lower case letter.
> Can you explain?
Consistency with most of the rest of the messages emitted by Git.
CodingGuidelines call for lowercase and omission of the full-stop
(period).
The choice, of course, is subjective, but these are the guidelines
upon which the project has settled for better or worse. (You can
certainly find older code, which predates the guideline, using
capitalized messages, but it's good to adhere to the guideline for new
code if possible.)
> >> +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.
>
> I could use words that repeat what is in the function signature.
> But I don't think I could add anything.
>
> I would have to say something like:
>
> Look up the oid that an equivalent object would have in a repository
> whose object format is "to".
>
> Is that helpful?
That would help clarify the function a bit for me. Explaining the
function's return value could also be useful.
^ permalink raw reply
* Re: [PATCH 05/30] loose: add a mapping between SHA-1 and SHA-256 for loose objects
From: Eric Sunshine @ 2023-10-02 2:36 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Junio C Hamano, git, brian m. carlson, Eric W . Biederman
In-Reply-To: <87v8bp93j5.fsf@gmail.froward.int.ebiederm.org>
On Sun, Oct 1, 2023 at 10:11 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > On Wed, Sep 27, 2023 at 3:56 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
> >> + for (; iter != kh_end(map); iter++) {
> >> + if (kh_exist(map, iter)) {
> >> + if (oideq(&kh_key(map, iter), the_hash_algo->empty_tree) ||
> >> + oideq(&kh_key(map, iter), the_hash_algo->empty_blob))
> >> + continue;
> >> + strbuf_addf(&buf, "%s %s\n", oid_to_hex(&kh_key(map, iter)), oid_to_hex(kh_value(map, iter)));
> >> + if (write_in_full(fd, buf.buf, buf.len) < 0)
> >> + goto errout;
> >> + strbuf_reset(&buf);
> >> + }
> >> + }
> >
> > Nit: If you call strbuf_reset() immediately before strbuf_addf(), then
> > you save the reader of the code the effort of having to scan backward
> > through the function to verify that `buf` is empty the first time
> > through the loop.
>
> I am actually perplexed why the code works this way.
>
> My gut says we should create the entire buffer in memory and then
> write it to disk all in a single system call, and not perform
> this line buffering.
I think I had a similar question/thought while reading the patch but...
> Doing that would remove the need for the strbuf_reset entirely,
> and would just require the buffer to be primed with the
> loose_object_header.
>
> But what is there works and I will leave it for now.
... came to this same conclusion.
> It isn't a bug, and it can be improved with a follow up patch.
Exactly.
I haven't thought through the consequences, but perhaps brian was
worried about the memory footprint of buffering the whole thing before
writing to disk.
^ permalink raw reply
* [PATCH v2 00/30] initial support for multiple hash functions
From: Eric W. Biederman @ 2023-10-02 2:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric Sunshine, Eric W. Biederman
In-Reply-To: <87jzsbjt0a.fsf@gmail.froward.int.ebiederm.org>
This addresses all of the known test failures from v1 of this set of
changes. In particular I have reworked commit_tree_extended which
was flagged by smatch, -Werror=array-bounds, and the leak detector.
One functional bug was fixed in repo_for_each_abbrev where it was
mistakenly displaying too many ambiguous oids.
I am posting this so that people review and testing of this patchset
won't be distracted by the known and fixed issues.
A key part of the hash function transition plan is a way that a single
git repository can inter-operate with git repositories whose storage
hash function is SHA-1 and git repositories whose storage hash function
is SHA-256.
This interoperability can defined in terms of two repositories one whose
storage hash function is SHA-1 and another whose storage hash function
is SHA-256. Those two repositories receive exactly the same objects,
but they store them in different but equivalent ways.
For a repository that has one storage hash function to inter-operate
with a repository that has a different storage hash function requires
the first repository to be able produce it's objects as if they were
stored in the second hash function.
This series of changes focuses on implementing the pieces that allow
a repository that uses one storage hash function to produce the objects
that would have been stored with a second storage hash function.
The final patch in this series is the addition of a test that creates
two repositories one that uses SHA-1 as it's storage hash function
and the other that uses SHA-256 as it's storage hash function.
Identical operations are performed on the two repositories, and their
compatibility objects are compared to verify they are the same.
AKA the SHA-1 repository on the fly generates the objects store in
the SHA-256 repository, and the SHA-256 repository on the fly generates
the objects that are stored in the SHA-1 repository.
There are two fundamental technologies for enabling this.
- The ability to convert a stored object into the object the
other repository would have stored.
- The ability to remember a mapping between SHA-1 and SHA-256 oids
of equivalent objects.
With such technologies it is very easy to implement user facing changes.
To avoid locking git into poor decisions by accident I have done my best
to minimize the user facing changes, while still building the internal
infrastructure that is needed for interoperability.
All of this work is inspired by earlier work on interoperability by
"brian m. carlson" and some of the key pieces of code are still his.
To get to the point where I can test if a SHA-1 and a SHA-256 repository
can on the fly generate each other, I have made some small user-facing
changes.
git rev-parse now supports --output-object-format as a way to query
the internal mapping tables between oids and report the equivalent
oid of the other format.
git cat-file when given a oid that does not match the repositories
storage format will now attempt to find the oids equivalent object that
is stored in the repository and if found dynamically generate the object
that would have been stored in a repository with a different storage
hash function and display the object.
An additional file loose-object-index will be stored in ".git/objects/".
An additional option "extensions.compatObjectFormat" is implemented,
that generates and stores mappings between the oids of objects stored in
the repository and oids of the equivalent objects that would be stored
in a repository show storage format was extensions.compatObjectFormat.
Eric W. Biederman (23):
object-file-convert: stubs for converting from one object format to another
oid-array: teach oid-array to handle multiple kinds of oids
object-names: support input of oids in any supported hash
repository: add a compatibility hash algorithm
loose: compatibilty short name support
object-file: update the loose object map when writing loose objects
object-file: add a compat_oid_in parameter to write_object_file_flags
commit: convert mergetag before computing the signature of a commit
commit: export add_header_signature to support handling signatures on tags
tag: sign both hashes
object: factor out parse_mode out of fast-import and tree-walk into in object.h
object-file-convert: don't leak when converting tag objects
object-file-convert: convert commits that embed signed tags
object-file: update object_info_extended to reencode objects
rev-parse: add an --output-object-format parameter
builtin/cat-file: let the oid determine the output algorithm
tree-walk: init_tree_desc take an oid to get the hash algorithm
object-file: handle compat objects in check_object_signature
builtin/ls-tree: let the oid determine the output algorithm
test-lib: compute the compatibility hash so tests may use it
t1006: rename sha1 to oid
t1006: test oid compatibility with cat-file
t1016-compatObjectFormat: add tests to verify the conversion between objects
brian m. carlson (7):
loose: add a mapping between SHA-1 and SHA-256 for loose objects
commit: write commits for both hashes
cache: add a function to read an OID of a specific algorithm
object-file-convert: add a function to convert trees between algorithms
object-file-convert: convert tag objects when writing
object-file-convert: convert commit objects when writing
repository: implement extensions.compatObjectFormat
Documentation/config/extensions.txt | 12 ++
Documentation/git-rev-parse.txt | 12 ++
Makefile | 3 +
archive.c | 3 +-
builtin/am.c | 6 +-
builtin/cat-file.c | 12 +-
builtin/checkout.c | 8 +-
builtin/clone.c | 2 +-
builtin/commit.c | 2 +-
builtin/fast-import.c | 18 +-
builtin/grep.c | 8 +-
builtin/ls-tree.c | 5 +-
builtin/merge.c | 3 +-
builtin/pack-objects.c | 6 +-
builtin/read-tree.c | 2 +-
builtin/rev-parse.c | 25 ++-
builtin/stash.c | 5 +-
builtin/tag.c | 45 ++++-
cache-tree.c | 4 +-
commit.c | 221 ++++++++++++++++-----
commit.h | 1 +
delta-islands.c | 2 +-
diff-lib.c | 2 +-
fsck.c | 6 +-
hash-ll.h | 1 +
hash.h | 9 +-
http-push.c | 2 +-
list-objects.c | 2 +-
loose.c | 259 ++++++++++++++++++++++++
loose.h | 22 +++
match-trees.c | 4 +-
merge-ort.c | 11 +-
merge-recursive.c | 2 +-
merge.c | 3 +-
object-file-convert.c | 277 ++++++++++++++++++++++++++
object-file-convert.h | 24 +++
object-file.c | 212 ++++++++++++++++++--
object-name.c | 46 +++--
object-name.h | 3 +-
object-store-ll.h | 7 +-
object.c | 2 +
object.h | 18 ++
oid-array.c | 12 +-
pack-bitmap-write.c | 2 +-
packfile.c | 3 +-
reflog.c | 2 +-
repository.c | 14 ++
repository.h | 4 +
revision.c | 4 +-
setup.c | 22 +++
setup.h | 1 +
t/helper/test-delete-gpgsig.c | 62 ++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t1006-cat-file.sh | 379 +++++++++++++++++++++---------------
t/t1016-compatObjectFormat.sh | 281 ++++++++++++++++++++++++++
t/t1016/gpg | 2 +
t/test-lib-functions.sh | 17 +-
tree-walk.c | 58 +++---
tree-walk.h | 7 +-
tree.c | 2 +-
walker.c | 2 +-
62 files changed, 1844 insertions(+), 349 deletions(-)
Eric
^ permalink raw reply
* [PATCH v2 01/30] object-file-convert: stubs for converting from one object format to another
From: Eric W. Biederman @ 2023-10-02 2:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric Sunshine, Eric W. Biederman
In-Reply-To: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
From: "Eric W. Biederman" <ebiederm@xmission.com>
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 conversation to be done and it is an
error to use this function on them.
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 equivalent object file generated
with the target hash algorithm.
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>
---
Makefile | 1 +
object-file-convert.c | 57 +++++++++++++++++++++++++++++++++++++++++++
object-file-convert.h | 24 ++++++++++++++++++
3 files changed, 82 insertions(+)
create mode 100644 object-file-convert.c
create mode 100644 object-file-convert.h
diff --git a/Makefile b/Makefile
index 577630936535..f7e824f25cda 100644
--- a/Makefile
+++ b/Makefile
@@ -1073,6 +1073,7 @@ LIB_OBJS += notes-cache.o
LIB_OBJS += notes-merge.o
LIB_OBJS += notes-utils.o
LIB_OBJS += notes.o
+LIB_OBJS += object-file-convert.o
LIB_OBJS += object-file.o
LIB_OBJS += object-name.o
LIB_OBJS += object.o
diff --git a/object-file-convert.c b/object-file-convert.c
new file mode 100644
index 000000000000..4777aba83636
--- /dev/null
+++ b/object-file-convert.c
@@ -0,0 +1,57 @@
+#include "git-compat-util.h"
+#include "gettext.h"
+#include "strbuf.h"
+#include "repository.h"
+#include "hash-ll.h"
+#include "object.h"
+#include "object-file-convert.h"
+
+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 algorithm is not set, then we're using the
+ * default hash algorithm for that object.
+ */
+ 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;
+}
+
+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))
+ BUG("Refusing noop object file conversion");
+
+ 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;
+ }
+ die(_("Failed to convert object from %s to %s"),
+ from->name, to->name);
+}
diff --git a/object-file-convert.h b/object-file-convert.h
new file mode 100644
index 000000000000..a4f802aa8eea
--- /dev/null
+++ b/object-file-convert.h
@@ -0,0 +1,24 @@
+#ifndef OBJECT_CONVERT_H
+#define OBJECT_CONVERT_H
+
+struct repository;
+struct object_id;
+struct git_hash_algo;
+struct strbuf;
+#include "object.h"
+
+int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
+ const struct git_hash_algo *to, struct object_id *dest);
+
+/*
+ * 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);
+
+#endif /* OBJECT_CONVERT_H */
--
2.41.0
^ permalink raw reply related
* [PATCH v2 02/30] oid-array: teach oid-array to handle multiple kinds of oids
From: Eric W. Biederman @ 2023-10-02 2:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric Sunshine, Eric W. Biederman
In-Reply-To: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
From: "Eric W. Biederman" <ebiederm@xmission.com>
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
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 different kinds of
oids simultaneously.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
oid-array.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/oid-array.c b/oid-array.c
index 8e4717746c31..1f36651754ed 100644
--- a/oid-array.c
+++ b/oid-array.c
@@ -6,12 +6,20 @@ void oid_array_append(struct oid_array *array, const struct object_id *oid)
{
ALLOC_GROW(array->oid, array->nr + 1, array->alloc);
oidcpy(&array->oid[array->nr++], oid);
+ if (!oid->algo)
+ oid_set_algo(&array->oid[array->nr - 1], the_hash_algo);
array->sorted = 0;
}
-static int void_hashcmp(const void *a, const void *b)
+static int void_hashcmp(const void *va, const void *vb)
{
- return oidcmp(a, b);
+ const struct object_id *a = va, *b = vb;
+ int ret;
+ if (a->algo == b->algo)
+ ret = oidcmp(a, b);
+ else
+ ret = a->algo > b->algo ? 1 : -1;
+ return ret;
}
void oid_array_sort(struct oid_array *array)
--
2.41.0
^ permalink raw reply related
* [PATCH v2 03/30] object-names: support input of oids in any supported hash
From: Eric W. Biederman @ 2023-10-02 2:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric Sunshine, Eric W. Biederman
In-Reply-To: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
From: "Eric W. Biederman" <ebiederm@xmission.com>
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 accepted 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 if GIT_OID_HASH_ANY was not supplied. 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>
---
builtin/rev-parse.c | 2 +-
hash-ll.h | 1 +
object-name.c | 46 ++++++++++++++++++++++++++++++++++-----------
object-name.h | 3 ++-
4 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index fde8861ca4e0..43e96765400c 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -882,7 +882,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
}
if (skip_prefix(arg, "--disambiguate=", &arg)) {
- repo_for_each_abbrev(the_repository, arg,
+ repo_for_each_abbrev(the_repository, arg, the_hash_algo,
show_abbrev, NULL);
continue;
}
diff --git a/hash-ll.h b/hash-ll.h
index 10d84cc20888..2cfde63ae1cf 100644
--- a/hash-ll.h
+++ b/hash-ll.h
@@ -145,6 +145,7 @@ struct object_id {
#define GET_OID_RECORD_PATH 0200
#define GET_OID_ONLY_TO_DIE 04000
#define GET_OID_REQUIRE_PATH 010000
+#define GET_OID_HASH_ANY 020000
#define GET_OID_DISAMBIGUATORS \
(GET_OID_COMMIT | GET_OID_COMMITTISH | \
diff --git a/object-name.c b/object-name.c
index 0bfa29dbbfe9..7dd6e5e47566 100644
--- a/object-name.c
+++ b/object-name.c
@@ -25,6 +25,7 @@
#include "midx.h"
#include "commit-reach.h"
#include "date.h"
+#include "object-file-convert.h"
static int get_oid_oneline(struct repository *r, const char *, struct object_id *, struct commit_list *);
@@ -49,6 +50,7 @@ struct disambiguate_state {
static void update_candidates(struct disambiguate_state *ds, const struct object_id *current)
{
+ /* The hash algorithm of current has already been filtered */
if (ds->always_call_fn) {
ds->ambiguous = ds->fn(ds->repo, current, ds->cb_data) ? 1 : 0;
return;
@@ -134,6 +136,8 @@ static void unique_in_midx(struct multi_pack_index *m,
{
uint32_t num, i, first = 0;
const struct object_id *current = NULL;
+ int len = ds->len > ds->repo->hash_algo->hexsz ?
+ ds->repo->hash_algo->hexsz : ds->len;
num = m->num_objects;
if (!num)
@@ -149,7 +153,7 @@ static void unique_in_midx(struct multi_pack_index *m,
for (i = first; i < num && !ds->ambiguous; i++) {
struct object_id oid;
current = nth_midxed_object_oid(&oid, m, i);
- if (!match_hash(ds->len, ds->bin_pfx.hash, current->hash))
+ if (!match_hash(len, ds->bin_pfx.hash, current->hash))
break;
update_candidates(ds, current);
}
@@ -159,6 +163,8 @@ static void unique_in_pack(struct packed_git *p,
struct disambiguate_state *ds)
{
uint32_t num, i, first = 0;
+ int len = ds->len > ds->repo->hash_algo->hexsz ?
+ ds->repo->hash_algo->hexsz : ds->len;
if (p->multi_pack_index)
return;
@@ -177,7 +183,7 @@ static void unique_in_pack(struct packed_git *p,
for (i = first; i < num && !ds->ambiguous; i++) {
struct object_id oid;
nth_packed_object_id(&oid, p, i);
- if (!match_hash(ds->len, ds->bin_pfx.hash, oid.hash))
+ if (!match_hash(len, ds->bin_pfx.hash, oid.hash))
break;
update_candidates(ds, &oid);
}
@@ -188,6 +194,10 @@ static void find_short_packed_object(struct disambiguate_state *ds)
struct multi_pack_index *m;
struct packed_git *p;
+ /* Skip, unless oids from the storage hash algorithm are wanted */
+ if (ds->bin_pfx.algo && (&hash_algos[ds->bin_pfx.algo] != ds->repo->hash_algo))
+ return;
+
for (m = get_multi_pack_index(ds->repo); m && !ds->ambiguous;
m = m->next)
unique_in_midx(m, ds);
@@ -326,11 +336,12 @@ int set_disambiguate_hint_config(const char *var, const char *value)
static int init_object_disambiguation(struct repository *r,
const char *name, int len,
+ const struct git_hash_algo *algo,
struct disambiguate_state *ds)
{
int i;
- if (len < MINIMUM_ABBREV || len > the_hash_algo->hexsz)
+ if (len < MINIMUM_ABBREV || len > GIT_MAX_HEXSZ)
return -1;
memset(ds, 0, sizeof(*ds));
@@ -357,6 +368,7 @@ static int init_object_disambiguation(struct repository *r,
ds->len = len;
ds->hex_pfx[len] = '\0';
ds->repo = r;
+ ds->bin_pfx.algo = algo ? hash_algo_by_ptr(algo) : GIT_HASH_UNKNOWN;
prepare_alt_odb(r);
return 0;
}
@@ -491,9 +503,10 @@ static int repo_collect_ambiguous(struct repository *r UNUSED,
return collect_ambiguous(oid, data);
}
-static int sort_ambiguous(const void *a, const void *b, void *ctx)
+static int sort_ambiguous(const void *va, const void *vb, void *ctx)
{
struct repository *sort_ambiguous_repo = ctx;
+ const struct object_id *a = va, *b = vb;
int a_type = oid_object_info(sort_ambiguous_repo, a, NULL);
int b_type = oid_object_info(sort_ambiguous_repo, b, NULL);
int a_type_sort;
@@ -503,8 +516,12 @@ static int sort_ambiguous(const void *a, const void *b, void *ctx)
* Sorts by hash within the same object type, just as
* oid_array_for_each_unique() would do.
*/
- if (a_type == b_type)
- return oidcmp(a, b);
+ if (a_type == b_type) {
+ if (a->algo == b->algo)
+ return oidcmp(a, b);
+ else
+ return a->algo > b->algo ? 1 : -1;
+ }
/*
* Between object types show tags, then commits, and finally
@@ -533,8 +550,12 @@ static enum get_oid_result get_short_oid(struct repository *r,
int status;
struct disambiguate_state ds;
int quietly = !!(flags & GET_OID_QUIETLY);
+ const struct git_hash_algo *algo = r->hash_algo;
+
+ if (flags & GET_OID_HASH_ANY)
+ algo = NULL;
- if (init_object_disambiguation(r, name, len, &ds) < 0)
+ if (init_object_disambiguation(r, name, len, algo, &ds) < 0)
return -1;
if (HAS_MULTI_BITS(flags & GET_OID_DISAMBIGUATORS))
@@ -588,7 +609,7 @@ static enum get_oid_result get_short_oid(struct repository *r,
if (!ds.ambiguous)
ds.fn = NULL;
- repo_for_each_abbrev(r, ds.hex_pfx, collect_ambiguous, &collect);
+ repo_for_each_abbrev(r, ds.hex_pfx, algo, collect_ambiguous, &collect);
sort_ambiguous_oid_array(r, &collect);
if (oid_array_for_each(&collect, show_ambiguous_object, &out))
@@ -610,13 +631,14 @@ static enum get_oid_result get_short_oid(struct repository *r,
}
int repo_for_each_abbrev(struct repository *r, const char *prefix,
+ const struct git_hash_algo *algo,
each_abbrev_fn fn, void *cb_data)
{
struct oid_array collect = OID_ARRAY_INIT;
struct disambiguate_state ds;
int ret;
- if (init_object_disambiguation(r, prefix, strlen(prefix), &ds) < 0)
+ if (init_object_disambiguation(r, prefix, strlen(prefix), algo, &ds) < 0)
return -1;
ds.always_call_fn = 1;
@@ -787,10 +809,12 @@ void strbuf_add_unique_abbrev(struct strbuf *sb, const struct object_id *oid,
int repo_find_unique_abbrev_r(struct repository *r, char *hex,
const struct object_id *oid, int len)
{
+ const struct git_hash_algo *algo =
+ oid->algo ? &hash_algos[oid->algo] : r->hash_algo;
struct disambiguate_state ds;
struct min_abbrev_data mad;
struct object_id oid_ret;
- const unsigned hexsz = r->hash_algo->hexsz;
+ const unsigned hexsz = algo->hexsz;
if (len < 0) {
unsigned long count = repo_approximate_object_count(r);
@@ -826,7 +850,7 @@ int repo_find_unique_abbrev_r(struct repository *r, char *hex,
find_abbrev_len_packed(&mad);
- if (init_object_disambiguation(r, hex, mad.cur_len, &ds) < 0)
+ if (init_object_disambiguation(r, hex, mad.cur_len, algo, &ds) < 0)
return -1;
ds.fn = repo_extend_abbrev_len;
diff --git a/object-name.h b/object-name.h
index 9ae522307148..064ddc97d1fe 100644
--- a/object-name.h
+++ b/object-name.h
@@ -67,7 +67,8 @@ enum get_oid_result get_oid_with_context(struct repository *repo, const char *st
typedef int each_abbrev_fn(const struct object_id *oid, void *);
-int repo_for_each_abbrev(struct repository *r, const char *prefix, each_abbrev_fn, void *);
+int repo_for_each_abbrev(struct repository *r, const char *prefix,
+ const struct git_hash_algo *algo, each_abbrev_fn, void *);
int set_disambiguate_hint_config(const char *var, const char *value);
--
2.41.0
^ permalink raw reply related
* [PATCH v2 04/30] repository: add a compatibility hash algorithm
From: Eric W. Biederman @ 2023-10-02 2:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric Sunshine, Eric W. Biederman
In-Reply-To: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
From: "Eric W. Biederman" <ebiederm@xmission.com>
We currently have support for using a full stage 4 SHA-256
implementation. However, we'd like to support interoperability with
SHA-1 repositories as well. The transition plan anticipates a
compatibility hash algorithm configuration option that we can use to
implement support for this. Let's add an element to the repository
structure that indicates the compatibility hash algorithm so we can use
it when we need to consider interoperability between algorithms.
Add a helper function repo_set_compat_hash_algo that takes a
compatibility hash algorithm and sets "repo->compat_hash_algo". If
GIT_HASH_UNKNOWN is passed as the compatibility hash algorithm
"repo->compat_hash_algo" is set to NULL.
For now, the code results in "repo->compat_hash_algo" always being set
to NULL, but that will change once a configuration option is added.
Inspired-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
repository.c | 8 ++++++++
repository.h | 4 ++++
setup.c | 3 +++
3 files changed, 15 insertions(+)
diff --git a/repository.c b/repository.c
index a7679ceeaa45..80252b79e93e 100644
--- a/repository.c
+++ b/repository.c
@@ -104,6 +104,13 @@ void repo_set_hash_algo(struct repository *repo, int hash_algo)
repo->hash_algo = &hash_algos[hash_algo];
}
+void repo_set_compat_hash_algo(struct repository *repo, int algo)
+{
+ if (hash_algo_by_ptr(repo->hash_algo) == algo)
+ BUG("hash_algo and compat_hash_algo match");
+ repo->compat_hash_algo = algo ? &hash_algos[algo] : NULL;
+}
+
/*
* Attempt to resolve and set the provided 'gitdir' for repository 'repo'.
* Return 0 upon success and a non-zero value upon failure.
@@ -184,6 +191,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->repository_format_worktree_config = format.worktree_config;
/* take ownership of format.partial_clone */
diff --git a/repository.h b/repository.h
index 5f18486f6465..bf3fc601cc53 100644
--- a/repository.h
+++ b/repository.h
@@ -160,6 +160,9 @@ struct repository {
/* Repository's current hash algorithm, as serialized on disk. */
const struct git_hash_algo *hash_algo;
+ /* Repository's compatibility hash algorithm. */
+ const struct git_hash_algo *compat_hash_algo;
+
/* A unique-id for tracing purposes. */
int trace2_repo_id;
@@ -199,6 +202,7 @@ void repo_set_gitdir(struct repository *repo, const char *root,
const struct set_gitdir_args *extra_args);
void repo_set_worktree(struct repository *repo, const char *path);
void repo_set_hash_algo(struct repository *repo, int algo);
+void repo_set_compat_hash_algo(struct repository *repo, int compat_algo);
void initialize_the_repository(void);
RESULT_MUST_BE_USED
int repo_init(struct repository *r, const char *gitdir, const char *worktree);
diff --git a/setup.c b/setup.c
index 18927a847b86..aa8bf5da5226 100644
--- a/setup.c
+++ b/setup.c
@@ -1564,6 +1564,8 @@ 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);
the_repository->repository_format_worktree_config =
repo_fmt.worktree_config;
/* take ownership of repo_fmt.partial_clone */
@@ -1657,6 +1659,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);
the_repository->repository_format_worktree_config =
fmt->worktree_config;
the_repository->repository_format_partial_clone =
--
2.41.0
^ permalink raw reply related
* [PATCH v2 06/30] loose: compatibilty short name support
From: Eric W. Biederman @ 2023-10-02 2:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric Sunshine, Eric W. Biederman
In-Reply-To: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
From: "Eric W. Biederman" <ebiederm@xmission.com>
Update loose_objects_cache when udpating the loose objects map. This
oidtree is used to discover which oids are possibilities when
resolving short names, and it can support a mixture of sha1
and sha256 oids.
With this any oid recorded objects/loose-objects-idx is usable
for resolving an oid to an object.
To make this maintainable a helper insert_loose_map is factored
out of load_one_loose_object_map and repo_add_loose_object_map,
and then modified to also update the loose_objects_cache.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
loose.c | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/loose.c b/loose.c
index 6ba73cc84dca..f6faa6216a08 100644
--- a/loose.c
+++ b/loose.c
@@ -7,6 +7,7 @@
#include "gettext.h"
#include "loose.h"
#include "lockfile.h"
+#include "oidtree.h"
static const char *loose_object_header = "# loose-object-idx\n";
@@ -42,6 +43,21 @@ static int insert_oid_pair(kh_oid_map_t *map, const struct object_id *key, const
return 1;
}
+static int insert_loose_map(struct object_directory *odb,
+ const struct object_id *oid,
+ const struct object_id *compat_oid)
+{
+ struct loose_object_map *map = odb->loose_map;
+ int inserted = 0;
+
+ inserted |= insert_oid_pair(map->to_compat, oid, compat_oid);
+ inserted |= insert_oid_pair(map->to_storage, compat_oid, oid);
+ if (inserted)
+ oidtree_insert(odb->loose_objects_cache, compat_oid);
+
+ return inserted;
+}
+
static int load_one_loose_object_map(struct repository *repo, struct object_directory *dir)
{
struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
@@ -49,15 +65,14 @@ static int load_one_loose_object_map(struct repository *repo, struct object_dire
if (!dir->loose_map)
loose_object_map_init(&dir->loose_map);
+ if (!dir->loose_objects_cache) {
+ ALLOC_ARRAY(dir->loose_objects_cache, 1);
+ oidtree_init(dir->loose_objects_cache);
+ }
- insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->empty_tree, repo->compat_hash_algo->empty_tree);
- insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->empty_tree, repo->hash_algo->empty_tree);
-
- insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);
- insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->empty_blob, repo->hash_algo->empty_blob);
-
- insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);
- insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->null_oid, repo->hash_algo->null_oid);
+ insert_loose_map(dir, repo->hash_algo->empty_tree, repo->compat_hash_algo->empty_tree);
+ insert_loose_map(dir, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);
+ insert_loose_map(dir, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);
strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
fp = fopen(path.buf, "rb");
@@ -77,8 +92,7 @@ static int load_one_loose_object_map(struct repository *repo, struct object_dire
parse_oid_hex_algop(p, &compat_oid, &p, repo->compat_hash_algo) ||
p != buf.buf + buf.len)
goto err;
- insert_oid_pair(dir->loose_map->to_compat, &oid, &compat_oid);
- insert_oid_pair(dir->loose_map->to_storage, &compat_oid, &oid);
+ insert_loose_map(dir, &oid, &compat_oid);
}
strbuf_release(&buf);
@@ -197,8 +211,7 @@ int repo_add_loose_object_map(struct repository *repo, const struct object_id *o
if (!should_use_loose_object_map(repo))
return 0;
- inserted |= insert_oid_pair(repo->objects->odb->loose_map->to_compat, oid, compat_oid);
- inserted |= insert_oid_pair(repo->objects->odb->loose_map->to_storage, compat_oid, oid);
+ inserted = insert_loose_map(repo->objects->odb, oid, compat_oid);
if (inserted)
return write_one_object(repo, oid, compat_oid);
return 0;
--
2.41.0
^ permalink raw reply related
* [PATCH v2 05/30] loose: add a mapping between SHA-1 and SHA-256 for loose objects
From: Eric W. Biederman @ 2023-10-02 2:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, brian m. carlson, Eric Sunshine, Eric W. Biederman
In-Reply-To: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
From: "brian m. carlson" <sandals@crustytoothpaste.net>
As part of the transition plan, we'd like to add a file in the .git
directory that maps loose objects between SHA-1 and SHA-256. Let's
implement the specification in the transition plan and store this data
on a per-repository basis in struct repository.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
Makefile | 1 +
loose.c | 246 ++++++++++++++++++++++++++++++++++++++++++
loose.h | 22 ++++
object-file-convert.c | 14 ++-
object-store-ll.h | 3 +
object.c | 2 +
repository.c | 6 ++
7 files changed, 293 insertions(+), 1 deletion(-)
create mode 100644 loose.c
create mode 100644 loose.h
diff --git a/Makefile b/Makefile
index f7e824f25cda..3c18664def9a 100644
--- a/Makefile
+++ b/Makefile
@@ -1053,6 +1053,7 @@ LIB_OBJS += list-objects-filter.o
LIB_OBJS += list-objects.o
LIB_OBJS += lockfile.o
LIB_OBJS += log-tree.o
+LIB_OBJS += loose.o
LIB_OBJS += ls-refs.o
LIB_OBJS += mailinfo.o
LIB_OBJS += mailmap.o
diff --git a/loose.c b/loose.c
new file mode 100644
index 000000000000..6ba73cc84dca
--- /dev/null
+++ b/loose.c
@@ -0,0 +1,246 @@
+#include "git-compat-util.h"
+#include "hash.h"
+#include "path.h"
+#include "object-store.h"
+#include "hex.h"
+#include "wrapper.h"
+#include "gettext.h"
+#include "loose.h"
+#include "lockfile.h"
+
+static const char *loose_object_header = "# loose-object-idx\n";
+
+static inline int should_use_loose_object_map(struct repository *repo)
+{
+ return repo->compat_hash_algo && repo->gitdir;
+}
+
+void loose_object_map_init(struct loose_object_map **map)
+{
+ struct loose_object_map *m;
+ m = xmalloc(sizeof(**map));
+ m->to_compat = kh_init_oid_map();
+ m->to_storage = kh_init_oid_map();
+ *map = m;
+}
+
+static int insert_oid_pair(kh_oid_map_t *map, const struct object_id *key, const struct object_id *value)
+{
+ khiter_t pos;
+ int ret;
+ struct object_id *stored;
+
+ pos = kh_put_oid_map(map, *key, &ret);
+
+ /* This item already exists in the map. */
+ if (ret == 0)
+ return 0;
+
+ stored = xmalloc(sizeof(*stored));
+ oidcpy(stored, value);
+ kh_value(map, pos) = stored;
+ return 1;
+}
+
+static int load_one_loose_object_map(struct repository *repo, struct object_directory *dir)
+{
+ struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
+ FILE *fp;
+
+ if (!dir->loose_map)
+ loose_object_map_init(&dir->loose_map);
+
+ insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->empty_tree, repo->compat_hash_algo->empty_tree);
+ insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->empty_tree, repo->hash_algo->empty_tree);
+
+ insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);
+ insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->empty_blob, repo->hash_algo->empty_blob);
+
+ insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);
+ insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->null_oid, repo->hash_algo->null_oid);
+
+ strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
+ fp = fopen(path.buf, "rb");
+ if (!fp) {
+ strbuf_release(&path);
+ return 0;
+ }
+
+ errno = 0;
+ if (strbuf_getwholeline(&buf, fp, '\n') || strcmp(buf.buf, loose_object_header))
+ goto err;
+ while (!strbuf_getline_lf(&buf, fp)) {
+ const char *p;
+ struct object_id oid, compat_oid;
+ if (parse_oid_hex_algop(buf.buf, &oid, &p, repo->hash_algo) ||
+ *p++ != ' ' ||
+ parse_oid_hex_algop(p, &compat_oid, &p, repo->compat_hash_algo) ||
+ p != buf.buf + buf.len)
+ goto err;
+ insert_oid_pair(dir->loose_map->to_compat, &oid, &compat_oid);
+ insert_oid_pair(dir->loose_map->to_storage, &compat_oid, &oid);
+ }
+
+ strbuf_release(&buf);
+ strbuf_release(&path);
+ return errno ? -1 : 0;
+err:
+ strbuf_release(&buf);
+ strbuf_release(&path);
+ return -1;
+}
+
+int repo_read_loose_object_map(struct repository *repo)
+{
+ struct object_directory *dir;
+
+ if (!should_use_loose_object_map(repo))
+ return 0;
+
+ prepare_alt_odb(repo);
+
+ for (dir = repo->objects->odb; dir; dir = dir->next) {
+ if (load_one_loose_object_map(repo, dir) < 0) {
+ return -1;
+ }
+ }
+ return 0;
+}
+
+int repo_write_loose_object_map(struct repository *repo)
+{
+ kh_oid_map_t *map = repo->objects->odb->loose_map->to_compat;
+ struct lock_file lock;
+ int fd;
+ khiter_t iter;
+ struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
+
+ if (!should_use_loose_object_map(repo))
+ return 0;
+
+ strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
+ fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1);
+ iter = kh_begin(map);
+ if (write_in_full(fd, loose_object_header, strlen(loose_object_header)) < 0)
+ goto errout;
+
+ for (; iter != kh_end(map); iter++) {
+ if (kh_exist(map, iter)) {
+ if (oideq(&kh_key(map, iter), the_hash_algo->empty_tree) ||
+ oideq(&kh_key(map, iter), the_hash_algo->empty_blob))
+ continue;
+ strbuf_addf(&buf, "%s %s\n", oid_to_hex(&kh_key(map, iter)), oid_to_hex(kh_value(map, iter)));
+ if (write_in_full(fd, buf.buf, buf.len) < 0)
+ goto errout;
+ strbuf_reset(&buf);
+ }
+ }
+ strbuf_release(&buf);
+ if (commit_lock_file(&lock) < 0) {
+ error_errno(_("could not write loose object index %s"), path.buf);
+ strbuf_release(&path);
+ return -1;
+ }
+ strbuf_release(&path);
+ return 0;
+errout:
+ rollback_lock_file(&lock);
+ strbuf_release(&buf);
+ error_errno(_("failed to write loose object index %s\n"), path.buf);
+ strbuf_release(&path);
+ return -1;
+}
+
+static int write_one_object(struct repository *repo, const struct object_id *oid,
+ const struct object_id *compat_oid)
+{
+ struct lock_file lock;
+ int fd;
+ struct stat st;
+ struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
+
+ strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
+ hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1);
+
+ fd = open(path.buf, O_WRONLY | O_CREAT | O_APPEND, 0666);
+ if (fd < 0)
+ goto errout;
+ if (fstat(fd, &st) < 0)
+ goto errout;
+ if (!st.st_size && write_in_full(fd, loose_object_header, strlen(loose_object_header)) < 0)
+ goto errout;
+
+ strbuf_addf(&buf, "%s %s\n", oid_to_hex(oid), oid_to_hex(compat_oid));
+ if (write_in_full(fd, buf.buf, buf.len) < 0)
+ goto errout;
+ if (close(fd))
+ goto errout;
+ adjust_shared_perm(path.buf);
+ rollback_lock_file(&lock);
+ strbuf_release(&buf);
+ strbuf_release(&path);
+ return 0;
+errout:
+ error_errno(_("failed to write loose object index %s\n"), path.buf);
+ close(fd);
+ rollback_lock_file(&lock);
+ strbuf_release(&buf);
+ strbuf_release(&path);
+ return -1;
+}
+
+int repo_add_loose_object_map(struct repository *repo, const struct object_id *oid,
+ const struct object_id *compat_oid)
+{
+ int inserted = 0;
+
+ if (!should_use_loose_object_map(repo))
+ return 0;
+
+ inserted |= insert_oid_pair(repo->objects->odb->loose_map->to_compat, oid, compat_oid);
+ inserted |= insert_oid_pair(repo->objects->odb->loose_map->to_storage, compat_oid, oid);
+ if (inserted)
+ return write_one_object(repo, oid, compat_oid);
+ return 0;
+}
+
+int repo_loose_object_map_oid(struct repository *repo,
+ const struct object_id *src,
+ const struct git_hash_algo *to,
+ struct object_id *dest)
+{
+ struct object_directory *dir;
+ kh_oid_map_t *map;
+ khiter_t pos;
+
+ for (dir = repo->objects->odb; dir; dir = dir->next) {
+ struct loose_object_map *loose_map = dir->loose_map;
+ if (!loose_map)
+ continue;
+ map = (to == repo->compat_hash_algo) ?
+ loose_map->to_compat :
+ loose_map->to_storage;
+ pos = kh_get_oid_map(map, *src);
+ if (pos < kh_end(map)) {
+ oidcpy(dest, kh_value(map, pos));
+ return 0;
+ }
+ }
+ return -1;
+}
+
+void loose_object_map_clear(struct loose_object_map **map)
+{
+ struct loose_object_map *m = *map;
+ struct object_id *oid;
+
+ if (!m)
+ return;
+
+ kh_foreach_value(m->to_compat, oid, free(oid));
+ kh_foreach_value(m->to_storage, oid, free(oid));
+ kh_destroy_oid_map(m->to_compat);
+ kh_destroy_oid_map(m->to_storage);
+ free(m);
+ *map = NULL;
+}
diff --git a/loose.h b/loose.h
new file mode 100644
index 000000000000..2c2957072c5f
--- /dev/null
+++ b/loose.h
@@ -0,0 +1,22 @@
+#ifndef LOOSE_H
+#define LOOSE_H
+
+#include "khash.h"
+
+struct loose_object_map {
+ kh_oid_map_t *to_compat;
+ kh_oid_map_t *to_storage;
+};
+
+void loose_object_map_init(struct loose_object_map **map);
+void loose_object_map_clear(struct loose_object_map **map);
+int repo_loose_object_map_oid(struct repository *repo,
+ const struct object_id *src,
+ const struct git_hash_algo *dest_algo,
+ struct object_id *dest);
+int repo_add_loose_object_map(struct repository *repo, const struct object_id *oid,
+ const struct object_id *compat_oid);
+int repo_read_loose_object_map(struct repository *repo);
+int repo_write_loose_object_map(struct repository *repo);
+
+#endif
diff --git a/object-file-convert.c b/object-file-convert.c
index 4777aba83636..1ec945eaa17f 100644
--- a/object-file-convert.c
+++ b/object-file-convert.c
@@ -4,6 +4,7 @@
#include "repository.h"
#include "hash-ll.h"
#include "object.h"
+#include "loose.h"
#include "object-file-convert.h"
int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
@@ -21,7 +22,18 @@ int repo_oid_to_algop(struct repository *repo, const struct object_id *src,
oidcpy(dest, src);
return 0;
}
- return -1;
+ if (repo_loose_object_map_oid(repo, src, to, dest)) {
+ /*
+ * We may have loaded the object map at repo initialization but
+ * another process (perhaps upstream of a pipe from us) may have
+ * written a new object into the map. If the object is missing,
+ * let's reload the map to see if the object has appeared.
+ */
+ repo_read_loose_object_map(repo);
+ if (repo_loose_object_map_oid(repo, src, to, dest))
+ return -1;
+ }
+ return 0;
}
int convert_object_file(struct strbuf *outbuf,
diff --git a/object-store-ll.h b/object-store-ll.h
index 26a3895c821c..bc76d6bec80d 100644
--- a/object-store-ll.h
+++ b/object-store-ll.h
@@ -26,6 +26,9 @@ struct object_directory {
uint32_t loose_objects_subdir_seen[8]; /* 256 bits */
struct oidtree *loose_objects_cache;
+ /* Map between object IDs for loose objects. */
+ struct loose_object_map *loose_map;
+
/*
* This is a temporary object store created by the tmp_objdir
* facility. Disable ref updates since the objects in the store
diff --git a/object.c b/object.c
index 2c61e4c86217..186a0a47c0fb 100644
--- a/object.c
+++ b/object.c
@@ -13,6 +13,7 @@
#include "alloc.h"
#include "packfile.h"
#include "commit-graph.h"
+#include "loose.h"
unsigned int get_max_object_index(void)
{
@@ -540,6 +541,7 @@ void free_object_directory(struct object_directory *odb)
{
free(odb->path);
odb_clear_loose_cache(odb);
+ loose_object_map_clear(&odb->loose_map);
free(odb);
}
diff --git a/repository.c b/repository.c
index 80252b79e93e..6214f61cf4e7 100644
--- a/repository.c
+++ b/repository.c
@@ -14,6 +14,7 @@
#include "read-cache-ll.h"
#include "remote.h"
#include "setup.h"
+#include "loose.h"
#include "submodule-config.h"
#include "sparse-index.h"
#include "trace2.h"
@@ -109,6 +110,8 @@ void repo_set_compat_hash_algo(struct repository *repo, int algo)
if (hash_algo_by_ptr(repo->hash_algo) == algo)
BUG("hash_algo and compat_hash_algo match");
repo->compat_hash_algo = algo ? &hash_algos[algo] : NULL;
+ if (repo->compat_hash_algo)
+ repo_read_loose_object_map(repo);
}
/*
@@ -201,6 +204,9 @@ int repo_init(struct repository *repo,
if (worktree)
repo_set_worktree(repo, worktree);
+ if (repo->compat_hash_algo)
+ repo_read_loose_object_map(repo);
+
clear_repository_format(&format);
return 0;
--
2.41.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox