Git development
 help / color / mirror / Atom feed
* [PATCH v2] RelNotes: tweak 2.43.0 release notes
From: Andy Koppe @ 2023-11-15 19:07 UTC (permalink / raw)
  To: git; +Cc: Andy Koppe
In-Reply-To: <20231114223127.11292-1-andy.koppe@gmail.com>

Add some more detail on the $(decorate) log format placeholder and tweak
the wording on some other points.

Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
---
Now with 100% more signature. Sorry for the unnecessary noise.

 Documentation/RelNotes/2.43.0.txt | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/Documentation/RelNotes/2.43.0.txt b/Documentation/RelNotes/2.43.0.txt
index 770543c464e..9a68074a4a5 100644
--- a/Documentation/RelNotes/2.43.0.txt
+++ b/Documentation/RelNotes/2.43.0.txt
@@ -10,8 +10,8 @@ Backward Compatibility Notes
    prefix.  If you are negatively affected by this change, please use
    "--subject-prefix=PATCH --rfc" as a replacement.
 
- * "git rev-list --stdin" learned to take non-revisions (like "--not")
-   recently from the standard input, but the way such a "--not" was
+ * "git rev-list --stdin" recently learned to take non-revisions (like
+   "--not") from the standard input, but the way such a "--not" was
    handled was quite confusing, which has been rethought.  The updated
    rule is that "--not" given from the command line only affects revs
    given from the command line that comes but not revs read from the
@@ -43,10 +43,10 @@ UI, Workflows & Features
 
  * Git GUI updates.
 
- * "git format-patch" learns a way to feed cover letter description,
-   that (1) can be used on detached HEAD where there is no branch
-   description available, and (2) also can override the branch
-   description if there is one.
+ * "git format-patch" learns option "--description-file" to feed in a
+   cover letter description that can be used when no branch description
+   is available, or that can override the branch description if there is
+   one.
 
  * Use of --max-pack-size to allow multiple packfiles to be created is
    now supported even when we are sending unreachable objects to cruft
@@ -56,7 +56,9 @@ UI, Workflows & Features
    "--subject-prefix" option and used "[RFC PATCH]"; now we will add
    "RFC" prefix to whatever subject prefix is specified.
 
- * "git log --format" has been taught the %(decorate) placeholder.
+ * "git log" format strings now support a "%(decorate)" placeholder that
+   can be used to customize the symbols and the tag prefix used in ref
+   decorations.
 
  * The default log message created by "git revert", when reverting a
    commit that records a revert, has been tweaked, to encourage people
@@ -99,7 +101,7 @@ UI, Workflows & Features
  * The attribute subsystem learned to honor `attr.tree` configuration
    that specifies which tree to read the .gitattributes files from.
 
- * "git merge-file" learns a mode to read three contents to be merged
+ * "git merge-file" learns a mode to read three files to be merged
    from blob objects.
 
 
@@ -127,7 +129,7 @@ Performance, Internal Implementation, Development Support etc.
  * The code to keep track of existing packs in the repository while
    repacking has been refactored.
 
- * The "streaming" interface used for bulk-checkin codepath has been
+ * The "streaming" interface used for the bulk-checkin codepath has been
    narrowed to take only blob objects for now, with no real loss of
    functionality.
 
-- 
2.43.0-rc1


^ permalink raw reply related

* Bug: "Received" misspelled in remote message
From: Alan Dove @ 2023-11-15 18:46 UTC (permalink / raw)
  To: git

Hello:

Using Git 2.40.1 on a private server, I see this message whenever I
push a commit:

"remote: Recieved update on checked-out branch, queueing deployment."

"Received" is misspelled. 

I realize this is a very minor issue, but it should also be very easy
to correct. As someone who suffers from Proofreader's Eye, this error
deals psychic damage to me on a daily basis.

Thanks.

-- 
 --Alan

Alan Dove, Ph.D.
alandove.com
917.273.0544



^ permalink raw reply

* Re: [PATCH] docs: submitting-patches: improve the base commit explanation
From: Borislav Petkov @ 2023-11-15 18:07 UTC (permalink / raw)
  To: Kees Cook; +Cc: Jonathan Corbet, workflows, linux-doc, LKML, git
In-Reply-To: <202311150948.F6E39AD@keescook>

On Wed, Nov 15, 2023 at 09:49:31AM -0800, Kees Cook wrote:
> On Wed, Nov 15, 2023 at 06:03:30PM +0100, Borislav Petkov wrote:
> > From: "Borislav Petkov (AMD)" <bp@alien8.de>
> > 
> > After receiving a second patchset this week without knowing which tree
> > it applies on and trying to apply it on the obvious ones and failing,
> > make sure the base tree information which needs to be supplied in the
> > 0th message of the patchset is spelled out more explicitly.
> > 
> > Also, make the formulations stronger as this really is a requirement and
> > not only a useful thing anymore.
> > 
> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> 
> Yup, I wonder if making "--base=auto" a default in git might be a good
> idea too?

Not a bad idea. And if not needed, one can simply ignore it when reading
the cover letter.

CCing the git ML for comment/opinions.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply

* [RFC PATCH v2 2/2] send-email: remove stray characters from usage
From: Todd Zullinger @ 2023-11-15 17:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
	Ondřej Pohořelský
In-Reply-To: <20231115173952.339303-1-tmz@pobox.com>

