Re: Bug report: git branch behaves as if --no-replace-objects is passed
On Mon, Mar 29, 2021 at 11:05:05PM -0700, Elijah Newren wrote:
> it all works well. BUT, if I try to use it with branch it doesn't work:
>
> $ git branch --contains deadbeefdeadbeefdeadbeefdeadbeefdeadbeef
> $
I'm not surprised here. The replace mechanism is usually "if you are trying to access object X, then access the contents of Y instead". But I don't think we generally rewrite references from other objects. So in that sense, no ref "contains" deadbeef, because nobody points to it as an ancestor.
I guess "branch --contains" could convert the mention of the replaced object on the command-line, something like this:
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 4542d4d3f9..200be4e3d2 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -6,6 +6,7 @@
#include "string-list.h"
#include "strvec.h"
#include "oid-array.h"
+#include "replace-object.h"
/*----- some often used options -----*/
@@ -84,6 +85,7 @@ int parse_opt_verbosity_cb(const struct option *opt, const char *arg,
int parse_opt_commits(const struct option *opt, const char *arg, int unset)
{
struct object_id oid;
+ const struct object_id *replace;
struct commit *commit;
BUG_ON_OPT_NEG(unset);
@@ -92,7 +94,9 @@ int parse_opt_commits(const struct option *opt, const char *arg, int unset)
return -1;
if (get_oid(arg, &oid))
return error("malformed object name %s", arg);
- commit = lookup_commit_reference(the_repository, &oid);
+
+ replace = lookup_replace_object(the_repository, &oid);
+ commit = lookup_commit_reference(the_repository, replace);
if (!commit)
return error("no such commit %s", arg);
commit_list_insert(commit, opt->value);
though if we go that route, I suspect we ought to be adding both the
original _and_ the replacement.
I'm not entirely sure this is a good direction, though.
> and possibly worse, if I create a new branch based on it and use it:
>
> $ git branch foobar deadbeefdeadbeefdeadbeefdeadbeefdeadbeef
> $ git checkout foobar
> $ echo stuff >empty
> $ git add empty
> $ git commit -m more
>
> then it's clear that branch created foobar pointing to the replaced
> object rather than the replacement object -- despite the fact that the
> replaced object doesn't even exist within this repo:
>
> $ git cat-file -p HEAD
> tree 18108bae26dc91af2055bc66cc9fea278012dbd3
> parent deadbeefdeadbeefdeadbeefdeadbeefdeadbeef
> author Elijah Newren <newren@gmail.com> 1617083739 -0700
> committer Elijah Newren <newren@gmail.com> 1617083739 -0700
>
> more
Yeah, that's pretty horrible. I do think you're using replace objects in
a way they weren't really intended for, in two ways:
- usually you'd actually have deadbeef, and you just want to rewrite
it to something else
- you wouldn't usually work directly with the replace object on the
command line like this. Usually the intent is to patch some old
section of history to get a different view.
None of which is an excuse exactly. But just to say that I'm not too
surprised that the "replace" mechanism is a bit half-baked, and there
are probably a lot of weird implications nobody has thought through.
Patches welcome, of course, though I suspect it may be a rabbit hole
that isn't worth your time. :)
> I poked around in the code a little but it is not at all clear to me
> why some parts of the code (log, diff) translate replace refs
> correctly, while others (branch) don't. It is clear from the output
> that log is aware that the refs are replaced, which makes me wonder if
> every caller needs to be aware of replace refs for them to work
> correctly everywhere, because I couldn't find a missing environment
> setup for "branch".
Right. To work as you expect above, basically everything that is going
to look at an oid would need to consider whether to use a replace ref.
Shoving the lookup into get_oid() would "fix" that, but I suspect would
confuse some other callers. Right now the rule is "when you access the
object contents, pretend as if it had X instead of Y", so we can enforce
that at the layer of accessing the object database.
-Peff
Elijah Newren
