git-filter-repo icon indicating copy to clipboard operation
git-filter-repo copied to clipboard

Changing order of commit msg update

Open HassanAkbar opened this issue 3 years ago • 4 comments

Currently the order of execution is:

  1. Update the SHA references in the messages
  2. Execute the message-callback to update the message
  3. Added an option --order-by-date to order commits by date rather than the default topological sort.

While adding SHA references in a commit message using message-callback, the references were not getting updated and were pointing to old commit SHA. So I changed the order and now it first executes the message-callback and then updates the reference.

This is a contribution from Ribose (riboseinc).

HassanAkbar avatar Dec 02 '21 15:12 HassanAkbar

To provide some background information: this PR stems from the need of updating SVN revision references to their corresponding Git commits (https://github.com/ietf-ribose/github-migration-project/issues/17#issuecomment-984711460)

We are trying to migrate SVN repositories to Git, and wish to migrate the SVN mentions in commit messages to their corresponding Git commit hashes.

In order to do so, we have to maintain a stable ancestor Git history (i.e. all ancestor commits of the commit to be modified should not change) when updating every commit message, so that we can apply the mention update to all descendant commits. (ref: https://github.com/ietf-ribose/github-migration-project/issues/17#issuecomment-984216598)

Thank you @newren for this fantastic work!

ronaldtse avatar Dec 03 '21 03:12 ronaldtse

Hi,

I'm so sorry for being absent for so long.

The changes are very interesting! Thanks for submitting this. A couple small notes:

Commit messages should be wrapped at 72 characters, and the Signed-off-by should always be on a line of its own, and should include your email address.

You have two logically independent changes here, so they really should be split out into two commits.

The changes around ordering of message callback and hash updates in commit messages is a very good catch; well spotted!

I think I'd prefer to use --date-order for the option name since that's already ingrained in git and what it translates to; one less thing to remember. That option probably needs a more thorough explanation in the commit message as well. I think it actually would have broken some exports at the time you submitted it. In fact, the first bug I ever fixed in git involved forcing fast-export to use topological ordering instead of date ordering to avoid a bug (see https://github.com/git/git/commit/784f8affe4dfc8ceec93803d6c22b4b8467a4642). I think the real underlying issue was fixed last December (see https://github.com/git/git/commit/726a228dfb19fae8befa0f209436f8fae7919a58). So perhaps the commit message for the --date-order commit would be something like:

filter-repo: add a --date-order option

This adds a --date-order option to filter-repo, which passes it along to
fast-export.  Once upon a time, this option might have caused problems
for git-fast-export, as per git.git commit 784f8affe4 ("fast-export:
ensure we traverse commits in topological order", 2009-02-10), but it
should be safe with Git >= 2.35 due to git.git commit 726a228dfb
("fast-export: fix surprising behavior with --first-parent",
2021-12-16).

Followed by a blank line and your Signed-off-by.

Anyway, again I'm sorry for being absent for so long. If you can restructure them slightly, I'd be happy to merge them. Also, since you do have at least a basic Signed-off-by, and since I was gone so long, let me know if you'd like me to restructure the commits.

newren avatar Sep 24 '22 22:09 newren

Hi @newren ,

I'm sorry I somehow missed your message. I've update the commits according to your suggestions.

HassanAkbar avatar Oct 13 '23 11:10 HassanAkbar

@newren If you need any support here, I will be able to help.

HassanAkbar avatar Apr 28 '24 07:04 HassanAkbar

Tweaked the commits to fix them up and merged as 4f30eee3719d (Merge branch 'ha/commit-message-order', 2024-07-02).

Sorry for the long delays; thanks for your patience and for your contribution!

newren avatar Jul 03 '24 03:07 newren