All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anatoly Borodin <anatoly.borodin@gmail.com>
To: git@vger.kernel.org
Subject: Re: Bugs in git filter-branch (git replace related)
Date: Fri, 29 Jan 2016 18:24:07 +0000 (UTC)	[thread overview]
Message-ID: <n8gao5$3c6$1@ger.gmane.org> (raw)
In-Reply-To: 20160129061820.GB23106@sigill.intra.peff.net

Hi Jeff,


I've created a gist with the script
https://gist.github.com/anatolyborodin/6505a364a68584f13846

I've added some changes and a second test (will be discussed in the
comments).


Jeff King <peff@peff.net> wrote:
> I'm not sure if this is a bug or not. The "empty commit" check works by
> checking the tree sha1s, without doing a full diff respecting replace
> refs.
> 
> You're expecting git to notice a tree change, even though it never even
> examined the tree in the first place (because you didn't give it a tree
> or index filter).

When git-filter-branch(1) says "If you have any grafts or replacement
refs defined, running this command will make them permanent.", and it
doesn't work like that because of some optimization, it *is* a bug.

> Try:
> 
>   git filter-branch --prune-empty --tree-filter true master
> 
> which will force git to go through the motions of checking out the
> replaced content and re-examining it.

Thank you, I've added this command to the script, and it works! I think
it should be added to git-filter-branch(1), if there is no other way to
rewrite the optimization.

>> Bug 2: the replace refs are not ignored (they can epresent blobs, trees etc,
>> but even if they represent commits - should they be rewritten?).
> 
> You told it "--all", which is passed to rev-list, where it means "all
> refs". I agree that running filter-branch on refs/replace is probably
> not going to yield useful results, but I'm not sure if it is
> filter-branch's responsibility to second-guess the rev-list options.

Look how `git log --all` works (see the second test in the script): it
ignores (without any messages) the blobs, and writes only the commits.
For example, the same commit log message is printed twice in the second
testcase.

Maybe it makes sence, I don't know. I would suggest that all
refs/replace/* heads should be ignored by `git log`. `git rev-list
--no-replace` maybe?

> Probably the documentation for filter-branch should recommend
> "--branches --tags" instead of "--all", though.

Or redefine `--all` as "all refs excepting refs/replace/*". Who would
really want to run `--all` the way it works now?

The blobs replaces should be ignored, as in `git log --all`. Is there
any reason to rewrite refs/rebase/hash if it's a replace commit?


-- 
Mit freundlichen Grüßen,
Anatoly Borodin

  reply	other threads:[~2016-01-29 18:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-28 14:46 Bugs in git filter-branch (git replace related) Anatoly Borodin
2016-01-29  6:18 ` Jeff King
2016-01-29 18:24   ` Anatoly Borodin [this message]
2016-01-29 23:11     ` Jeff King
2016-02-08 23:55       ` A different bug in git-filter-branch (v2.7.0) Anatoly Borodin
2016-02-22 21:13         ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='n8gao5$3c6$1@ger.gmane.org' \
    --to=anatoly.borodin@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.