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@xxxxxxxxx> 1617083739 -0700 > committer Elijah Newren <newren@xxxxxxxxx> 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