A few stray single quotes crept into the usage string in a2ce608244
(send-email docs: add format-patch options, 2021-10-25).  Remove them.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
This is not scrictly tied to the previous commit.  It just stood out
while I was reviewing the usage output.

 git-send-email.perl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 94046e0fb7..cd2f0ae14e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -28,8 +28,8 @@
 
 sub usage {
 	print <<EOT;
-git send-email' [<options>] <file|directory>
-git send-email' [<options>] <format-patch options>
+git send-email [<options>] <file|directory>
+git send-email [<options>] <format-patch options>
 git send-email --dump-aliases
 
   Composing:
-- 
2.43.0.rc2


^ permalink raw reply related

* [RFC PATCH v2 1/2] send-email: avoid duplicate specification warnings
From: Todd Zullinger @ 2023-11-15 17:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
	Ondřej Pohořelský
In-Reply-To: <20231115173952.339303-1-tmz@pobox.com>

With perl-Getopt-Long >= 2.55 a warning is issued for options which are
specified more than once.  In addition to causing users to see warnings,
this results in test failures which compare the output.  An example,
from t9001-send-email.37:

  | +++ diff -u expect actual
  | --- expect      2023-11-14 10:38:23.854346488 +0000
  | +++ actual      2023-11-14 10:38:23.848346466 +0000
  | @@ -1,2 +1,7 @@
  | +Duplicate specification "no-chain-reply-to" for option "no-chain-reply-to"
  | +Duplicate specification "to-cover|to-cover!" for option "to-cover"
  | +Duplicate specification "cc-cover|cc-cover!" for option "cc-cover"
  | +Duplicate specification "no-thread" for option "no-thread"
  | +Duplicate specification "no-to-cover" for option "no-to-cover"
  |  fatal: longline.patch:35 is longer than 998 characters
  |  warning: no patches were sent
  | error: last command exited with $?=1
  | not ok 37 - reject long lines

Remove the duplicate option specs.

Teach `--git-completion-helper` to output the '--no-' options.  They are
not included in the options hash and would otherwise be lost.

Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
I compared the output from

    git send-email --git-completion-helper | tr ' ' ' '\n' | sort

before and after the change to ensure no options were lost (or added).

I also confirmed that each of the options changed did not result in any
error.  Unrecognized options result in an error from `git format-patch`,
e.g.:

    $ git send-email --foo
    fatal: unrecognized argument: --foo
    format-patch -o /tmp/PaqtbH3jCw --foo: command returned error: 128

A little history:

  Support for the '--no-' prefix was added in Getopt::Long >= 2.33, in
  commit 8ca8b48 (Negatable options (with "!") now also support the
  "no-" prefix., 2003-04-04).  Getopt::Long 2.34 was included in
  perl-5.8.1 (2003-09-25), per Module::CoreList[1].
  
  We list perl-5.8 as the minimum version in INSTALL.  This would leave
  users with perl-5.8.0 (2002-07-18) with non-working arguments for
  options where we're removing the explicit 'no-' variant.
  
  The explicit 'no-' opts were added in f471494303 (git-send-email.perl:
  support no- prefix with older GetOptions, 2015-01-30), specifically to
  support perl-5.8.0 which includes the older Getopt::Long.
    
It may be time to bump the Perl requirement to 5.8.1 (2003-09-25) or
even 5.10.0 (2007-12-18).  We last bumped the requirement from 5.6 to
5.8 in d48b284183 (perl: bump the required Perl version to 5.8 from
5.6.[21], 2010-09-24).

Another option to avoid the warning from Getopt::Long >= 2.55 would be
to remove the '!' negation, but that would drop support for the 'no'
prefix variants (e.g.: `--nocc-cover`).  While these are not documented
(and I don't think they ever were[2]), they have worked for a long, long
time.  Odds are good that some scripts rely on them and we don't want
anyone yelling at Junio.

I lean toward dropping support for the 21-year-old 5.8.0.

If there is a way to have our cake without any consequence, I'm happy to
hear it.  If not, I'll add a commit which bumps the requirement in
general or notes that some git-send-email requires perl >= 5.8.1 and
adjusts the 'use' line there to `use 5.008001;`.

[1] http://perlpunks.de/corelist/mversion?module=Getopt%3A%3ALong

[2] The 'no-' opts were added in f471494303 (git-send-email.perl:
    support no- prefix with older GetOptions, 2015-01-30).  The commit
    message says "the help only mentions the 'no-' prefix and not the
    'no' prefix, add explicit support for the 'no-' prefix to support
    older GetOptions versions."

 git-send-email.perl | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index cacdbd6bb2..94046e0fb7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -119,13 +119,16 @@ sub completion_helper {
 
 	foreach my $key (keys %$original_opts) {
 		unless (exists $not_for_completion{$key}) {
-			$key =~ s/!$//;
+			my $negate = ($key =~ s/!$//);
 
 			if ($key =~ /[:=][si]$/) {
 				$key =~ s/[:=][si]$//;
 				push (@send_email_opts, "--$_=") foreach (split (/\|/, $key));
 			} else {
 				push (@send_email_opts, "--$_") foreach (split (/\|/, $key));
+				if ($negate) {
+					push (@send_email_opts, "--no-$_") foreach (split (/\|/, $key));
+				}
 			}
 		}
 	}
@@ -491,7 +494,6 @@ sub config_regexp {
 		    "bcc=s" => \@getopt_bcc,
 		    "no-bcc" => \$no_bcc,
 		    "chain-reply-to!" => \$chain_reply_to,
-		    "no-chain-reply-to" => sub {$chain_reply_to = 0},
 		    "sendmail-cmd=s" => \$sendmail_cmd,
 		    "smtp-server=s" => \$smtp_server,
 		    "smtp-server-option=s" => \@smtp_server_options,
@@ -506,36 +508,27 @@ sub config_regexp {
 		    "smtp-auth=s" => \$smtp_auth,
 		    "no-smtp-auth" => sub {$smtp_auth = 'none'},
 		    "annotate!" => \$annotate,
-		    "no-annotate" => sub {$annotate = 0},
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
 		    "header-cmd=s" => \$header_cmd,
 		    "no-header-cmd" => \$no_header_cmd,
 		    "suppress-from!" => \$suppress_from,
-		    "no-suppress-from" => sub {$suppress_from = 0},
 		    "suppress-cc=s" => \@suppress_cc,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
-		    "no-signed-off-cc|no-signed-off-by-cc" => sub {$signed_off_by_cc = 0},
-		    "cc-cover|cc-cover!" => \$cover_cc,
-		    "no-cc-cover" => sub {$cover_cc = 0},
-		    "to-cover|to-cover!" => \$cover_to,
-		    "no-to-cover" => sub {$cover_to = 0},
+		    "cc-cover!" => \$cover_cc,
+		    "to-cover!" => \$cover_to,
 		    "confirm=s" => \$confirm,
 		    "dry-run" => \$dry_run,
 		    "envelope-sender=s" => \$envelope_sender,
 		    "thread!" => \$thread,
-		    "no-thread" => sub {$thread = 0},
 		    "validate!" => \$validate,
-		    "no-validate" => sub {$validate = 0},
 		    "transfer-encoding=s" => \$target_xfer_encoding,
 		    "format-patch!" => \$format_patch,
-		    "no-format-patch" => sub {$format_patch = 0},
 		    "8bit-encoding=s" => \$auto_8bit_encoding,
 		    "compose-encoding=s" => \$compose_encoding,
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
-		    "no-xmailer" => sub {$use_xmailer = 0},
 		    "batch-size=i" => \$batch_size,
 		    "relogin-delay=i" => \$relogin_delay,
 		    "git-completion-helper" => \$git_completion_helper,
-- 
2.43.0.rc2


^ permalink raw reply related

* [RFC PATCH v2 0/2] send-email: avoid duplicate specification warnings
From: Todd Zullinger @ 2023-11-15 17:39 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
	Ondřej Pohořelský
In-Reply-To: <xmqqy1ezx2mq.fsf@gitster.g>

Changes since v1:

    * Teach `--git-completion-helper` to output the '--no-' options.
      They are not included in the options hash and would otherwise be
      lost.

    * Restore the `--signed-off-cc` alias which was mistakenly removed.

Todd Zullinger (2):
  send-email: avoid duplicate specification warnings
  send-email: remove stray characters from usage

 git-send-email.perl | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

-- 
2.43.0.rc2


^ permalink raw reply

* Re: Git Rename Detection Bug
From: Philip Oakley @ 2023-11-15 16:51 UTC (permalink / raw)
  To: Elijah Newren, Jeremy Pridmore; +Cc: git@vger.kernel.org, Paul Baumgartner
In-Reply-To: <CABPp-BHEX+SyophEfgRqDbNdrAS3=bptt_cKzHLBSutnBAxexw@mail.gmail.com>

Hi Elijah,

On 11/11/2023 05:46, Elijah Newren wrote:
> * filename similarity is extraordinarily expensive compared to exact
> renames, and if not carefully handled, can sometimes rival the cost of
> file content similarity computations given our spanhash
> representations.

I've not heard of spanhash representation before. Any references or
further reading?

>    Exact renames are tasked with finding renames even
> if they are known to not be relevant, simply because exact renames can
> do so very quickly.  If we change that, we throw a monkey wrench in
> our performance handling elsewhere and have to rethink a number of
> other things.

--
Philip

^ permalink raw reply

* gitmodulesSymlink .gitmodules is a symbolic link
From: flobee.code @ 2023-11-15 16:01 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 185 bytes --]

Hi

I can not rebase symlinks of an 6 years old repository.
See attached files (two languages, EN auto translation) for more details.
Created with `git bugreport`

Kind regards
Florian

[-- Attachment #2: git-bugreport-2023-11-15-1605-EN.txt --]
[-- Type: text/plain, Size: 7152 bytes --]

[EN](#EN) Translated by https://www.deepl.com/translator



# EN

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your problem.
understand your problem.


What did you do before the error occurred? (Steps to reproduce your error
reproduce your error)


For 6 years I only did remote pushes locally to my local master.
github.com then refused to accept one.


The reason: 
And now github.com but also `git` itself are bitching in detail.


github.com, as it no longer accepts a push:
    
    index-pack failed
    remote: error: object [hash]: gitmodulesSymlink: .gitmodules is a symbolic link


And `git` itself also aborts. So I can't solve the problems this way.


    git filter-branch --tree-filter 'rm -f .gitmodules' HEAD
    Rewrite [SomeHash] (3/185) (0 seconds passed, remaining 0 predicted) \
        error: Invalid path '.gitmodules' Could not initialize the index


The same attempt also failed with an older VM where `git`
version 2.11.0 was still available.





What did you expect to happen? (Expected behavior)

Rebase should work. Always :-)



What happened instead? (Actual behavior)

Abort of git.

    git filter-branch --tree-filter 'rm -f .gitmodules' HEAD
    Rewrite [SomeHash] (3/185) (0 seconds passed, remaining 0 predicted) \
        error: Invalid path '.gitmodules' Could not initialize the index



What is the difference between what you expected and what actually happened?
really happened?

With a current git version I can't fix such errors.

But in general I think the exclusion of symlinks to git system files is a mistake. It is implemented too sweepingly in my eyes.


Any other comments you would like to add:


My `.gitmodules` were always a matching file in the same directory to
to become aware that something is changing during a merge.
Branches are staging areas for me, among other things) 
```
    .gitmodules -> symlinks to one of these files 
                    (depending on the current branch)

        -> .gitmodules_stable
                ^ ------------------ Staging to stable, release
        -> .gitmodules_testing
                ^ ------------------ Staging to testing, testing and preview
        -> .gitmodules_unstable
                ^ ^ ^---<---------<-- Feature branch A
                +-----<---------<-- Feature branch B
                +-------<---------<-- Feature branch C ...
```

With a merge unstable -> testing (-ff), settings from unstable would simply be
would simply be included/adopted in testing without drawing attention to it.
attention. This also has the consequence that you are no longer made aware
that the dependencies of the submodules must also be raised,
before the merge is clean/finished.

If you are working on several submodules, this makes perfect sense. Especially
if you don't have or want to have everything online in order to check/test the current
to check/test the current status etc. before you then bind the hashes of the submodules and
then make everything public piece by piece depending on the dependency.
What an effort :-/

With `git` itself (version 2.39.0, Debian Bookworm, 2023) you can still work locally.
continue to work. With github.com, however, the fun is over. If it is in the
repository `git` system files like `.gitignore` or `.gitmodules` that are symlinks
are symlinks, github.com refuses to do any work to fix the bugs.
the bugs. Neither via the web interface nor via `git push --force` itself. Whew.

So: After 6 years! In between there were local updates from time to time, but these were
were not made public. The operating system, a Debian, is updated about every
updated every two years. So there have been at least 3 new `git` versions since then for
for this project. With the current version of `git` I cannot fix the problems
fix the problems. And that is really bad!

## Solution

Option 3. was the solution. A Debian Wheezy (7.3) brought a `git` 
version 1.7.10.4, which I used to rebase one of the known and problematic commits. 
known and problematic commits. I may have traveled too far into the
past, but this gave me a basis for testing.
Maybe it will stay that way and everything will be fine again?

Following this, and in order to do everything automatically again, the
git filter-branch came to mind again:

    git filter-branch --tree-filter 'rm -f .gitmodules' HEAD

Did its job, but didn't help anything Direction github.com as `push --force` for
for this branch.

    - git filter-branch --tree-filter 'rm -f .gitmodules' HEAD
    + git filter-branch --tree-filter 'rm -f .gitmodules'

So: complete and over everything please.

But to make this situation tidy:

    .gitmodules ->- symlink to one of these... ->-
    -> .gitmodules_master
    -> .gitmodules_stable
    -> .gitmodules_testing
    -> .gitmodules_unstable


'rm -f .gitmodules' deletes, but does not delete the underlying bindings of the 
submodules. Here, too, there were mixtures in the use/application, as the
history clearly showed on closer inspection. But the roadmap for doing it this way 
remained consistent.

I had to fetch the local repository from the backup quite often in order to be able to repeat the many test steps.
to be able to repeat the many steps of tests...

Finally: Submodules (`.gitmodules`) were always a symlink in the idea.
So in order for it to fit for the repair in the future, all symlinks must be resolved into a
real file, so that the rebase via `git filter-branch
--tree-filter` can run cleanly and the dependencies in the base remain clean.
remain clean. And that looks like this:

    # If .gitmodules is a symlink: 
    # - Find the path to the real file
    # - Delete the symlink
    # - Copy the real file to .gitmodules
    git filter-branch --tree-filter \
        'if [ -h .gitmodules ];then loc=$(readlink .gitmodules); rm -f .gitmodules; cp $loc .gitmodules; fi;'
    
This solved the problem. Symlinks removed and replaced with the real content
replaced by the real content. However, the complete history has now been rewritten. Locally as
as well as the newer `git` versions (clients and local server) reported
no more errors.

# Conclusion

I don't like the way `git` currently works. Symlinks help in
my case. They are transparent and available in every branch within the repository
and are maintained. 

The most important thing: they make work easier as they draw attention to changes.
changes. This saves an enormous amount of time and running around afterwards if you forget to
to correct the submodules because with an almost forward merge it is simply treated 
treated as new = better. Submodules are bound in the same branch workflow
in the same branch workflow.





Please check the rest of the error report below.
You can delete any line you do not want to report.


[System Info]
git Version:
git version 2.39.2
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.1.0-13-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.55-1 (2023-09-29) x86_64
Compiler Info: gnuc: 12.2
libc Info: glibc: 2.36
$SHELL (typically, interactive shell): /bin/zsh


[Aktivierte Hooks]


[-- Attachment #3: git-bugreport-2023-11-15-1605.txt --]
[-- Type: text/plain, Size: 7360 bytes --]

[DE](#DE)
[EN] See other text file



# DE

Vielen Dank für das Ausfüllen eines Git-Fehlerberichts!
Bitte antworten Sie auf die folgenden Fragen, um uns dabei zu helfen, Ihr
Problem zu verstehen.

Was haben Sie gemacht, bevor der Fehler auftrat? (Schritte, um Ihr Fehler
zu reproduzieren)

6 Jahre lang habe ich nur lokal zu meinem lokalen master remote pushes gemacht.
github.com hat dann eine Annahme verweigert.

Der Grund: 
Und nun sind github.com aber auch `git` selbst im Detail zickig.

github.com, da es einen Push nicht mehr akzeptiert:
    
    index-pack failed
    remote: error: object [hash]: gitmodulesSymlink: .gitmodules is a symbolic link

Und `git` selbst bricht auch ab. Ich kann die Probleme so also nicht lösen.

    git filter-branch --tree-filter 'rm -f .gitmodules' HEAD
    Rewrite [SomeHash] (3/185) (0 seconds passed, remaining 0 predicted) \
        error: Invalid path '.gitmodules' Could not initialize the index

Der gleiche Versuch scheiterte ebenfalls mit einer älteren VM, wo noch `git`
Version 2.11.0 verfügbar war.



Was haben Sie erwartet, was passieren soll? (Erwartetes Verhalten)

Rebase soll funktionieren. Immer :-)



Was ist stattdessen passiert? (Wirkliches Verhalten)

Abbruch von git.

    git filter-branch --tree-filter 'rm -f .gitmodules' HEAD
    Rewrite [SomeHash] (3/185) (0 seconds passed, remaining 0 predicted) \
        error: Invalid path '.gitmodules' Could not initialize the index



Was ist der Unterschied zwischen dem, was Sie erwartet haben und was
wirklich passiert ist?

Mit einer aktuellen git Version kann ich derartige Fehler nicht bereinigen.

Aber ansich halte ich das ausgrenzen von symlinks zu git system Dateien für einen Fehler. Es ist zu Pauschal in meinen Augen implementiert.


Sonstige Anmerkungen, die Sie hinzufügen möchten:

Meine `.gitmodules` waren immer eine passende Datei im gleichen Verzeichnis, um
bei einem Merge darauf aufmerksam zu werden, das sich etwas ändert.
Branches sind bei mir u.a Staging- Bereiche) 
```
    .gitmodules -> symlinks zu einer dieser Dateien 
                    (abhängig zum aktuellen Branch)

        -> .gitmodules_stable
                ^ ------------------ Staging to stable, release
        ->  .gitmodules_testing
                ^ ------------------ Staging to testing, testing and preview
        ->  .gitmodules_unstable
                ^ ^ ^---<---------<-- Feature branch A
                | +-----<---------<-- Feature branch B
                +-------<---------<-- Feature branch C ...
```

Bei einem merge unstable -> testing (-ff) würden Einstellungen von unstable
einfach in testing mit einbezogen/übernommen werden, ohne darauf aufmerksam zu
werden. Was auch zur Folge hat, dass man nicht mehr darauf aufmerksam gemacht
wird, das die Abhängigkeiten der Submodule ebenfalls angehoben werden müssen,
bevor der merge sauber/ fertig ist.

Wenn an mehreren Submodulen gearbeitet wird, macht das durchaus Sinn. Gerade
dann, wenn man eben nicht alles online hat oder haben will, um einen aktuellen
Stand zu prüfen/testen etc. bevor man dann die Hashes der Submodule bindet und
alles dann je nach Abhängigkeit Stück für Stück öffentlich stellt.
Was für ein Aufwand :-/

Mit `git` selbst (Version 2.39.0, Debian Bookworm, 2023) kann man lokal noch
weiter arbeiten. Bei github.com ist allerdings Schluss mit lustig. Wenn es im
Repository `git` System Dateien gibt wie `.gitignore` oder `.gitmodules` die
Symlinks sind, verweigert github.com jegliche Arbeit, die Bugs beseitigen zu
können. Weder über das Webinterface noch via `git push --force` selbst. Uff.

Also: Nach 6 Jahren! Dazwischen gab es lokal immer wieder mal Updates, die aber
nicht öffentlich gestellt wurden. Das Betriebssystem, ein Debian, wird etwa alle
zwei Jahre aktualisiert. Neue `git` Versionen sind also mind. 3 Stk. seither für
dieses Projekt dabei. Mit der aktuellen Version von `git` kann ich die Probleme
allerdings nicht beheben. Und das ist wirklich schlecht!

## Lösungsweg

Option 3. war dann die Lösung. Ein Debian Wheezy (7.3) brachte eine `git` 
Version 1.7.10.4 mit, mit der ich einen Rebase auf einen der mir bekannten und 
problembehafteten Commits zulies. Ich bin vielleicht zu weit in die
Vergangenheit gereist, aber hiermit hatte ich erst einmal eine Basis zum testen.
Vielleicht bleibt es so und alles ist wieder gut!?

Darauf folgend und, um wieder automatisch alles zu erledigen, kam mir der
git filter-branch wieder in den Gedanken:

    git filter-branch --tree-filter 'rm -f .gitmodules' HEAD

Tat seinen Dienst, half aber nichts Richtung github.com als `push --force` für
diesen Branch.

    - git filter-branch --tree-filter 'rm -f .gitmodules' HEAD
    + git filter-branch --tree-filter 'rm -f .gitmodules'

Also: komplett und über alles bitte.

Damit diese Situation aber ordentlich wird:

    .gitmodules ->- symlink zum einem dieser... ->-
    -> .gitmodules_master
    -> .gitmodules_stable
    -> .gitmodules_testing
    -> .gitmodules_unstable

'rm -f .gitmodules' löscht, löst aber nicht die zugrunde liegenden Bindungen der 
Submodule. Auch hier gab es Mischungen in der Nutzung/ Verwendung, wie die
Historie beim genauen betrachten deutlich zeigte. Aber der Fahrplan, es so zu 
machen, blieb einheitlich.

Ich habe das lokale Repository recht oft aus dem Back-up holen müssen, um dann
die vielen Steps von Tests wiederholen zu können...

Schließlich: Submodule (`.gitmodules`) waren in der Idee immer ein Symlink.
Damit es zukünftig für die Reparatur passt, müssen also alle symlinks in eine
echte Datei aufgelöst werden, damit der rebase via `git filter-branch
--tree-filter` sauber laufen kann und die Abhängigkeiten in der Basis sauber
bleiben. Und das sieht dann wie folgt aus:

    # Wenn .gitmodules ein symlink ist: 
    #   - Finde den Pfad zur echten Datei
    #   - Lösche den Symlink
    #   - Kopiere die echte Datei zu .gitmodules
    git filter-branch --tree-filter \
        'if [ -h .gitmodules ];then loc=$(readlink .gitmodules); rm -f .gitmodules; cp $loc .gitmodules; fi;'
    
Damit war dieses Problem erledigt. Symlinks weg und durch den echten Inhalt
getauscht. Allerdings wurde nun die komplette History neu geschrieben. Lokal als
auch die neueren `git` Versionen (Klienten und lokaler Server) meldeten
keinerlei Fehler mehr.

# Fazit

Ich mag diese Funktionsweise von `git` aktuell hierzu nicht. Symlinks helfen in
meinem Fall. Sie sind transparent und in jedem Branch innerhalb des Repositories
verfügbar und werden gepflegt. 

Das wichtigste: Sie erleichten die Arbeit da sie auf Veränderungen aufmerksam
machen. Das spart ungemein viel Zeit und das hinterher laufen wenn man vergisst
die Submodule zu korrigieren da es bei einem fast forward merge schlicht weg 
als neu = besser behandeld wird. Submudule werden im gleichen branch-Workflow
so gebunden.





Bitte überprüfen Sie den restlichen Teil des Fehlerberichts unten.
Sie können jede Zeile löschen, die Sie nicht mitteilen möchten.


[System Info]
git Version:
git version 2.39.2
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.1.0-13-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.55-1 (2023-09-29) x86_64
Compiler Info: gnuc: 12.2
libc Info: glibc: 2.36
$SHELL (typically, interactive shell): /bin/zsh


[Aktivierte Hooks]



# EN


^ permalink raw reply

* Re: Git Rename Detection Bug
From: Philip Oakley @ 2023-11-15 15:35 UTC (permalink / raw)
  To: Junio C Hamano, Elijah Newren
  Cc: Jeremy Pridmore, git@vger.kernel.org, Paul Baumgartner
In-Reply-To: <xmqqzfzimuv2.fsf@gitster.g>

On 12/11/2023 23:09, Junio C Hamano wrote:
> Elijah Newren <newren@gmail.com> writes:
> 
>>> Could I suggest that we are missing a piece of terminology, to wit,
>>> BLOBSAME. It's a compatriot to TREESAME, as used in `git log` for
>>> history simplification (based on a tree's pathspec, most commonly a
>>> commit's top level path).
>>
>> We could add it, but I'm not sure how it helps.  We already had 'exact
>> rename' which seems to fit the bill as well, and 'blob' is something
>> someone new to Git is unlikely to know.
> 
> Also, as Philip said, TREESAME is a concept foreign to rename
> detection codepath.  It is a property of a commit (not a tree)

That (property of a commit?) wasn't really obvious to me.

I'd always thought of it as a comparison between two trees, commonly
those associated with two commits. Though it could also be thought of as
the operation "TREESAME to" that binds in the second tree.

 and
> tells us if it has the same tree object as its relevant parents (in
> which case it can be simplified away if it is a merge).  I do not
> mind rename codepath using a jargon (or two) to express "in trees A
> and B, this subtree of A records the same tree object as a subtree
> of B at a different path (i.e., the contents of these two subtrees
> at different paths are the same)" but the word used to express that
> should not be TREESAME to avoid confusion.

Maybe it's that the explanation of TREESAME in
rev-list-options.txt#L419-L436 [1] has a similar set of confusions about
how subtrees are considered and the path v filename confusions.

  And the other word to
> express "this path in tree A records a blob object that is identical
> to this other path in tree B" should not be BLOBSAME, as the word
> strongly hints it is somehow related to TREESAME.

Yep. Naming is hard.

> 
> Thanks.
> 
> 
Philip

[1]
https://github.com/git/git/blob/master/Documentation/rev-list-options.txt#L419-L436

^ permalink raw reply

* Re: [PATCH v5 0/3] rebase: support --autosquash without -i
From: Phillip Wood @ 2023-11-15 15:09 UTC (permalink / raw)
  To: Andy Koppe, git; +Cc: gitster, phillip.wood
In-Reply-To: <20231114214339.10925-1-andy.koppe@gmail.com>

Hi Andy

On 14/11/2023 21:43, Andy Koppe wrote:
> Make rebase --autosquash work without --interactive, but limit
> rebase.autoSquash's effects to interactive mode, and improve testing
> and documentation.
> 
> Changes from v4:
> - Fix amend vs apply backend thinko in commit messages.
> - Squash patch 3 for testing into patch 2 and improve the commit
>    message.
> - No source changes.
> 
> Thanks again to Junio and Phillip for their reviews.

Thanks for the re-roll this version looks good to me

Best Wishes

Phillip

> 
> Andy Koppe (3):
>    rebase: fully ignore rebase.autoSquash without -i
>    rebase: support --autosquash without -i
>    rebase: rewrite --(no-)autosquash documentation
> 
>   Documentation/config/rebase.txt        |  4 ++-
>   Documentation/git-rebase.txt           | 34 +++++++++++++----------
>   builtin/rebase.c                       | 17 +++++-------
>   t/t3415-rebase-autosquash.sh           | 38 +++++++++++++++++++-------
>   t/t3422-rebase-incompatible-options.sh | 12 --------
>   5 files changed, 58 insertions(+), 47 deletions(-)
> 

^ permalink raw reply

* Re: [PATCH v6 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-11-15 14:51 UTC (permalink / raw)
  To: Elijah Newren, Johannes Schindelin
  Cc: git, Junio C Hamano, Patrick Steinhardt, John Cai, Derrick Stolee,
	Phillip Wood, Calvin Wan, Toon Claes, Dragan Simic, Linus Arver
In-Reply-To: <CAP8UFD24fzhiecJtANqEsxvh1mxT4pKR=QjfUFZh8C6HQE-k1A@mail.gmail.com>

Hi Elijah and Dscho,

On Tue, Nov 7, 2023 at 10:43 AM Christian Couder
<christian.couder@gmail.com> wrote:
> On Tue, Nov 7, 2023 at 3:44 AM Elijah Newren <newren@gmail.com> wrote:
> > Looking good, just one comment on one small hunk...
> >
> > On Thu, Nov 2, 2023 at 6:52 AM Christian Couder
> > <christian.couder@gmail.com> wrote:
> > >
> > [...]
> >
> > >     @@ builtin/replay.c: int cmd_replay(int argc, const char **argv, const char *prefix
> > >      -
> > >         strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
> > >
> > >     ++  /*
> > >     ++   * TODO: For now, let's warn when we see an option that we are
> > >     ++   * going to override after setup_revisions() below. In the
> > >     ++   * future we might want to either die() or allow them if we
> > >     ++   * think they could be useful though.
> > >     ++   */
> > >     ++  for (i = 0; i < argc; i++) {
> > >     ++          if (!strcmp(argv[i], "--reverse") || !strcmp(argv[i], "--date-order") ||
> > >     ++              !strcmp(argv[i], "--topo-order") || !strcmp(argv[i], "--author-date-order") ||
> > >     ++              !strcmp(argv[i], "--full-history"))
> > >     ++                  warning(_("option '%s' will be overridden"), argv[i]);
> > >     ++  }
> > >     ++

> > 2) This seems like an inefficient way to provide this warning; could
> > we avoid parsing the arguments for an extra time?  Perhaps instead
> >   a) set the desired values here, before setup_revisions()
> >   b) after setup_revisions, check whether these values differ from the
> > desired values, if so throw a warning.
> >   c) set the desired values, again
>
> Yeah, that would work. The downside is that it would be more difficult
> in the warning to tell the user which command line option was
> overridden as there are some values changed by different options.
> Maybe we can come up with a warning message that is still useful and
> enough for now, as the command is supposed to be used by experienced
> users. Perhaps something like:
>
> warning(_("some rev walking options will be overridden as '%s' bit in
> 'struct rev_info' will be forced"), "sort_order");

I have implemented this in the v7 I just sent.

Thanks!

^ permalink raw reply

* Re: [PATCH v6 00/14] Introduce new `git replay` command
From: Christian Couder @ 2023-11-15 14:46 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Junio C Hamano, Patrick Steinhardt, Elijah Newren, John Cai,
	Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
	Dragan Simic, Linus Arver
In-Reply-To: <fcfacd1a-cf5a-a393-d2e0-3c0388ae3529@gmx.de>

Hi Dscho,

On Wed, Nov 8, 2023 at 1:47 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Thu, 2 Nov 2023, Christian Couder wrote:

> >     + ## Documentation/git-replay.txt (new) ##
> >     +@@
> >     ++git-replay(1)
> >     ++=============
> >     ++
> >     ++NAME
> >     ++----
> >     ++git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos too
> >     ++
> >     ++
> >     ++SYNOPSIS
> >     ++--------
> >     ++[verse]
> >     ++'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
>
> Technically, at this stage `git replay` requires precisely 5 arguments, so
> the `<revision>...` is incorrect and should be `<upstream> <branch>`
> instead. But it's not worth a new iteration to fix this.

It was actually:

'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL

(you can see it there:
https://lore.kernel.org/git/20231102135151.843758-3-christian.couder@gmail.com/)

But I made a mistake in the range-diff command as I ran it against a
previous development version instead of the current one.

> >     ++
> >     ++DESCRIPTION
> >     ++-----------
> >     ++
> >     ++Takes a range of commits and replays them onto a new location.
> >     ++
> >     ++THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
> >     ++
> >     ++OPTIONS
> >     ++-------
> >     ++
> >     ++--onto <newbase>::
> >     ++  Starting point at which to create the new commits.  May be any
> >     ++  valid commit, and not just an existing branch name.
> >     ++
>
> Traditionally, this would be a place to describe the `<revision>` argument
> (or, in this patch, to reflect the current state of `builtin/replay.c`,
> the `<upstream> <branch>` arguments instead).

I have fixed that in the v7 I just sent with the following:

+SYNOPSIS
+--------
+[verse]
+'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
+
+DESCRIPTION
+-----------
+
+Takes a range of commits, specified by <oldbase> and <branch>, and
+replays them onto a new location (see `--onto` option below).
+
+THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
+

Thanks for your review!

^ permalink raw reply

* Re: Git Rename Detection Bug
From: Philip Oakley @ 2023-11-15 14:36 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Jeremy Pridmore, git@vger.kernel.org, Paul Baumgartner
In-Reply-To: <CABPp-BEtva2WTGQG3Qs4EbZLK_RJC9vuA-2OYxkTPExgowwvqQ@mail.gmail.com>

Hi Elijah,
sorry for the delay in replying.

On 11/11/2023 15:13, Elijah Newren wrote:
> Hi,
> 
> On Sat, Nov 11, 2023 at 3:08 AM Philip Oakley <philipoakley@iee.email> wrote:
>>
>> Hi all,
>>
>> On 11/11/2023 05:46, Elijah Newren wrote:
>>> The fact that you were trying to "undo" renames and "redo the correct
>>> ones" suggested there's something you still didn't understand about
>>> rename detection, though.
>>
>>
>> Could I suggest that we are missing a piece of terminology, to wit,
>> BLOBSAME. It's a compatriot to TREESAME, as used in `git log` for
>> history simplification (based on a tree's pathspec, most commonly a
>> commit's top level path).
> 
> We could add it, but I'm not sure how it helps.  We already had 'exact
> rename' which seems to fit the bill as well,

My point was that we already had the confusion of mental models, with
both sides essentially thinking they had an "exact rename", hence my
thought was to add a rather distinct technical name which reflected the
Git mind-shift. Without something to bring folks up short they'll
continue, erroneously, with their prior mental models.


 and 'blob' is something
> someone new to Git is unlikely to know.

I'd agree that BLOBSAME is new, but we should be proactive in ensuring
folk do have the mind shift from the old centralised VCS misunderstandings.

> 
> Perhaps it's useful in some other context, though?
> 
>> File rename, at it's most basic, is when the blob associated with that
>> changed path is identical, i.e. BLOBSAME. There is no need to 'record'
>> the action of renaming, moving or whatever, the content sameness is
>> right there, in plain sight, as an identical blob name.   After that
>> (files with slight variations) it is a load of heuristics, but starting
>> with BLOBSAME we see how easy the basic rename detection is, and why
>> renames (and de-dup) don't need recording.
> 
> This is incorrect.  Let's say you have a file foo:
>    * base version: foo has hash A
>    * our version: foo has been renamed to bar, but bar still has hash A
>    * their version: foo has been modified; it now has hash B
> 
> The foo->bar is an exact rename (or they are BLOBSAME if you prefer),
> but the renaming/moving/whatever is a critical piece of information
> because the changes to foo in 'their' version need to be applied to
> bar to get the correct end results.

Isn't that what I thought I'd said?
Hash A = Hash A => identical content;
Hash A != B => different content.

> 
> I do not know if in Jeremy's case foo has been modified on the
> unrenamed side.  But the following hypothetical is exactly the type of
> problem Jeremy is hitting: what should happen when 'our' version has
> both a new 'bar' and a new 'baz' file that each have hash A?  In that
> case, to which one was foo renamed?  It's inherently ambiguous.

true, the terminology hasn't kept up with the methodology for blob
content, and the independent meta-data. In previous 'ort' discussions I
didn't really understand what the '1/2' renames (and other
nomenclatures) really meant with respect to paths, filenames, content
and the ours / theirs / base distinctions.
> 
>> The heuristics of 'rename with small change' is trickier, but for a
>> basic understanding, starting at BLOBSAME (and TREESAME for directory
>> renames) should make it easier to grasp the concepts.
> 
> Interesting; TREESAME isn't used within directory rename detection
> currently; it is only used currently when two (or three) trees with
> the same name are TREESAME, in order to potentially avoid recursing
> into the tree.  But even then, having two trees with the same name be
> TREESAME isn't enough on its own to avoid recursing into that tree,
> because the other side could have added files within the same-named
> tree and we need to know about those added files because they could be
> part of renames involving other files outside that tree. 

definitely easy to get confused on these cases...

>      There would
> probably be similar challenges to attempting to apply the concept of
> TREESAME to directory rename detection to two trees of different
> names, but it's at least an interesting idea.  Hmm....
> 


Thanks for the insights.

Philip

^ permalink raw reply

* [PATCH v7 14/14] replay: stop assuming replayed branches do not diverge
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

The replay command is able to replay multiple branches but when some of
them are based on other replayed branches, their commit should be
replayed onto already replayed commits.

For this purpose, let's store the replayed commit and its original
commit in a key value store, so that we can easily find and reuse a
replayed commit instead of the original one.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c         | 44 ++++++++++++++++++++++++++--------
 t/t3650-replay-basics.sh | 52 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index e14e33bcc5..f37e511d8e 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -223,20 +223,33 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
 	strset_clear(&rinfo.positive_refs);
 }
 
+static struct commit *mapped_commit(kh_oid_map_t *replayed_commits,
+				    struct commit *commit,
+				    struct commit *fallback)
+{
+	khint_t pos = kh_get_oid_map(replayed_commits, commit->object.oid);
+	if (pos == kh_end(replayed_commits))
+		return fallback;
+	return kh_value(replayed_commits, pos);
+}
+
 static struct commit *pick_regular_commit(struct commit *pickme,
-					  struct commit *last_commit,
+					  kh_oid_map_t *replayed_commits,
+					  struct commit *onto,
 					  struct merge_options *merge_opt,
 					  struct merge_result *result)
 {
-	struct commit *base;
+	struct commit *base, *replayed_base;
 	struct tree *pickme_tree, *base_tree;
 
 	base = pickme->parents->item;
+	replayed_base = mapped_commit(replayed_commits, base, onto);
 
+	result->tree = repo_get_commit_tree(the_repository, replayed_base);
 	pickme_tree = repo_get_commit_tree(the_repository, pickme);
 	base_tree = repo_get_commit_tree(the_repository, base);
 
-	merge_opt->branch1 = short_commit_name(last_commit);
+	merge_opt->branch1 = short_commit_name(replayed_base);
 	merge_opt->branch2 = short_commit_name(pickme);
 	merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
 
@@ -250,7 +263,7 @@ static struct commit *pick_regular_commit(struct commit *pickme,
 	merge_opt->ancestor = NULL;
 	if (!result->clean)
 		return NULL;
-	return create_commit(result->tree, pickme, last_commit);
+	return create_commit(result->tree, pickme, replayed_base);
 }
 
 int cmd_replay(int argc, const char **argv, const char *prefix)
@@ -266,6 +279,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct merge_options merge_opt;
 	struct merge_result result;
 	struct strset *update_refs = NULL;
+	kh_oid_map_t *replayed_commits;
 	int ret = 0;
 
 	const char * const replay_usage[] = {
@@ -362,21 +376,30 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	init_merge_options(&merge_opt, the_repository);
 	memset(&result, 0, sizeof(result));
 	merge_opt.show_rename_progress = 0;
-
-	result.tree = repo_get_commit_tree(the_repository, onto);
 	last_commit = onto;
+	replayed_commits = kh_init_oid_map();
 	while ((commit = get_revision(&revs))) {
 		const struct name_decoration *decoration;
+		khint_t pos;
+		int hr;
 
 		if (!commit->parents)
 			die(_("replaying down to root commit is not supported yet!"));
 		if (commit->parents->next)
 			die(_("replaying merge commits is not supported yet!"));
 
-		last_commit = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+		last_commit = pick_regular_commit(commit, replayed_commits, onto,
+						  &merge_opt, &result);
 		if (!last_commit)
 			break;
 
+		/* Record commit -> last_commit mapping */
+		pos = kh_put_oid_map(replayed_commits, commit->object.oid, &hr);
+		if (hr == 0)
+			BUG("Duplicate rewritten commit: %s\n",
+			    oid_to_hex(&commit->object.oid));
+		kh_value(replayed_commits, pos) = last_commit;
+
 		/* Update any necessary branches */
 		if (advance_name)
 			continue;
@@ -405,13 +428,14 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	}
 
 	merge_finalize(&merge_opt, &result);
-	ret = result.clean;
-
-cleanup:
+	kh_destroy_oid_map(replayed_commits);
 	if (update_refs) {
 		strset_clear(update_refs);
 		free(update_refs);
 	}
+	ret = result.clean;
+
+cleanup:
 	release_revisions(&revs);
 
 	/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index d6286f9580..389670262e 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -143,4 +143,56 @@ test_expect_success 'using replay on bare repo to also rebase a contained branch
 	test_cmp expect result-bare
 '
 
+test_expect_success 'using replay to rebase multiple divergent branches' '
+	git replay --onto main ^topic1 topic2 topic4 >result &&
+
+	test_line_count = 2 result &&
+	cut -f 3 -d " " result >new-branch-tips &&
+
+	git log --format=%s $(head -n 1 new-branch-tips) >actual &&
+	test_write_lines E D M L B A >expect &&
+	test_cmp expect actual &&
+
+	git log --format=%s $(tail -n 1 new-branch-tips) >actual &&
+	test_write_lines J I M L B A >expect &&
+	test_cmp expect actual &&
+
+	printf "update refs/heads/topic2 " >expect &&
+	printf "%s " $(head -n 1 new-branch-tips) >>expect &&
+	git rev-parse topic2 >>expect &&
+	printf "update refs/heads/topic4 " >>expect &&
+	printf "%s " $(tail -n 1 new-branch-tips) >>expect &&
+	git rev-parse topic4 >>expect &&
+
+	test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to rebase multiple divergent branches, including contained ones' '
+	git -C bare replay --contained --onto main ^main topic2 topic3 topic4 >result &&
+
+	test_line_count = 4 result &&
+	cut -f 3 -d " " result >new-branch-tips &&
+
+	>expect &&
+	for i in 2 1 3 4
+	do
+		printf "update refs/heads/topic$i " >>expect &&
+		printf "%s " $(grep topic$i result | cut -f 3 -d " ") >>expect &&
+		git -C bare rev-parse topic$i >>expect || return 1
+	done &&
+
+	test_cmp expect result &&
+
+	test_write_lines F C M L B A >expect1 &&
+	test_write_lines E D C M L B A >expect2 &&
+	test_write_lines H G F C M L B A >expect3 &&
+	test_write_lines J I M L B A >expect4 &&
+
+	for i in 1 2 3 4
+	do
+		git -C bare log --format=%s $(grep topic$i result | cut -f 3 -d " ") >actual &&
+		test_cmp expect$i actual || return 1
+	done
+'
+
 test_done
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 13/14] replay: add --contained to rebase contained branches
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Let's add a `--contained` option that can be used along with
`--onto` to rebase all the branches contained in the <revision-range>
argument.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replay.txt | 12 +++++++++++-
 builtin/replay.c             | 13 +++++++++++--
 t/t3650-replay-basics.sh     | 29 +++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index 6daa5b4275..133d7af9ee 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -9,7 +9,7 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
 SYNOPSIS
 --------
 [verse]
-'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
+'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
 
 DESCRIPTION
 -----------
@@ -96,6 +96,16 @@ top of the exact same new base, they only differ in that the first
 provides instructions to make mybranch point at the new commits and
 the second provides instructions to make target point at them.
 
+What if you have a stack of branches, one depending upon another, and
+you'd really like to rebase the whole set?
+
+------------
+$ git replay --contained --onto origin/main origin/main..tipbranch
+update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+update refs/heads/tipbranch ${NEW_tipbranch_HASH} ${OLD_tipbranch_HASH}
+------------
+
 When calling `git replay`, one does not need to specify a range of
 commits to replay using the syntax `A..B`; any range expression will
 do:
diff --git a/builtin/replay.c b/builtin/replay.c
index 1a419cb7fd..e14e33bcc5 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -258,6 +258,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	const char *advance_name = NULL;
 	struct commit *onto = NULL;
 	const char *onto_name = NULL;
+	int contained = 0;
 
 	struct rev_info revs;
 	struct commit *last_commit = NULL;
@@ -268,7 +269,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	int ret = 0;
 
 	const char * const replay_usage[] = {
-		N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL"),
+		N_("git replay ([--contained] --onto <newbase> | --advance <branch>) "
+		   "<revision-range>... # EXPERIMENTAL"),
 		NULL
 	};
 	struct option replay_options[] = {
@@ -278,6 +280,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "onto", &onto_name,
 			   N_("revision"),
 			   N_("replay onto given commit")),
+		OPT_BOOL(0, "contained", &contained,
+			 N_("advance all branches contained in revision-range")),
 		OPT_END()
 	};
 
@@ -289,6 +293,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		usage_with_options(replay_usage, replay_options);
 	}
 
+	if (advance_name && contained)
+		die(_("options '%s' and '%s' cannot be used together"),
+		    "--advance", "--contained");
+
 	repo_init_revisions(the_repository, &revs, prefix);
 
 	/*
@@ -377,7 +385,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			continue;
 		while (decoration) {
 			if (decoration->type == DECORATION_REF_LOCAL &&
-			    strset_contains(update_refs, decoration->name)) {
+			    (contained || strset_contains(update_refs,
+							  decoration->name))) {
 				printf("update %s %s %s\n",
 				       decoration->name,
 				       oid_to_hex(&last_commit->object.oid),
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 68a87e7803..d6286f9580 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -114,4 +114,33 @@ test_expect_success 'replay fails when both --advance and --onto are omitted' '
 	test_must_fail git replay topic1..topic2 >result
 '
 
+test_expect_success 'using replay to also rebase a contained branch' '
+	git replay --contained --onto main main..topic3 >result &&
+
+	test_line_count = 2 result &&
+	cut -f 3 -d " " result >new-branch-tips &&
+
+	git log --format=%s $(head -n 1 new-branch-tips) >actual &&
+	test_write_lines F C M L B A >expect &&
+	test_cmp expect actual &&
+
+	git log --format=%s $(tail -n 1 new-branch-tips) >actual &&
+	test_write_lines H G F C M L B A >expect &&
+	test_cmp expect actual &&
+
+	printf "update refs/heads/topic1 " >expect &&
+	printf "%s " $(head -n 1 new-branch-tips) >>expect &&
+	git rev-parse topic1 >>expect &&
+	printf "update refs/heads/topic3 " >>expect &&
+	printf "%s " $(tail -n 1 new-branch-tips) >>expect &&
+	git rev-parse topic3 >>expect &&
+
+	test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to also rebase a contained branch' '
+	git -C bare replay --contained --onto main main..topic3 >result-bare &&
+	test_cmp expect result-bare
+'
+
 test_done
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 12/14] replay: add --advance or 'cherry-pick' mode
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

There is already a 'rebase' mode with `--onto`. Let's add an 'advance' or
'cherry-pick' mode with `--advance`. This new mode will make the target
branch advance as we replay commits onto it.

The replayed commits should have a single tip, so that it's clear where
the target branch should be advanced. If they have more than one tip,
this new mode will error out.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replay.txt |  41 ++++++--
 builtin/replay.c             | 185 +++++++++++++++++++++++++++++++++--
 t/t3650-replay-basics.sh     |  34 +++++++
 3 files changed, 243 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index fab4ea0178..6daa5b4275 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -9,7 +9,7 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
 SYNOPSIS
 --------
 [verse]
-'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
+'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL
 
 DESCRIPTION
 -----------
@@ -29,14 +29,25 @@ OPTIONS
 	Starting point at which to create the new commits.  May be any
 	valid commit, and not just an existing branch name.
 +
-The update-ref command(s) in the output will update the branch(es) in
-the revision range to point at the new commits, similar to the way how
-`git rebase --update-refs` updates multiple branches in the affected
-range.
+When `--onto` is specified, the update-ref command(s) in the output will
+update the branch(es) in the revision range to point at the new
+commits, similar to the way how `git rebase --update-refs` updates
+multiple branches in the affected range.
+
+--advance <branch>::
+	Starting point at which to create the new commits; must be a
+	branch name.
++
+When `--advance` is specified, the update-ref command(s) in the output
+will update the branch passed as an argument to `--advance` to point at
+the new commits (in other words, this mimics a cherry-pick operation).
 
 <revision-range>::
-	Range of commits to replay; see "Specifying Ranges" in
-	linkgit:git-rev-parse and the "Commit Limiting" options below.
+	Range of commits to replay. More than one <revision-range> can
+	be passed, but in `--advance <branch>` mode, they should have
+	a single tip, so that it's clear where <branch> should point
+	to. See "Specifying Ranges" in linkgit:git-rev-parse and the
+	"Commit Limiting" options below.
 
 include::rev-list-options.txt[]
 
@@ -51,7 +62,9 @@ input to `git update-ref --stdin`.  It is of the form:
 	update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
 
 where the number of refs updated depends on the arguments passed and
-the shape of the history being replayed.
+the shape of the history being replayed.  When using `--advance`, the
+number of refs updated is always one, but for `--onto`, it can be one
+or more (rebasing multiple branches simultaneously is supported).
 
 EXIT STATUS
 -----------
@@ -71,6 +84,18 @@ $ git replay --onto target origin/main..mybranch
 update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH}
 ------------
 
+To cherry-pick the commits from mybranch onto target:
+
+------------
+$ git replay --advance target origin/main..mybranch
+update refs/heads/target ${NEW_target_HASH} ${OLD_target_HASH}
+------------
+
+Note that the first two examples replay the exact same commits and on
+top of the exact same new base, they only differ in that the first
+provides instructions to make mybranch point at the new commits and
+the second provides instructions to make target point at them.
+
 When calling `git replay`, one does not need to specify a range of
 commits to replay using the syntax `A..B`; any range expression will
 do:
diff --git a/builtin/replay.c b/builtin/replay.c
index 7a660020d1..1a419cb7fd 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -14,6 +14,7 @@
 #include "parse-options.h"
 #include "refs.h"
 #include "revision.h"
+#include "strmap.h"
 #include <oidset.h>
 #include <tree.h>
 
@@ -82,6 +83,146 @@ static struct commit *create_commit(struct tree *tree,
 	return (struct commit *)obj;
 }
 
+struct ref_info {
+	struct commit *onto;
+	struct strset positive_refs;
+	struct strset negative_refs;
+	int positive_refexprs;
+	int negative_refexprs;
+};
+
+static void get_ref_information(struct rev_cmdline_info *cmd_info,
+				struct ref_info *ref_info)
+{
+	int i;
+
+	ref_info->onto = NULL;
+	strset_init(&ref_info->positive_refs);
+	strset_init(&ref_info->negative_refs);
+	ref_info->positive_refexprs = 0;
+	ref_info->negative_refexprs = 0;
+
+	/*
+	 * When the user specifies e.g.
+	 *   git replay origin/main..mybranch
+	 *   git replay ^origin/next mybranch1 mybranch2
+	 * we want to be able to determine where to replay the commits.  In
+	 * these examples, the branches are probably based on an old version
+	 * of either origin/main or origin/next, so we want to replay on the
+	 * newest version of that branch.  In contrast we would want to error
+	 * out if they ran
+	 *   git replay ^origin/master ^origin/next mybranch
+	 *   git replay mybranch~2..mybranch
+	 * the first of those because there's no unique base to choose, and
+	 * the second because they'd likely just be replaying commits on top
+	 * of the same commit and not making any difference.
+	 */
+	for (i = 0; i < cmd_info->nr; i++) {
+		struct rev_cmdline_entry *e = cmd_info->rev + i;
+		struct object_id oid;
+		const char *refexpr = e->name;
+		char *fullname = NULL;
+		int can_uniquely_dwim = 1;
+
+		if (*refexpr == '^')
+			refexpr++;
+		if (repo_dwim_ref(the_repository, refexpr, strlen(refexpr), &oid, &fullname, 0) != 1)
+			can_uniquely_dwim = 0;
+
+		if (e->flags & BOTTOM) {
+			if (can_uniquely_dwim)
+				strset_add(&ref_info->negative_refs, fullname);
+			if (!ref_info->negative_refexprs)
+				ref_info->onto = lookup_commit_reference_gently(the_repository,
+										&e->item->oid, 1);
+			ref_info->negative_refexprs++;
+		} else {
+			if (can_uniquely_dwim)
+				strset_add(&ref_info->positive_refs, fullname);
+			ref_info->positive_refexprs++;
+		}
+
+		free(fullname);
+	}
+}
+
+static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
+				  const char *onto_name,
+				  const char **advance_name,
+				  struct commit **onto,
+				  struct strset **update_refs)
+{
+	struct ref_info rinfo;
+
+	get_ref_information(cmd_info, &rinfo);
+	if (!rinfo.positive_refexprs)
+		die(_("need some commits to replay"));
+	if (onto_name && *advance_name)
+		die(_("--onto and --advance are incompatible"));
+	else if (onto_name) {
+		*onto = peel_committish(onto_name);
+		if (rinfo.positive_refexprs <
+		    strset_get_size(&rinfo.positive_refs))
+			die(_("all positive revisions given must be references"));
+	} else if (*advance_name) {
+		struct object_id oid;
+		char *fullname = NULL;
+
+		*onto = peel_committish(*advance_name);
+		if (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name),
+			     &oid, &fullname, 0) == 1) {
+			*advance_name = fullname;
+		} else {
+			die(_("argument to --advance must be a reference"));
+		}
+		if (rinfo.positive_refexprs > 1)
+			die(_("cannot advance target with multiple sources because ordering would be ill-defined"));
+	} else {
+		int positive_refs_complete = (
+			rinfo.positive_refexprs ==
+			strset_get_size(&rinfo.positive_refs));
+		int negative_refs_complete = (
+			rinfo.negative_refexprs ==
+			strset_get_size(&rinfo.negative_refs));
+		/*
+		 * We need either positive_refs_complete or
+		 * negative_refs_complete, but not both.
+		 */
+		if (rinfo.negative_refexprs > 0 &&
+		    positive_refs_complete == negative_refs_complete)
+			die(_("cannot implicitly determine whether this is an --advance or --onto operation"));
+		if (negative_refs_complete) {
+			struct hashmap_iter iter;
+			struct strmap_entry *entry;
+
+			if (rinfo.negative_refexprs == 0)
+				die(_("all positive revisions given must be references"));
+			else if (rinfo.negative_refexprs > 1)
+				die(_("cannot implicitly determine whether this is an --advance or --onto operation"));
+			else if (rinfo.positive_refexprs > 1)
+				die(_("cannot advance target with multiple source branches because ordering would be ill-defined"));
+
+			/* Only one entry, but we have to loop to get it */
+			strset_for_each_entry(&rinfo.negative_refs,
+					      &iter, entry) {
+				*advance_name = entry->key;
+			}
+		} else { /* positive_refs_complete */
+			if (rinfo.negative_refexprs > 1)
+				die(_("cannot implicitly determine correct base for --onto"));
+			if (rinfo.negative_refexprs == 1)
+				*onto = rinfo.onto;
+		}
+	}
+	if (!*advance_name) {
+		*update_refs = xcalloc(1, sizeof(**update_refs));
+		**update_refs = rinfo.positive_refs;
+		memset(&rinfo.positive_refs, 0, sizeof(**update_refs));
+	}
+	strset_clear(&rinfo.negative_refs);
+	strset_clear(&rinfo.positive_refs);
+}
+
 static struct commit *pick_regular_commit(struct commit *pickme,
 					  struct commit *last_commit,
 					  struct merge_options *merge_opt,
@@ -114,20 +255,26 @@ static struct commit *pick_regular_commit(struct commit *pickme,
 
 int cmd_replay(int argc, const char **argv, const char *prefix)
 {
-	struct commit *onto;
+	const char *advance_name = NULL;
+	struct commit *onto = NULL;
 	const char *onto_name = NULL;
-	struct commit *last_commit = NULL;
+
 	struct rev_info revs;
+	struct commit *last_commit = NULL;
 	struct commit *commit;
 	struct merge_options merge_opt;
 	struct merge_result result;
+	struct strset *update_refs = NULL;
 	int ret = 0;
 
 	const char * const replay_usage[] = {
-		N_("git replay --onto <newbase> <revision-range>... # EXPERIMENTAL"),
+		N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>... # EXPERIMENTAL"),
 		NULL
 	};
 	struct option replay_options[] = {
+		OPT_STRING(0, "advance", &advance_name,
+			   N_("branch"),
+			   N_("make replay advance given branch")),
 		OPT_STRING(0, "onto", &onto_name,
 			   N_("revision"),
 			   N_("replay onto given commit")),
@@ -137,13 +284,11 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
 
-	if (!onto_name) {
-		error(_("option --onto is mandatory"));
+	if (!onto_name && !advance_name) {
+		error(_("option --onto or --advance is mandatory"));
 		usage_with_options(replay_usage, replay_options);
 	}
 
-	onto = peel_committish(onto_name);
-
 	repo_init_revisions(the_repository, &revs, prefix);
 
 	/*
@@ -195,6 +340,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		revs.simplify_history = 0;
 	}
 
+	determine_replay_mode(&revs.cmdline, onto_name, &advance_name,
+			      &onto, &update_refs);
+
+	if (!onto) /* FIXME: Should handle replaying down to root commit */
+		die("Replaying down to root commit is not supported yet!");
+
 	if (prepare_revision_walk(&revs) < 0) {
 		ret = error(_("error preparing revisions"));
 		goto cleanup;
@@ -203,6 +354,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	init_merge_options(&merge_opt, the_repository);
 	memset(&result, 0, sizeof(result));
 	merge_opt.show_rename_progress = 0;
+
 	result.tree = repo_get_commit_tree(the_repository, onto);
 	last_commit = onto;
 	while ((commit = get_revision(&revs))) {
@@ -217,12 +369,15 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		if (!last_commit)
 			break;
 
+		/* Update any necessary branches */
+		if (advance_name)
+			continue;
 		decoration = get_name_decoration(&commit->object);
 		if (!decoration)
 			continue;
-
 		while (decoration) {
-			if (decoration->type == DECORATION_REF_LOCAL) {
+			if (decoration->type == DECORATION_REF_LOCAL &&
+			    strset_contains(update_refs, decoration->name)) {
 				printf("update %s %s %s\n",
 				       decoration->name,
 				       oid_to_hex(&last_commit->object.oid),
@@ -232,10 +387,22 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		}
 	}
 
+	/* In --advance mode, advance the target ref */
+	if (result.clean == 1 && advance_name) {
+		printf("update %s %s %s\n",
+		       advance_name,
+		       oid_to_hex(&last_commit->object.oid),
+		       oid_to_hex(&onto->object.oid));
+	}
+
 	merge_finalize(&merge_opt, &result);
 	ret = result.clean;
 
 cleanup:
+	if (update_refs) {
+		strset_clear(update_refs);
+		free(update_refs);
+	}
 	release_revisions(&revs);
 
 	/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index a1da4f9ef9..68a87e7803 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -80,4 +80,38 @@ test_expect_success 'using replay on bare repo to rebase with a conflict' '
 	test_expect_code 1 git -C bare replay --onto topic1 B..conflict
 '
 
+test_expect_success 'using replay to perform basic cherry-pick' '
+	# The differences between this test and previous ones are:
+	#   --advance vs --onto
+	# 2nd field of result is refs/heads/main vs. refs/heads/topic2
+	# 4th field of result is hash for main instead of hash for topic2
+
+	git replay --advance main topic1..topic2 >result &&
+
+	test_line_count = 1 result &&
+
+	git log --format=%s $(cut -f 3 -d " " result) >actual &&
+	test_write_lines E D M L B A >expect &&
+	test_cmp expect actual &&
+
+	printf "update refs/heads/main " >expect &&
+	printf "%s " $(cut -f 3 -d " " result) >>expect &&
+	git rev-parse main >>expect &&
+
+	test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to perform basic cherry-pick' '
+	git -C bare replay --advance main topic1..topic2 >result-bare &&
+	test_cmp expect result-bare
+'
+
+test_expect_success 'replay on bare repo fails with both --advance and --onto' '
+	test_must_fail git -C bare replay --advance main --onto main topic1..topic2 >result-bare
+'
+
+test_expect_success 'replay fails when both --advance and --onto are omitted' '
+	test_must_fail git replay topic1..topic2 >result
+'
+
 test_done
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 11/14] replay: use standard revision ranges
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Instead of the fixed "<oldbase> <branch>" arguments, the replay
command now accepts "<revision-range>..." arguments in a similar
way as many other Git commands. This makes its interface more
standard and more flexible.

This also enables many revision related options accepted and
eaten by setup_revisions(). If the replay command was a high level
one or had a high level mode, it would make sense to restrict some
of the possible options, like those generating non-contiguous
history, as they could be confusing for most users.

Also as the interface of the command is now mostly finalized,
we can add more documentation and more testcases to make sure
the command will continue to work as designed in the future.

We only document the rev-list related options among all the
revision related options that are now accepted, as the rev-list
related ones are probably the most useful for now.

Helped-by: Dragan Simic <dsimic@manjaro.org>
Helped-by: Linus Arver <linusa@google.com>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replay.txt             | 58 ++++++++++++++++++++++--
 builtin/replay.c                         | 21 ++-------
 t/t3650-replay-basics.sh                 | 12 ++++-
 t/t6429-merge-sequence-rename-caching.sh | 18 ++++----
 4 files changed, 77 insertions(+), 32 deletions(-)

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index 87a85a7f57..fab4ea0178 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -9,16 +9,16 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t
 SYNOPSIS
 --------
 [verse]
-'git replay' --onto <newbase> <oldbase> <branch> # EXPERIMENTAL
+'git replay' --onto <newbase> <revision-range>... # EXPERIMENTAL
 
 DESCRIPTION
 -----------
 
-Takes a range of commits, specified by <oldbase> and <branch>, and
-replays them onto a new location (see `--onto` option below). Leaves
+Takes ranges of commits and replays them onto a new location. Leaves
 the working tree and the index untouched, and updates no references.
 The output of this command is meant to be used as input to
-`git update-ref --stdin`, which would update the relevant branches.
+`git update-ref --stdin`, which would update the relevant branches
+(see the OUTPUT section below).
 
 THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
 
@@ -28,6 +28,30 @@ OPTIONS
 --onto <newbase>::
 	Starting point at which to create the new commits.  May be any
 	valid commit, and not just an existing branch name.
++
+The update-ref command(s) in the output will update the branch(es) in
+the revision range to point at the new commits, similar to the way how
+`git rebase --update-refs` updates multiple branches in the affected
+range.
+
+<revision-range>::
+	Range of commits to replay; see "Specifying Ranges" in
+	linkgit:git-rev-parse and the "Commit Limiting" options below.
+
+include::rev-list-options.txt[]
+
+OUTPUT
+------
+
+When there are no conflicts, the output of this command is usable as
+input to `git update-ref --stdin`.  It is of the form:
+
+	update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+	update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+	update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
+
+where the number of refs updated depends on the arguments passed and
+the shape of the history being replayed.
 
 EXIT STATUS
 -----------
@@ -37,6 +61,32 @@ the replay has conflicts, the exit status is 1.  If the replay is not
 able to complete (or start) due to some kind of error, the exit status
 is something other than 0 or 1.
 
+EXAMPLES
+--------
+
+To simply rebase `mybranch` onto `target`:
+
+------------
+$ git replay --onto target origin/main..mybranch
+update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH}
+------------
+
+When calling `git replay`, one does not need to specify a range of
+commits to replay using the syntax `A..B`; any range expression will
+do:
+
+------------
+$ git replay --onto origin/main ^base branch1 branch2 branch3
+update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH}
+update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH}
+update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH}
+------------
+
+This will simultaneously rebase `branch1`, `branch2`, and `branch3`,
+all commits they have since `base`, playing them on top of
+`origin/main`. These three branches may have commits on top of `base`
+that they have in common, but that does not need to be the case.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/replay.c b/builtin/replay.c
index 73a25e9e85..7a660020d1 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -14,7 +14,6 @@
 #include "parse-options.h"
 #include "refs.h"
 #include "revision.h"
-#include "strvec.h"
 #include <oidset.h>
 #include <tree.h>
 
@@ -118,16 +117,14 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct commit *onto;
 	const char *onto_name = NULL;
 	struct commit *last_commit = NULL;
-	struct strvec rev_walk_args = STRVEC_INIT;
 	struct rev_info revs;
 	struct commit *commit;
 	struct merge_options merge_opt;
 	struct merge_result result;
-	struct strbuf branch_name = STRBUF_INIT;
 	int ret = 0;
 
 	const char * const replay_usage[] = {
-		N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
+		N_("git replay --onto <newbase> <revision-range>... # EXPERIMENTAL"),
 		NULL
 	};
 	struct option replay_options[] = {
@@ -145,18 +142,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		usage_with_options(replay_usage, replay_options);
 	}
 
-	if (argc != 3) {
-		error(_("bad number of arguments"));
-		usage_with_options(replay_usage, replay_options);
-	}
-
 	onto = peel_committish(onto_name);
-	strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
 
 	repo_init_revisions(the_repository, &revs, prefix);
 
-	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
-
 	/*
 	 * Set desired values for rev walking options here. If they
 	 * are changed by some user specified option in setup_revisions()
@@ -171,8 +160,9 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	revs.topo_order = 1;
 	revs.simplify_history = 0;
 
-	if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
-		ret = error(_("unhandled options"));
+	argc = setup_revisions(argc, argv, &revs, NULL);
+	if (argc > 1) {
+		ret = error(_("unrecognized argument: %s"), argv[1]);
 		goto cleanup;
 	}
 
@@ -205,8 +195,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		revs.simplify_history = 0;
 	}
 
-	strvec_clear(&rev_walk_args);
-
 	if (prepare_revision_walk(&revs) < 0) {
 		ret = error(_("error preparing revisions"));
 		goto cleanup;
@@ -248,7 +236,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	ret = result.clean;
 
 cleanup:
-	strbuf_release(&branch_name);
 	release_revisions(&revs);
 
 	/* Return */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 3567c98362..a1da4f9ef9 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -52,7 +52,7 @@ test_expect_success 'setup bare' '
 '
 
 test_expect_success 'using replay to rebase two branches, one on top of other' '
-	git replay --onto main topic1 topic2 >result &&
+	git replay --onto main topic1..topic2 >result &&
 
 	test_line_count = 1 result &&
 
@@ -68,8 +68,16 @@ test_expect_success 'using replay to rebase two branches, one on top of other' '
 '
 
 test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
-	git -C bare replay --onto main topic1 topic2 >result-bare &&
+	git -C bare replay --onto main topic1..topic2 >result-bare &&
 	test_cmp expect result-bare
 '
 
+test_expect_success 'using replay to rebase with a conflict' '
+	test_expect_code 1 git replay --onto topic1 B..conflict
+'
+
+test_expect_success 'using replay on bare repo to rebase with a conflict' '
+	test_expect_code 1 git -C bare replay --onto topic1 B..conflict
+'
+
 test_done
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index 099aefeffc..0f39ed0d08 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -71,7 +71,7 @@ test_expect_success 'caching renames does not preclude finding new ones' '
 
 		git switch upstream &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -201,7 +201,7 @@ test_expect_success 'rename same file identically, then reintroduce it' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -279,7 +279,7 @@ test_expect_success 'rename same file identically, then add file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -357,7 +357,7 @@ test_expect_success 'cached dir rename does not prevent noticing later conflict'
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		test_must_fail git replay --onto HEAD upstream~1 topic >output &&
+		test_must_fail git replay --onto HEAD upstream~1..topic >output &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls
@@ -456,7 +456,7 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -523,7 +523,7 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -626,7 +626,7 @@ test_expect_success 'caching renames only on upstream side, part 1' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
@@ -685,7 +685,7 @@ test_expect_success 'caching renames only on upstream side, part 2' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic >out &&
+		git replay --onto HEAD upstream~1..topic >out &&
 		git update-ref --stdin <out &&
 		git checkout topic &&
 
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 10/14] replay: make it a minimal server side command
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

We want this command to be a minimal command that just does server side
picking of commits, displaying the results on stdout for higher level
scripts to consume.

So let's simplify it:
  * remove the worktree and index reading/writing,
  * remove the ref (and reflog) updating,
  * remove the assumptions tying us to HEAD, since (a) this is not a
    rebase and (b) we want to be able to pick commits in a bare repo,
    i.e. to/from branches that are not checked out and not the main
    branch,
  * remove unneeded includes,
  * handle rebasing multiple branches by printing on stdout the update
    ref commands that should be performed.

The output can be piped into `git update-ref --stdin` for the ref
updates to happen.

In the future to make it easier for users to use this command
directly maybe an option can be added to automatically pipe its output
into `git update-ref`.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-replay.txt             |  5 +-
 builtin/replay.c                         | 78 ++++++++----------------
 t/t3650-replay-basics.sh                 | 19 +++++-
 t/t6429-merge-sequence-rename-caching.sh | 39 +++++++-----
 4 files changed, 72 insertions(+), 69 deletions(-)

diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt
index 0349058b66..87a85a7f57 100644
--- a/Documentation/git-replay.txt
+++ b/Documentation/git-replay.txt
@@ -15,7 +15,10 @@ DESCRIPTION
 -----------
 
 Takes a range of commits, specified by <oldbase> and <branch>, and
-replays them onto a new location (see `--onto` option below).
+replays them onto a new location (see `--onto` option below). Leaves
+the working tree and the index untouched, and updates no references.
+The output of this command is meant to be used as input to
+`git update-ref --stdin`, which would update the relevant branches.
 
 THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
 
diff --git a/builtin/replay.c b/builtin/replay.c
index 30292d219d..73a25e9e85 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -6,11 +6,7 @@
 #include "git-compat-util.h"
 
 #include "builtin.h"
-#include "cache-tree.h"
-#include "commit.h"
 #include "environment.h"
-#include "gettext.h"
-#include "hash.h"
 #include "hex.h"
 #include "lockfile.h"
 #include "merge-ort.h"
@@ -18,8 +14,6 @@
 #include "parse-options.h"
 #include "refs.h"
 #include "revision.h"
-#include "sequencer.h"
-#include "setup.h"
 #include "strvec.h"
 #include <oidset.h>
 #include <tree.h>
@@ -102,6 +96,7 @@ static struct commit *pick_regular_commit(struct commit *pickme,
 	pickme_tree = repo_get_commit_tree(the_repository, pickme);
 	base_tree = repo_get_commit_tree(the_repository, base);
 
+	merge_opt->branch1 = short_commit_name(last_commit);
 	merge_opt->branch2 = short_commit_name(pickme);
 	merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
 
@@ -122,15 +117,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 {
 	struct commit *onto;
 	const char *onto_name = NULL;
-	struct commit *last_commit = NULL, *last_picked_commit = NULL;
-	struct lock_file lock = LOCK_INIT;
+	struct commit *last_commit = NULL;
 	struct strvec rev_walk_args = STRVEC_INIT;
 	struct rev_info revs;
 	struct commit *commit;
 	struct merge_options merge_opt;
-	struct tree *head_tree;
 	struct merge_result result;
-	struct strbuf reflog_msg = STRBUF_INIT;
 	struct strbuf branch_name = STRBUF_INIT;
 	int ret = 0;
 
@@ -161,10 +153,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	onto = peel_committish(onto_name);
 	strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
 
-	repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR);
-	if (repo_read_index(the_repository) < 0)
-		BUG("Could not read index");
-
 	repo_init_revisions(the_repository, &revs, prefix);
 
 	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
@@ -227,58 +215,44 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	init_merge_options(&merge_opt, the_repository);
 	memset(&result, 0, sizeof(result));
 	merge_opt.show_rename_progress = 0;
-	merge_opt.branch1 = "HEAD";
-	head_tree = repo_get_commit_tree(the_repository, onto);
-	result.tree = head_tree;
+	result.tree = repo_get_commit_tree(the_repository, onto);
 	last_commit = onto;
 	while ((commit = get_revision(&revs))) {
-		struct commit *pick;
+		const struct name_decoration *decoration;
 
 		if (!commit->parents)
 			die(_("replaying down to root commit is not supported yet!"));
 		if (commit->parents->next)
 			die(_("replaying merge commits is not supported yet!"));
 
-		pick = pick_regular_commit(commit, last_commit, &merge_opt, &result);
-		if (!pick)
+		last_commit = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+		if (!last_commit)
 			break;
-		last_commit = pick;
-		last_picked_commit = commit;
+
+		decoration = get_name_decoration(&commit->object);
+		if (!decoration)
+			continue;
+
+		while (decoration) {
+			if (decoration->type == DECORATION_REF_LOCAL) {
+				printf("update %s %s %s\n",
+				       decoration->name,
+				       oid_to_hex(&last_commit->object.oid),
+				       oid_to_hex(&commit->object.oid));
+			}
+			decoration = decoration->next;
+		}
 	}
 
 	merge_finalize(&merge_opt, &result);
+	ret = result.clean;
 
-	if (result.clean < 0)
-		exit(128);
-
-	if (result.clean) {
-		strbuf_addf(&reflog_msg, "finish rebase %s onto %s",
-			    oid_to_hex(&last_picked_commit->object.oid),
-			    oid_to_hex(&last_commit->object.oid));
-		if (update_ref(reflog_msg.buf, branch_name.buf,
-			       &last_commit->object.oid,
-			       &last_picked_commit->object.oid,
-			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
-			error(_("could not update %s"), argv[2]);
-			die("Failed to update %s", argv[2]);
-		}
-		if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
-			die(_("unable to update HEAD"));
-	} else {
-		strbuf_addf(&reflog_msg, "rebase progress up to %s",
-			    oid_to_hex(&last_picked_commit->object.oid));
-		if (update_ref(reflog_msg.buf, "HEAD",
-			       &last_commit->object.oid,
-			       &onto->object.oid,
-			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
-			error(_("could not update %s"), argv[2]);
-			die("Failed to update %s", argv[2]);
-		}
-	}
-	ret = (result.clean == 0);
 cleanup:
-	strbuf_release(&reflog_msg);
 	strbuf_release(&branch_name);
 	release_revisions(&revs);
-	return ret;
+
+	/* Return */
+	if (ret < 0)
+		exit(128);
+	return ret ? 0 : 1;
 }
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index b5b9f9ade2..3567c98362 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -47,12 +47,29 @@ test_expect_success 'setup' '
 	test_commit C.conflict C.t conflict
 '
 
+test_expect_success 'setup bare' '
+	git clone --bare . bare
+'
+
 test_expect_success 'using replay to rebase two branches, one on top of other' '
 	git replay --onto main topic1 topic2 >result &&
 
+	test_line_count = 1 result &&
+
 	git log --format=%s $(cut -f 3 -d " " result) >actual &&
 	test_write_lines E D M L B A >expect &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+
+	printf "update refs/heads/topic2 " >expect &&
+	printf "%s " $(cut -f 3 -d " " result) >>expect &&
+	git rev-parse topic2 >>expect &&
+
+	test_cmp expect result
+'
+
+test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' '
+	git -C bare replay --onto main topic1 topic2 >result-bare &&
+	test_cmp expect result-bare
 '
 
 test_done
diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh
index 7670b72008..099aefeffc 100755
--- a/t/t6429-merge-sequence-rename-caching.sh
+++ b/t/t6429-merge-sequence-rename-caching.sh
@@ -71,8 +71,9 @@ test_expect_success 'caching renames does not preclude finding new ones' '
 
 		git switch upstream &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		git ls-files >tracked-files &&
 		test_line_count = 2 tracked-files &&
@@ -140,7 +141,9 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 1 calls
@@ -198,8 +201,9 @@ test_expect_success 'rename same file identically, then reintroduce it' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		git ls-files >tracked &&
 		test_line_count = 2 tracked &&
@@ -275,8 +279,9 @@ test_expect_success 'rename same file identically, then add file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		git ls-files >tracked &&
 		test_line_count = 4 tracked &&
@@ -451,8 +456,9 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls &&
@@ -517,8 +523,9 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 3 calls &&
@@ -619,8 +626,9 @@ test_expect_success 'caching renames only on upstream side, part 1' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 1 calls &&
@@ -677,8 +685,9 @@ test_expect_success 'caching renames only on upstream side, part 2' '
 		GIT_TRACE2_PERF="$(pwd)/trace.output" &&
 		export GIT_TRACE2_PERF &&
 
-		git replay --onto HEAD upstream~1 topic &&
-		git reset --hard topic &&
+		git replay --onto HEAD upstream~1 topic >out &&
+		git update-ref --stdin <out &&
+		git checkout topic &&
 
 		grep region_enter.*diffcore_rename trace.output >calls &&
 		test_line_count = 2 calls &&
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 09/14] replay: remove HEAD related sanity check
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

We want replay to be a command that can be used on the server side on
any branch, not just the current one, so we are going to stop updating
HEAD in a future commit.

A "sanity check" that makes sure we are replaying the current branch
doesn't make sense anymore. Let's remove it.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c         | 8 +-------
 t/t3650-replay-basics.sh | 2 --
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 1035435705..30292d219d 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -123,7 +123,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct commit *onto;
 	const char *onto_name = NULL;
 	struct commit *last_commit = NULL, *last_picked_commit = NULL;
-	struct object_id head;
 	struct lock_file lock = LOCK_INIT;
 	struct strvec rev_walk_args = STRVEC_INIT;
 	struct rev_info revs;
@@ -162,11 +161,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	onto = peel_committish(onto_name);
 	strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
 
-	/* Sanity check */
-	if (repo_get_oid(the_repository, "HEAD", &head))
-		die(_("Cannot read HEAD"));
-	assert(oideq(&onto->object.oid, &head));
-
 	repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR);
 	if (repo_read_index(the_repository) < 0)
 		BUG("Could not read index");
@@ -275,7 +269,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			    oid_to_hex(&last_picked_commit->object.oid));
 		if (update_ref(reflog_msg.buf, "HEAD",
 			       &last_commit->object.oid,
-			       &head,
+			       &onto->object.oid,
 			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
 			error(_("could not update %s"), argv[2]);
 			die("Failed to update %s", argv[2]);
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 36c1b5082a..b5b9f9ade2 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -48,8 +48,6 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'using replay to rebase two branches, one on top of other' '
-	git switch main &&
-
 	git replay --onto main topic1 topic2 >result &&
 
 	git log --format=%s $(cut -f 3 -d " " result) >actual &&
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 08/14] replay: remove progress and info output
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

The replay command will be changed in a follow up commit, so that it
will not update refs directly, but instead it will print on stdout a
list of commands that can be consumed by `git update-ref --stdin`.

We don't want this output to be polluted by its current low value
output, so let's just remove the latter.

In the future, when the command gets an option to update refs by
itself, it will make a lot of sense to display a progress meter, but
we are not there yet.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 2e1df83027..1035435705 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -232,7 +232,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 
 	init_merge_options(&merge_opt, the_repository);
 	memset(&result, 0, sizeof(result));
-	merge_opt.show_rename_progress = 1;
+	merge_opt.show_rename_progress = 0;
 	merge_opt.branch1 = "HEAD";
 	head_tree = repo_get_commit_tree(the_repository, onto);
 	result.tree = head_tree;
@@ -240,9 +240,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	while ((commit = get_revision(&revs))) {
 		struct commit *pick;
 
-		fprintf(stderr, "Rebasing %s...\r",
-			oid_to_hex(&commit->object.oid));
-
 		if (!commit->parents)
 			die(_("replaying down to root commit is not supported yet!"));
 		if (commit->parents->next)
@@ -261,7 +258,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		exit(128);
 
 	if (result.clean) {
-		fprintf(stderr, "\nDone.\n");
 		strbuf_addf(&reflog_msg, "finish rebase %s onto %s",
 			    oid_to_hex(&last_picked_commit->object.oid),
 			    oid_to_hex(&last_commit->object.oid));
@@ -275,7 +271,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
 			die(_("unable to update HEAD"));
 	} else {
-		fprintf(stderr, "\nAborting: Hit a conflict.\n");
 		strbuf_addf(&reflog_msg, "rebase progress up to %s",
 			    oid_to_hex(&last_picked_commit->object.oid));
 		if (update_ref(reflog_msg.buf, "HEAD",
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 07/14] replay: add an important FIXME comment about gpg signing
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

We want to be able to handle signed commits in some way in the future,
but we are not ready to do it now. So for the time being let's just add
a FIXME comment to remind us about it.

These are different ways we could handle them:

  - in case of a cli user and if there was an interactive mode, we could
    perhaps ask if the user wants to sign again
  - we could add an option to just fail if there are signed commits

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 8302d35eca..2e1df83027 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -62,7 +62,7 @@ static struct commit *create_commit(struct tree *tree,
 	struct object *obj;
 	struct commit_list *parents = NULL;
 	char *author;
-	char *sign_commit = NULL;
+	char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
 	struct commit_extra_header *extra;
 	struct strbuf msg = STRBUF_INIT;
 	const char *out_enc = get_commit_output_encoding();
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 06/14] replay: change rev walking options
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Let's force the rev walking options we need after calling
setup_revisions() instead of before.

This might override some user supplied rev walking command line options
though. So let's detect that and warn users by:

  a) setting the desired values, before setup_revisions(),
  b) checking after setup_revisions() whether these values differ from
     the desired values,
  c) if so throwing a warning and setting the desired values again.

We want the command to work from older commits to newer ones by default.
Also we don't want history simplification, as we want to deal with all
the commits in the affected range.

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 5c4cbd11db..8302d35eca 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -173,22 +173,56 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 
 	repo_init_revisions(the_repository, &revs, prefix);
 
-	revs.verbose_header = 1;
-	revs.max_parents = 1;
-	revs.cherry_mark = 1;
-	revs.limited = 1;
+	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
+
+	/*
+	 * Set desired values for rev walking options here. If they
+	 * are changed by some user specified option in setup_revisions()
+	 * below, we will detect that below and then warn.
+	 *
+	 * TODO: In the future we might want to either die(), or allow
+	 * some options changing these values if we think they could
+	 * be useful.
+	 */
 	revs.reverse = 1;
-	revs.right_only = 1;
 	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
 	revs.topo_order = 1;
-
-	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
+	revs.simplify_history = 0;
 
 	if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
 		ret = error(_("unhandled options"));
 		goto cleanup;
 	}
 
+	/*
+	 * Detect and warn if we override some user specified rev
+	 * walking options.
+	 */
+	if (revs.reverse != 1) {
+		warning(_("some rev walking options will be overridden as "
+			  "'%s' bit in 'struct rev_info' will be forced"),
+			"reverse");
+		revs.reverse = 1;
+	}
+	if (revs.sort_order != REV_SORT_IN_GRAPH_ORDER) {
+		warning(_("some rev walking options will be overridden as "
+			  "'%s' bit in 'struct rev_info' will be forced"),
+			"sort_order");
+		revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+	}
+	if (revs.topo_order != 1) {
+		warning(_("some rev walking options will be overridden as "
+			  "'%s' bit in 'struct rev_info' will be forced"),
+			"topo_order");
+		revs.topo_order = 1;
+	}
+	if (revs.simplify_history != 0) {
+		warning(_("some rev walking options will be overridden as "
+			  "'%s' bit in 'struct rev_info' will be forced"),
+			"simplify_history");
+		revs.simplify_history = 0;
+	}
+
 	strvec_clear(&rev_walk_args);
 
 	if (prepare_revision_walk(&revs) < 0) {
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 05/14] replay: introduce pick_regular_commit()
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Let's refactor the code to handle a regular commit (a commit that is
neither a root commit nor a merge commit) into a single function instead
of keeping it inside cmd_replay().

This is good for separation of concerns, and this will help further work
in the future to replay merge commits.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 54 ++++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 32dbaaf028..5c4cbd11db 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -89,6 +89,35 @@ static struct commit *create_commit(struct tree *tree,
 	return (struct commit *)obj;
 }
 
+static struct commit *pick_regular_commit(struct commit *pickme,
+					  struct commit *last_commit,
+					  struct merge_options *merge_opt,
+					  struct merge_result *result)
+{
+	struct commit *base;
+	struct tree *pickme_tree, *base_tree;
+
+	base = pickme->parents->item;
+
+	pickme_tree = repo_get_commit_tree(the_repository, pickme);
+	base_tree = repo_get_commit_tree(the_repository, base);
+
+	merge_opt->branch2 = short_commit_name(pickme);
+	merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2);
+
+	merge_incore_nonrecursive(merge_opt,
+				  base_tree,
+				  result->tree,
+				  pickme_tree,
+				  result);
+
+	free((char*)merge_opt->ancestor);
+	merge_opt->ancestor = NULL;
+	if (!result->clean)
+		return NULL;
+	return create_commit(result->tree, pickme, last_commit);
+}
+
 int cmd_replay(int argc, const char **argv, const char *prefix)
 {
 	struct commit *onto;
@@ -100,7 +129,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct rev_info revs;
 	struct commit *commit;
 	struct merge_options merge_opt;
-	struct tree *next_tree, *base_tree, *head_tree;
+	struct tree *head_tree;
 	struct merge_result result;
 	struct strbuf reflog_msg = STRBUF_INIT;
 	struct strbuf branch_name = STRBUF_INIT;
@@ -175,7 +204,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	result.tree = head_tree;
 	last_commit = onto;
 	while ((commit = get_revision(&revs))) {
-		struct commit *base;
+		struct commit *pick;
 
 		fprintf(stderr, "Rebasing %s...\r",
 			oid_to_hex(&commit->object.oid));
@@ -185,26 +214,11 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		if (commit->parents->next)
 			die(_("replaying merge commits is not supported yet!"));
 
-		base = commit->parents->item;
-
-		next_tree = repo_get_commit_tree(the_repository, commit);
-		base_tree = repo_get_commit_tree(the_repository, base);
-
-		merge_opt.branch2 = short_commit_name(commit);
-		merge_opt.ancestor = xstrfmt("parent of %s", merge_opt.branch2);
-
-		merge_incore_nonrecursive(&merge_opt,
-					  base_tree,
-					  result.tree,
-					  next_tree,
-					  &result);
-
-		free((char*)merge_opt.ancestor);
-		merge_opt.ancestor = NULL;
-		if (!result.clean)
+		pick = pick_regular_commit(commit, last_commit, &merge_opt, &result);
+		if (!pick)
 			break;
+		last_commit = pick;
 		last_picked_commit = commit;
-		last_commit = create_commit(result.tree, commit, last_commit);
 	}
 
 	merge_finalize(&merge_opt, &result);
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 04/14] replay: die() instead of failing assert()
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

It's not a good idea for regular Git commands to use an assert() to
check for things that could happen but are not supported.

Let's die() with an explanation of the issue instead.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index afabb844d3..32dbaaf028 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -179,7 +179,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 
 		fprintf(stderr, "Rebasing %s...\r",
 			oid_to_hex(&commit->object.oid));
-		assert(commit->parents && !commit->parents->next);
+
+		if (!commit->parents)
+			die(_("replaying down to root commit is not supported yet!"));
+		if (commit->parents->next)
+			die(_("replaying merge commits is not supported yet!"));
+
 		base = commit->parents->item;
 
 		next_tree = repo_get_commit_tree(the_repository, commit);
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related

* [PATCH v7 03/14] replay: start using parse_options API
From: Christian Couder @ 2023-11-15 14:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Patrick Steinhardt, Johannes Schindelin,
	Elijah Newren, John Cai, Derrick Stolee, Phillip Wood, Calvin Wan,
	Toon Claes, Dragan Simic, Linus Arver, Christian Couder
In-Reply-To: <20231115143327.2441397-1-christian.couder@gmail.com>

From: Elijah Newren <newren@gmail.com>

Instead of manually parsing arguments, let's start using the parse_options
API. This way this new builtin will look more standard, and in some
upcoming commits will more easily be able to handle more command line
options.

Note that we plan to later use standard revision ranges instead of
hardcoded "<oldbase> <branch>" arguments. When we will use standard
revision ranges, it will be easier to check if there are no spurious
arguments if we keep ARGV[0], so let's call parse_options() with
PARSE_OPT_KEEP_ARGV0 even if we don't need ARGV[0] right now to avoid
some useless code churn.

Co-authored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/replay.c | 45 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index f2d8444417..afabb844d3 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -15,7 +15,7 @@
 #include "lockfile.h"
 #include "merge-ort.h"
 #include "object-name.h"
-#include "read-cache-ll.h"
+#include "parse-options.h"
 #include "refs.h"
 #include "revision.h"
 #include "sequencer.h"
@@ -92,6 +92,7 @@ static struct commit *create_commit(struct tree *tree,
 int cmd_replay(int argc, const char **argv, const char *prefix)
 {
 	struct commit *onto;
+	const char *onto_name = NULL;
 	struct commit *last_commit = NULL, *last_picked_commit = NULL;
 	struct object_id head;
 	struct lock_file lock = LOCK_INIT;
@@ -105,16 +106,32 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	struct strbuf branch_name = STRBUF_INIT;
 	int ret = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h")) {
-		printf("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL\n");
-		exit(129);
+	const char * const replay_usage[] = {
+		N_("git replay --onto <newbase> <oldbase> <branch> # EXPERIMENTAL"),
+		NULL
+	};
+	struct option replay_options[] = {
+		OPT_STRING(0, "onto", &onto_name,
+			   N_("revision"),
+			   N_("replay onto given commit")),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, prefix, replay_options, replay_usage,
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT);
+
+	if (!onto_name) {
+		error(_("option --onto is mandatory"));
+		usage_with_options(replay_usage, replay_options);
 	}
 
-	if (argc != 5 || strcmp(argv[1], "--onto"))
-		die("usage: read the code, figure out how to use it, then do so");
+	if (argc != 3) {
+		error(_("bad number of arguments"));
+		usage_with_options(replay_usage, replay_options);
+	}
 
-	onto = peel_committish(argv[2]);
-	strbuf_addf(&branch_name, "refs/heads/%s", argv[4]);
+	onto = peel_committish(onto_name);
+	strbuf_addf(&branch_name, "refs/heads/%s", argv[2]);
 
 	/* Sanity check */
 	if (repo_get_oid(the_repository, "HEAD", &head))
@@ -126,6 +143,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 		BUG("Could not read index");
 
 	repo_init_revisions(the_repository, &revs, prefix);
+
 	revs.verbose_header = 1;
 	revs.max_parents = 1;
 	revs.cherry_mark = 1;
@@ -134,7 +152,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 	revs.right_only = 1;
 	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
 	revs.topo_order = 1;
-	strvec_pushl(&rev_walk_args, "", argv[4], "--not", argv[3], NULL);
+
+	strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL);
 
 	if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) {
 		ret = error(_("unhandled options"));
@@ -197,8 +216,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			       &last_commit->object.oid,
 			       &last_picked_commit->object.oid,
 			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
-			error(_("could not update %s"), argv[4]);
-			die("Failed to update %s", argv[4]);
+			error(_("could not update %s"), argv[2]);
+			die("Failed to update %s", argv[2]);
 		}
 		if (create_symref("HEAD", branch_name.buf, reflog_msg.buf) < 0)
 			die(_("unable to update HEAD"));
@@ -210,8 +229,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
 			       &last_commit->object.oid,
 			       &head,
 			       REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR)) {
-			error(_("could not update %s"), argv[4]);
-			die("Failed to update %s", argv[4]);
+			error(_("could not update %s"), argv[2]);
+			die("Failed to update %s", argv[2]);
 		}
 	}
 	ret = (result.clean == 0);
-- 
2.43.0.rc1.15.g29556bcc86


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox