git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Derrick Stolee" <derrickstolee@github.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Elijah Newren" <newren@gmail.com>
Subject: [PATCH v5 00/10] rebase: fix several code/testing/documentation issues around flag incompatibilities
Date: Wed, 25 Jan 2023 04:03:44 +0000	[thread overview]
Message-ID: <pull.1466.v5.git.1674619434.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1466.v4.git.1674367961.gitgitgadget@gmail.com>

We had a report about --update-refs being ignored when --whitespace=fix was
passed, confusing an end user. These were already marked as incompatible in
the manual, but the code didn't check for the incompatibility and provide an
error to the user.

Folks brought up other flags tangentially when reviewing an earlier round of
this series, and I found we had more that were missing checks, and that we
were missing some testcases, and that the documentation was wrong on some of
the relevant options. So, I ended up getting lots of little fixes to
straighten these all out.

Changes since v4:

 * Pulled out the changes regarding incompatibility detection for
   --[no-]reapply-cherry-picks into a separate patch. Phillip helped a lot
   with understanding the behavior, suggesting changes, and getting the
   wording right, and I think it deserves its own patch with its own
   explanation.

Changes since v3:

 * Corrected the code surrounding --[no-]reapply-cherry-picks, and extended
   the testcases (Thanks to Phillip for pointing out my error)
 * I went ahead and implemented the better error message when the merge
   backend is triggered solely via config options.

Changes since v2:

 * Remove the extra patch and reword the comment in t3422 more thoroughly.
 * Add tests for other flag incompatibilities that were missing
 * Add code that was missing for checking various flag incompatibilities
 * Fix incorrect claims in the documentation around incompatible options
 * A few other miscellaneous fixups noticed while doing all the above

Changes since v1:

 * Add a patch nuking the -C option to rebase (fixes confusion around the
   comment in t3422 and acknowledges the fact that the option is totally and
   utterly useless and always has been. It literally never affects the
   results of a rebase.)

Signed-off-by: Elijah Newren newren@gmail.com

Elijah Newren (10):
  rebase: mark --update-refs as requiring the merge backend
  rebase: flag --apply and --merge as incompatible
  rebase: remove --allow-empty-message from incompatible opts
  rebase: fix docs about incompatibilities with --root
  rebase: fix incompatiblity checks for --[no-]reapply-cherry-picks
  rebase: add coverage of other incompatible options
  rebase: clarify the OPT_CMDMODE incompatibilities
  rebase: fix formatting of rebase --reapply-cherry-picks option in docs
  rebase: put rebase_options initialization in single place
  rebase: provide better error message for apply options vs. merge
    config

 Documentation/git-rebase.txt           | 77 ++++++++++++-------------
 builtin/rebase.c                       | 79 +++++++++++++++++++-------
 t/t3422-rebase-incompatible-options.sh | 71 +++++++++++++++++++++--
 3 files changed, 163 insertions(+), 64 deletions(-)


base-commit: 56c8fb1e95377900ec9d53c07886022af0a5d3c2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1466%2Fnewren%2Frebase-update-refs-imply-merge-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1466/newren/rebase-update-refs-imply-merge-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1466

Range-diff vs v4:

  1:  8a676e6ec1a =  1:  8a676e6ec1a rebase: mark --update-refs as requiring the merge backend
  2:  cc129b87185 =  2:  cc129b87185 rebase: flag --apply and --merge as incompatible
  3:  9464bbbe9ba =  3:  9464bbbe9ba rebase: remove --allow-empty-message from incompatible opts
  4:  b702f15ed7c =  4:  b702f15ed7c rebase: fix docs about incompatibilities with --root
  -:  ----------- >  5:  3a8429f3d2b rebase: fix incompatiblity checks for --[no-]reapply-cherry-picks
  5:  5e4851e611e !  6:  2777dd2788a rebase: add coverage of other incompatible options
     @@ Commit message
      
          The git-rebase manual noted several sets of incompatible options, but
          we were missing tests for a few of these.  Further, we were missing
     -    code checks for some of these, which could result in command line
     +    code checks for one of these, which could result in command line
          options being silently ignored.
      
          Also, note that adding a check for autosquash means that using
     @@ Commit message
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     - ## Documentation/git-rebase.txt ##
     -@@ Documentation/git-rebase.txt: are incompatible with the following options:
     -  * --exec
     -  * --no-keep-empty
     -  * --empty=
     -- * --reapply-cherry-picks
     -+ * --[no-]reapply-cherry-picks
     -  * --edit-todo
     -  * --update-refs
     -  * --root when used without --onto
     -
       ## builtin/rebase.c ##
     -@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
     - 		if (options.fork_point < 0)
     - 			options.fork_point = 0;
     - 	}
     -+	/*
     -+	 * The apply backend does not support --[no-]reapply-cherry-picks.
     -+	 * The behavior it implements by default is equivalent to
     -+	 * --no-reapply-cherry-picks (due to passing --cherry-picks to
     -+	 * format-patch), but --keep-base alters the upstream such that no
     -+	 * cherry-picks can be found (effectively making it act like
     -+	 * --reapply-cherry-picks).
     -+	 *
     -+	 * Now, if the user does specify --[no-]reapply-cherry-picks, but
     -+	 * does so in such a way that options.reapply_cherry_picks ==
     -+	 * keep_base, then the behavior they get will match what they
     -+	 * expect despite options.reapply_cherry_picks being ignored.  We
     -+	 * could just allow the flag in that case, but it seems better to
     -+	 * just alert the user that they've specified a flag that the
     -+	 * backend ignores.
     -+	 */
     -+	if (options.reapply_cherry_picks >= 0)
     -+		imply_merge(&options, options.reapply_cherry_picks ? "--reapply-cherry-picks" :
     -+								     "--no-reapply-cherry-picks");
     -+
     - 	/*
     - 	 * --keep-base defaults to --reapply-cherry-picks to avoid losing
     - 	 * commits when using this option.
     -@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
     - 	if (options.empty != EMPTY_UNSPECIFIED)
     - 		imply_merge(&options, "--empty");
     - 
     --	/*
     --	 * --keep-base implements --reapply-cherry-picks by altering upstream so
     --	 * it works with both backends.
     --	 */
     --	if (options.reapply_cherry_picks && !keep_base)
     --		imply_merge(&options, "--reapply-cherry-picks");
     --
     - 	if (gpg_sign)
     - 		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
     - 
      @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix)
       	if (options.update_refs)
       		imply_merge(&options, "--update-refs");
     @@ t/t3422-rebase-incompatible-options.sh: test_rebase_am_only () {
      +		test_must_fail git rebase $opt --empty=ask A
      +	"
      +
     -+	test_expect_success "$opt incompatible with --no-reapply-cherry-picks" "
     -+		git checkout B^0 &&
     -+		test_must_fail git rebase $opt --no-reapply-cherry-picks A
     -+	"
     -+
     -+	test_expect_success "$opt incompatible with --reapply-cherry-picks" "
     -+		git checkout B^0 &&
     -+		test_must_fail git rebase $opt --reapply-cherry-picks A
     -+	"
     -+
     - 	test_expect_success "$opt incompatible with --update-refs" "
     + 	test_expect_success "$opt incompatible with --no-reapply-cherry-picks" "
       		git checkout B^0 &&
     - 		test_must_fail git rebase $opt --update-refs A
     + 		test_must_fail git rebase $opt --no-reapply-cherry-picks A
  6:  21ae9e05313 !  7:  0d0541ea243 rebase: clarify the OPT_CMDMODE incompatibilities
     @@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below.
      @@ Documentation/git-rebase.txt: are incompatible with the following options:
        * --no-keep-empty
        * --empty=
     -  * --[no-]reapply-cherry-picks
     +  * --[no-]reapply-cherry-picks when used without --keep-base
      - * --edit-todo
        * --update-refs
        * --root when used without --onto
  7:  74a216bf0c0 =  8:  01808cf84a8 rebase: fix formatting of rebase --reapply-cherry-picks option in docs
  8:  a8adf7fda61 =  9:  f646abee524 rebase: put rebase_options initialization in single place
  9:  5cb00e5103b = 10:  328f5a1d534 rebase: provide better error message for apply options vs. merge config

-- 
gitgitgadget

  parent reply	other threads:[~2023-01-25  4:04 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-19  5:36 [PATCH] rebase: mark --update-refs as requiring the merge backend Elijah Newren via GitGitGadget
2023-01-19 21:47 ` Derrick Stolee
2023-01-20  1:54   ` Elijah Newren
2023-01-20 15:27   ` Junio C Hamano
2023-01-20 16:47     ` Elijah Newren
2023-01-20  4:56 ` [PATCH v2 0/2] " Elijah Newren via GitGitGadget
2023-01-20  4:56   ` [PATCH v2 1/2] rebase: remove completely useless -C option Elijah Newren via GitGitGadget
2023-01-20  5:40     ` Eric Sunshine
2023-01-20  6:42       ` Elijah Newren
2023-01-20  9:55     ` Martin Ågren
2023-01-20 15:32       ` Elijah Newren
2023-01-20 12:05     ` Junio C Hamano
2023-01-20 15:31       ` Elijah Newren
2023-01-20 16:15         ` Junio C Hamano
2023-01-21  4:52           ` Elijah Newren
2023-01-22  0:02             ` Junio C Hamano
2023-01-20  4:56   ` [PATCH v2 2/2] rebase: mark --update-refs as requiring the merge backend Elijah Newren via GitGitGadget
2023-01-20 16:46     ` Phillip Wood
2023-01-21  1:34       ` Elijah Newren
2023-01-21  1:55   ` [PATCH v3 0/7] rebase: fix several code/testing/documentation issues around flag incompatibilities Elijah Newren via GitGitGadget
2023-01-21  1:55     ` [PATCH v3 1/7] rebase: mark --update-refs as requiring the merge backend Elijah Newren via GitGitGadget
2023-01-21  1:55     ` [PATCH v3 2/7] rebase: flag --apply and --merge as incompatible Elijah Newren via GitGitGadget
2023-01-21  1:55     ` [PATCH v3 3/7] rebase: remove --allow-empty-message from incompatible opts Elijah Newren via GitGitGadget
2023-01-21 15:09       ` Phillip Wood
2023-01-21  1:55     ` [PATCH v3 4/7] rebase: fix docs about incompatibilities with --root Elijah Newren via GitGitGadget
2023-01-21  1:55     ` [PATCH v3 5/7] rebase: add coverage of other incompatible options Elijah Newren via GitGitGadget
2023-01-21 15:20       ` Phillip Wood
2023-01-21 19:25         ` Phillip Wood
2023-01-22  5:11           ` Elijah Newren
2023-01-21  1:55     ` [PATCH v3 6/7] rebase: clarify the OPT_CMDMODE incompatibilities Elijah Newren via GitGitGadget
2023-01-21  1:55     ` [PATCH v3 7/7] rebase: fix formatting of rebase --reapply-cherry-picks option in docs Elijah Newren via GitGitGadget
2023-01-21 15:21       ` Phillip Wood
2023-01-22  6:12     ` [PATCH v4 0/9] rebase: fix several code/testing/documentation issues around flag incompatibilities Elijah Newren via GitGitGadget
2023-01-22  6:12       ` [PATCH v4 1/9] rebase: mark --update-refs as requiring the merge backend Elijah Newren via GitGitGadget
2023-01-22  6:12       ` [PATCH v4 2/9] rebase: flag --apply and --merge as incompatible Elijah Newren via GitGitGadget
2023-01-22  6:12       ` [PATCH v4 3/9] rebase: remove --allow-empty-message from incompatible opts Elijah Newren via GitGitGadget
2023-01-22  6:12       ` [PATCH v4 4/9] rebase: fix docs about incompatibilities with --root Elijah Newren via GitGitGadget
2023-01-22  6:12       ` [PATCH v4 5/9] rebase: add coverage of other incompatible options Elijah Newren via GitGitGadget
2023-01-23 20:08         ` Phillip Wood
2023-01-24  2:36           ` Elijah Newren
2023-01-24 10:27             ` Phillip Wood
2023-01-24 13:16               ` Phillip Wood
2023-01-24 14:48                 ` Junio C Hamano
2023-01-24 15:41                 ` Elijah Newren
2023-01-24 16:48                   ` Phillip Wood
2023-01-24 17:12                     ` Elijah Newren
2023-01-24 19:21                       ` Phillip Wood
2023-01-22  6:12       ` [PATCH v4 6/9] rebase: clarify the OPT_CMDMODE incompatibilities Elijah Newren via GitGitGadget
2023-01-22  6:12       ` [PATCH v4 7/9] rebase: fix formatting of rebase --reapply-cherry-picks option in docs Elijah Newren via GitGitGadget
2023-01-22  6:12       ` [PATCH v4 8/9] rebase: put rebase_options initialization in single place Elijah Newren via GitGitGadget
2023-01-22  6:12       ` [PATCH v4 9/9] rebase: provide better error message for apply options vs. merge config Elijah Newren via GitGitGadget
2023-01-23 15:56       ` [PATCH v4 0/9] rebase: fix several code/testing/documentation issues around flag incompatibilities Derrick Stolee
2023-01-24  2:05         ` Elijah Newren
2023-01-25  4:03       ` Elijah Newren via GitGitGadget [this message]
2023-01-25  4:03         ` [PATCH v5 01/10] rebase: mark --update-refs as requiring the merge backend Elijah Newren via GitGitGadget
2023-01-25  4:03         ` [PATCH v5 02/10] rebase: flag --apply and --merge as incompatible Elijah Newren via GitGitGadget
2023-01-25  4:03         ` [PATCH v5 03/10] rebase: remove --allow-empty-message from incompatible opts Elijah Newren via GitGitGadget
2023-01-25  4:03         ` [PATCH v5 04/10] rebase: fix docs about incompatibilities with --root Elijah Newren via GitGitGadget
2023-01-25  4:03         ` [PATCH v5 05/10] rebase: fix incompatiblity checks for --[no-]reapply-cherry-picks Elijah Newren via GitGitGadget
2023-01-25 14:14           ` Phillip Wood
2023-01-25  4:03         ` [PATCH v5 06/10] rebase: add coverage of other incompatible options Elijah Newren via GitGitGadget
2023-01-25  4:03         ` [PATCH v5 07/10] rebase: clarify the OPT_CMDMODE incompatibilities Elijah Newren via GitGitGadget
2023-01-25  4:03         ` [PATCH v5 08/10] rebase: fix formatting of rebase --reapply-cherry-picks option in docs Elijah Newren via GitGitGadget
2023-01-25  4:03         ` [PATCH v5 09/10] rebase: put rebase_options initialization in single place Elijah Newren via GitGitGadget
2023-01-25  4:03         ` [PATCH v5 10/10] rebase: provide better error message for apply options vs. merge config Elijah Newren via GitGitGadget
2023-01-25 14:17         ` [PATCH v5 00/10] rebase: fix several code/testing/documentation issues around flag incompatibilities Phillip Wood
2023-01-25 16:39         ` Junio C Hamano
2023-01-25 16:48           ` Elijah Newren
2023-02-02 10:29         ` rebase --merge vs --whitespace=fix, was " Johannes Schindelin
2023-02-02 23:48           ` Elijah Newren

Reply instructions:

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

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

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=pull.1466.v5.git.1674619434.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=martin.agren@gmail.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).