Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions Documentation/RelNotes/2.56.0.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -867,12 +867,6 @@ Fixes since v2.55
Additionally, an obsolete variable used for retired Azure Pipelines
environments has been removed.

* Teach 'am', 'revert', and 'rebase' that running 'commit --amend' or a
partial 'commit <paths>' makes no sense during operations that stop
and return control to the user to resolve conflicts left in the
working tree, just like 'cherry-pick' and 'merge' do.
(merge cc499d40e5 en/no-amend-during-conflicts later to maint).

* The error path in 'git submodule--helper' has been updated to plug a
memory leak when a repository handle could not be obtained,
leveraging an updated idempotent repo_clear().
Expand Down
65 changes: 19 additions & 46 deletions builtin/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -515,25 +515,13 @@ static const char *prepare_index(const char **argv, const char *prefix,
*/
commit_style = COMMIT_PARTIAL;

switch (sequencer_ongoing_operation(the_repository, whence)) {
case ONGOING_NONE:
break;
case ONGOING_MERGE:
die(_("cannot do a partial commit during a merge."));
case ONGOING_CHERRY_PICK:
die(_("cannot do a partial commit during a cherry-pick."));
case ONGOING_REBASE_NOW_EMPTY:
/*
* A pick that became empty is not a conflict, and creating
* a new commit (partial or not) poses no problem.
*/
break;
case ONGOING_REVERT:
die(_("cannot do a partial commit during a revert."));
case ONGOING_AM:
die(_("cannot do a partial commit during an am session."));
case ONGOING_REBASE_CONFLICT:
die(_("cannot do a partial commit while resolving conflicts during a rebase."));
if (whence != FROM_COMMIT) {
if (whence == FROM_MERGE)
die(_("cannot do a partial commit during a merge."));
else if (is_from_cherry_pick(whence))
die(_("cannot do a partial commit during a cherry-pick."));
else if (is_from_rebase(whence))
die(_("cannot do a partial commit during a rebase."));
}

if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec))
Expand Down Expand Up @@ -905,7 +893,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
*/
else if (whence == FROM_MERGE)
hook_arg1 = "merge";
else if (is_from_cherry_pick(whence) || is_from_rebase_now_empty(whence)) {
else if (is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) {
hook_arg1 = "commit";
hook_arg2 = "CHERRY_PICK_HEAD";
}
Expand Down Expand Up @@ -1098,7 +1086,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (amend)
fputs(_(empty_amend_advice), stderr);
else if (is_from_cherry_pick(whence) ||
is_from_rebase_now_empty(whence)) {
whence == FROM_REBASE_PICK) {
fputs(_(empty_cherry_pick_advice), stderr);
if (whence == FROM_CHERRY_PICK_SINGLE)
fputs(_(empty_cherry_pick_advice_single), stderr);
Expand Down Expand Up @@ -1338,30 +1326,15 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_editor = 0;

/* Sanity check options */
if (amend) {
if (!current_head)
die(_("You have nothing to amend."));
/*
* Refuse to amend in the middle of any operation that is
* meant to record its result as a new commit on top of HEAD
* rather than by rewriting HEAD.
*/
switch (sequencer_ongoing_operation(s->repo, whence)) {
case ONGOING_NONE:
break;
case ONGOING_MERGE:
if (amend && !current_head)
die(_("You have nothing to amend."));
if (amend && whence != FROM_COMMIT) {
if (whence == FROM_MERGE)
die(_("You are in the middle of a merge -- cannot amend."));
case ONGOING_CHERRY_PICK:
else if (is_from_cherry_pick(whence))
die(_("You are in the middle of a cherry-pick -- cannot amend."));
case ONGOING_REBASE_NOW_EMPTY:
die(_("The now-empty commit has been dropped -- cannot amend."));
case ONGOING_REVERT:
die(_("You are in the middle of a revert -- cannot amend."));
case ONGOING_AM:
die(_("You are in the middle of an am session -- cannot amend."));
case ONGOING_REBASE_CONFLICT:
die(_("You are resolving conflicts during a rebase -- cannot amend."));
}
else if (whence == FROM_REBASE_PICK)
die(_("You are in the middle of a rebase -- cannot amend."));
}
if (fixup_message && squash_message)
die(_("options '%s' and '%s' cannot be used together"), "--squash", "--fixup");
Expand All @@ -1380,7 +1353,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (amend && !use_message && !fixup_message)
use_message = "HEAD";
if (!use_message && !is_from_cherry_pick(whence) &&
!is_from_rebase_now_empty(whence) && renew_authorship)
!is_from_rebase(whence) && renew_authorship)
die(_("--reset-author can be used only with -C, -c or --amend."));
if (use_message) {
use_message_buffer = read_commit_message(use_message);
Expand All @@ -1389,7 +1362,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
author_message_buffer = use_message_buffer;
}
}
if ((is_from_cherry_pick(whence) || is_from_rebase_now_empty(whence)) &&
if ((is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) &&
!renew_authorship) {
author_message = "CHERRY_PICK_HEAD";
author_message_buffer = read_commit_message(author_message);
Expand Down Expand Up @@ -1914,7 +1887,7 @@ int cmd_commit(int argc,
if (!reflog_msg)
reflog_msg = is_from_cherry_pick(whence)
? "commit (cherry-pick)"
: is_from_rebase_now_empty(whence)
: is_from_rebase(whence)
? "commit (rebase)"
: "commit";
commit_list_insert(current_head, &parents);
Expand Down
59 changes: 1 addition & 58 deletions sequencer.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,6 @@ static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
* command is processed, this file is deleted.
*/
static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
/*
* The apply ("am") backend keeps its state in the rebase-apply directory;
* the "applying" file within it marks a plain `git am` (as opposed to an
* apply-based rebase).
*/
static GIT_PATH_FUNC(apply_dir, "rebase-apply")
static GIT_PATH_FUNC(apply_path_applying, "rebase-apply/applying")
/*
* When we stop at a given patch via the "edit" command, this file contains
* the commit object name of the corresponding patch.
Expand Down Expand Up @@ -6994,7 +6987,7 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
!repo_get_oid(r, "REBASE_HEAD", &rebase_head) &&
!repo_get_oid(r, "CHERRY_PICK_HEAD", &cherry_pick_head) &&
oideq(&rebase_head, &cherry_pick_head))
*whence = FROM_REBASE_NOW_EMPTY;
*whence = FROM_REBASE_PICK;
else
*whence = FROM_CHERRY_PICK_SINGLE;

Expand All @@ -7004,56 +6997,6 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
return 0;
}

enum ongoing_operation sequencer_ongoing_operation(struct repository *r,
enum commit_whence whence)
{
/*
* The merge, cherry-pick, and (empty) rebase-pick stops are already
* distinguished by 'whence'.
*/
switch (whence) {
case FROM_MERGE:
return ONGOING_MERGE;
case FROM_CHERRY_PICK_SINGLE:
case FROM_CHERRY_PICK_MULTI:
return ONGOING_CHERRY_PICK;
case FROM_REBASE_NOW_EMPTY:
return ONGOING_REBASE_NOW_EMPTY;
case FROM_COMMIT:
break;
}

/*
* 'whence' is FROM_COMMIT, but we may still be in the middle of an
* operation that records its result on top of HEAD; detect those
* from their on-disk state.
*/

/* In the middle of a revert? */
if (refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD"))
return ONGOING_REVERT;

/* In the middle of an `am`? */
if (file_exists(apply_path_applying()))
return ONGOING_AM;

/*
* In the middle of a rebase that stopped for conflict resolution?
* The apply backend only ever stops for conflicts, so the presence
* of its state directory is enough. The merge backend writes
* stopped-sha whenever it hands control back to the user, but omits
* `amend` unless it stopped with HEAD already pointing at the commit
* to be amended (a clean edit/reword stop); its absence therefore
* marks a conflicted stop.
*/
if (file_exists(apply_dir()) ||
(file_exists(rebase_path_stopped_sha()) &&
!file_exists(rebase_path_amend())))
return ONGOING_REBASE_CONFLICT;

return ONGOING_NONE;
}

int sequencer_get_update_refs_state(const char *wt_dir,
struct string_list *refs)
{
Expand Down
24 changes: 0 additions & 24 deletions sequencer.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,30 +282,6 @@ int sequencer_get_last_command(struct repository* r,
enum replay_action *action);
int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);

/*
* An in-progress operation that records its result (often a conflict
* resolution) as a new commit on top of HEAD. Some ways of invoking
* "git commit" -- amending HEAD, or a partial commit -- are almost
* always a mistake during such an operation.
*/
enum ongoing_operation {
ONGOING_NONE = 0,
ONGOING_MERGE,
ONGOING_CHERRY_PICK,
ONGOING_REBASE_NOW_EMPTY,
ONGOING_REVERT,
ONGOING_AM,
ONGOING_REBASE_CONFLICT
};

/*
* Return which in-progress operation, if any, is underway; see enum
* ongoing_operation. 'whence' is the origin already computed for the
* pending commit.
*/
enum ongoing_operation sequencer_ongoing_operation(struct repository *r,
enum commit_whence whence);

/**
* Append the set of ref-OID pairs that are currently stored for the 'git
* rebase --update-refs' feature if such a rebase is currently happening.
Expand Down
128 changes: 4 additions & 124 deletions t/t3404-rebase-interactive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1858,7 +1858,7 @@ test_expect_success 'post-commit hook is called' '
test_cmp expect actual
'

test_expect_success 'partial commit is allowed when a rebase pick becomes empty' '
test_expect_success 'correct error message for partial commit after empty pick' '
test_when_finished "git rebase --abort" &&
(
set_fake_editor &&
Expand All @@ -1867,7 +1867,8 @@ test_expect_success 'partial commit is allowed when a rebase pick becomes empty'
test_must_fail git rebase -i A D
) &&
echo x >file1 &&
git commit file1
test_must_fail git commit file1 2>err &&
test_grep "cannot do a partial commit during a rebase." err
'

test_expect_success 'correct error message for commit --amend after empty pick' '
Expand All @@ -1880,128 +1881,7 @@ test_expect_success 'correct error message for commit --amend after empty pick'
) &&
echo x>file1 &&
test_must_fail git commit -a --amend 2>err &&
test_grep "now-empty commit has been dropped -- cannot amend." err
'

test_expect_success 'commit --amend is refused at a rebase conflict stop' '
test_when_finished "git rebase --abort" &&
git checkout --detach conflict-branch &&
(
set_fake_editor &&
FAKE_LINES="1 3" &&
export FAKE_LINES &&
test_must_fail git rebase -i A
) &&
test_path_is_file .git/rebase-merge/patch &&
test_path_is_missing .git/rebase-merge/amend &&
echo resolved >conflict &&
git add conflict &&
test_must_fail git commit --amend --no-edit 2>err &&
test_grep "You are resolving conflicts during a rebase -- cannot amend" err
'

test_expect_success 'commit --amend is refused when an "edit" pick conflicts' '
test_when_finished "git rebase --abort" &&
git checkout --detach conflict-branch &&
(
set_fake_editor &&
FAKE_LINES="1 edit 3" &&
export FAKE_LINES &&
test_must_fail git rebase -i A
) &&
test_path_is_file .git/rebase-merge/patch &&
test_path_is_missing .git/rebase-merge/amend &&
echo resolved >conflict &&
git add conflict &&
test_must_fail git commit --amend --no-edit 2>err &&
test_grep "You are resolving conflicts during a rebase -- cannot amend" err
'

test_expect_success 'commit --amend is allowed at a rebase edit stop' '
test_when_finished "git rebase --abort" &&
git checkout --detach no-conflict-branch &&
(
set_fake_editor &&
FAKE_LINES="edit 1 2 3 4" &&
export FAKE_LINES &&
git rebase -i A
) &&
test_path_is_file .git/rebase-merge/amend &&
echo tweak >fileJ &&
git add fileJ &&
git commit --amend --no-edit
'

test_expect_success 'commit --amend is allowed at a rebase break stop' '
test_when_finished "git rebase --abort" &&
git checkout --detach no-conflict-branch &&
(
set_fake_editor &&
FAKE_LINES="break 1 2 3 4" &&
export FAKE_LINES &&
git rebase -i A
) &&
test_must_fail git rev-parse --verify REBASE_HEAD &&
echo tweak >fileJ &&
git add fileJ &&
git commit --amend --no-edit
'

test_expect_success 'commit --amend is refused at an apply-backend conflict stop' '
test_when_finished "rm -rf apply-backend" &&
test_create_repo apply-backend &&
(
cd apply-backend &&
test_commit base file &&
git branch -M mainline &&
test_commit upstream file upstream &&
git checkout -b side mainline~1 &&
test_commit conflicting file side &&
test_commit unrelated other &&
test_must_fail git rebase --apply mainline &&
# the apply backend only ever stops for conflicts, and
# leaves HEAD on the previously-applied commit
test_path_is_dir .git/rebase-apply &&
test_path_is_missing .git/rebase-apply/applying &&
echo resolved >file &&
git add file &&
test_must_fail git commit --amend --no-edit 2>err &&
test_grep "You are resolving conflicts during a rebase -- cannot amend" err
)
'

test_expect_success 'partial commit is refused at a rebase conflict stop' '
test_when_finished "git rebase --abort" &&
git checkout --detach conflict-branch &&
(
set_fake_editor &&
FAKE_LINES="1 3" &&
export FAKE_LINES &&
test_must_fail git rebase -i A
) &&
echo resolved >conflict &&
git add conflict &&
test_must_fail git commit conflict 2>err &&
test_grep "cannot do a partial commit while resolving conflicts during a rebase." err
'

test_expect_success 'partial commit is refused at an apply-backend conflict stop' '
test_when_finished "rm -rf apply-backend" &&
test_create_repo apply-backend &&
(
cd apply-backend &&
test_commit base file &&
git branch -M mainline &&
test_commit upstream file upstream &&
git checkout -b side mainline~1 &&
test_commit conflicting file side &&
test_commit unrelated other &&
test_must_fail git rebase --apply mainline &&
echo resolved >file &&
git add file &&
test_must_fail git commit file 2>err &&
test_grep "cannot do a partial commit while resolving conflicts during a rebase." err
)
test_grep "middle of a rebase -- cannot amend." err
'

test_expect_success 'todo has correct onto hash' '
Expand Down
Loading