3

I've implemented automated merging script in Python. It should merge branches automatically and create pull requests if there are some merging conflicts.

Part of my script looks like:

from subprocess import check_call, CalledProcessError

# E.g. destination_branch = "master", source_branch = "release"
try:
    check_call(['git', 'checkout', '%s' % destination_branch])
    check_call(['git', 'merge', '--no-ff', '%s' % source_branch])
except CalledProcessError:
    # Creating pull request.

It looks like everything is good, but there are some issues here. After some automated merges I've got following errors: error: you need to resolve your current index first Dockerfile: needs merge. Also I'm printing status code of these two step. Status code is 1, which is not good.

As a result, I can see too many pull requests, most of them do not have any merging conflicts.

What is wrong here?

UPDATE:

Before merging something I also have a stuff for branch updating (to keep up-to-date version). It looks like:

try:
    check_call(['git', 'checkout', str(source_branch)])
    # branch successfully checked out
    check_call(['git', 'pull', 'origin', str(source_branch)])
except CalledProcessError:
    # Logging an errors.

One important thing to add:

As guys: @torek, @MarkAdelsberger mentioned in their comments, I've tried also their solution with adding git merge --abort command after failed merging for some reason. So, it doesn't help. It fails with another error: check_call([GIT_CMD, 'merge', '--abort']) File "/usr/lib/python2.7/subprocess.py", line 540, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['git', 'merge', '--abort']' returned non-zero exit status 128

So, I'm looking for some solution again....

Ideas?

smart
  • 1,975
  • 5
  • 26
  • 46
  • Does running these commands directly work? – Bryan K May 04 '17 at 20:02
  • @BryanK, to be honest, I've merged created pull requests without trying to do this manually. Now I'm waiting for some additional commits to make it possible to double check this. In general, these commands should work. – smart May 04 '17 at 20:07
  • I'm not a python guy so could be missing something; so I'll post my advice as a comment rather than answer at least for now... but this looks to me very much like a merge encountered conflicts and is awaiting manual resolution. If your intent is to back out the auto-merge attempt, then you need to issue `git merge --abort` – Mark Adelsberger May 04 '17 at 20:28
  • @MarkAdelsberger, as I mentioned above, it's Ok if there are some merging conflicts. I expect to have created pull requests for these cases.But there are also a lot of pull requests without merging conflicts, and it's unexpected for me... – smart May 04 '17 at 20:33
  • I suspect something broke somewhere else in the code. It looks like you probably have a broken repo state before calling `git checkout`, but either it's not throwing an error as it breaks or you're missing the check for it. – Bryan K May 04 '17 at 20:35
  • Well and good, but the error message and error codes you presented suggest that a merge (maybe a previous merge) was left in a "conflicts unresolved" state. – Mark Adelsberger May 04 '17 at 20:35
  • So what I'm saying is, when you create the PR, are you *also* issuing a `git merge --abort`? – Mark Adelsberger May 04 '17 at 20:36
  • @MarkAdelsberger, no, I'm not doing `git merge --abort` doing pull request – smart May 04 '17 at 20:37
  • Then as I said above, your repo is being left in a merging state and this is probably causing subsequent commands to fail – Mark Adelsberger May 04 '17 at 20:38
  • @BryanK, before merging I have something like this: `git checkout ;git pull origin ` – smart May 04 '17 at 20:39
  • `git pull` is just `git fetch` followed by `git merge`. This may leave you in a half-merged state to start with. – torek May 04 '17 at 20:41
  • If `merge --abort` returns 128, it probably means there was *not* a merge in progress. You need to abort *if and only if* a pending merge is in progress (or if you do it when no merge is in progress, you need to expect and handle the resulting 128 code). I'm still rather confident that `--abort` when it is needed is *exactly* what will help – Mark Adelsberger May 05 '17 at 17:52
  • @MarkAdelsberger, hm...but as I mentioned above, I'm calling `git merge --abort` only in exception section, when try block with `merge` or `pull` fails.. – smart May 05 '17 at 20:53
  • Is it possible that something other than a merge conflict would cause one of those commands to fail? Also keep in mind that while the steps @torek and I are talking about need to be looked at, this doesn't rule out possibly *other* errors existing as well, so if something else is putting you in an unexpected state then that will still have to be tracked down. The general take-away is that your script must be aware of (or able to determine) the current state of the git repo at any given point – Mark Adelsberger May 05 '17 at 21:24
  • This is one of the problems with writing general purpose code, including scripts: you must anticipate all possible failure cases, or at least all the ones that you care about and wish to handle, *before* they have actually occurred. Well, that, or spend a lot of time debugging and refactoring. :-) – torek May 05 '17 at 21:30
  • do you suggest to use `git status --porcelain` somehow before `git merge --abort`? – smart May 06 '17 at 16:30
  • @MarkAdelsberger,@torek thoughts? – smart May 08 '17 at 07:20
  • As far as I know, `status` will only tell you about files being unmerged (or otherwise having uncommitted or untracked changes); I don't know that you can have it tell you whether the repo is merging. The `__git_ps1` function (used for git-aware command prompts) checks for existence `.git/MERGE_HEAD` under the repo root to decide if a merge is in progress; you could do something like that, but then might also want to check for other statuses (cherry-picking, rebasing, bisecting, ...) that oculd also interfere. It's a big can of worms IMO, so maybe handing failure gracefully would be better – Mark Adelsberger May 08 '17 at 15:07

2 Answers2

3

An error message of the form you need to resolve your current index first means you, earlier, started a regular (non-fast-forward) merge and it failed with a merge conflict.

When git merge fails with a merge conflict, Git produces a status 1 exit. The check_call code will raise CalledProcessError if that particular failed merge occurred as part of this code, but all we know for sure is that there was such a failed merge earlier, not that it happened right here:

subprocess.CalledProcessError: Command '...' returned non-zero exit status 1

Once this happens—whether as part of your Python code, or by some other means—the repository's work-tree and index are left in this "merge failed, hand treatment required" state. If you don't know the repository's state in advance, you should find it out before you start, e.g., using git status --porcelain or git status --porcelain=v2 (see the documentation for git status; note that this web version does not show the new v2 format, which was first release in Git 2.11).

If you have put the work-tree and index into this partially merged, intermediate, requires-hand-treatment state and wish to restore it to the pre-merge state, simply run git merge --abort. Do not attempt a commit or a pull request from this half-vast1 state: the merge must be either finished, or aborted entirely.

Aside: '%s' % expr is silly, just use str(expr), or expr itself if it is already known to be a string.

1Say "half-vast" aloud, three times, fast. :-)

torek
  • 448,244
  • 59
  • 642
  • 775
  • So, the idea is to run `git merge --abort` in my `except` block, before creating a pull request. Is it right? – smart May 04 '17 at 20:44
  • I could be off here because, as I said before, I'm not a python guy... but at least in C, replacing `"%s", stringval` with `stringval` is a dangerous idea (since `stringval`'s value could contain `%`); is that not so in python as well? – Mark Adelsberger May 04 '17 at 20:44
  • @smart: Yes, where have we heard that before? – Mark Adelsberger May 04 '17 at 20:44
  • @MarkAdelsberger: Python isn't C: `'%s' % expr` calls `str.__mod__` with two arguments. The first is the string, in this case `'%s'`; the second is the arbitrary expression. If `expr` is a tuple, the result is often an error (`TypeError: not all arguments converted during string formatting`). Otherwise, the `%s` is replaced by the result of calling `str` on the expression. Since there are no other characters in the original `'%s'` string, the result is just `str(expr)`. So using `str(expr)` makes the thing not-fail if it's a tuple, which is the only significant difference. – torek May 04 '17 at 20:48
  • @MarkAdelsberger: FYI, this is the "old" formatting directive system in Python; since 2.7 and in 3.x, the new favorite is `stringexpr.format(args)`. The new formatting system uses braces instead of percent-signs, and avoids the clumsiness of single-argument `'literal with %s directive' % (expr,)` that is required in case `expr` itself is a tuple. – torek May 04 '17 at 20:50
  • @MarkAdelsberger: I really like Python for quick programming and prototyping. It's simple enough (unlike Perl :-) ), readable (unlike Perl and Ruby both, IMO), has first class functions, and does pretty much everything one needs done. Its big drawback is that the standard (CPython) implementation has one great big lock, the Global Interpreter Lock or GIL, that makes threading safer but terribly slow. It's also so vested in dynamism that it's extremely difficult to JIT-compile. – torek May 04 '17 at 20:55
  • @torek - My go-to for that kind of thing is perl, but that's only because I know it pretty well. In the mean time, we can all agree that smart needs to `--abort` his unfinished `merge`s and then should be fine, right? (This being really a thread about their question rather than our scripting habits...) – Mark Adelsberger May 04 '17 at 20:58
  • @MarkAdelsberger: Yes, the `--abort`-if-failed is the key here. I would add that it might be reasonable to not attempt the merges at all, if you're going to create pull requests anyway. The only point to trying the merge like this is to see if it can be done fully automated ... and if that's true, why not let the one integrating the pull request do the fully-automated merge at his/her end? – torek May 04 '17 at 21:00
  • @torek, so, I want to have automated merged from release branch into feature branches (if it possible to merge without conflicts) to have up-to-date all of our feature branches. Changes between release branches (we have few of them) are done via pull requests together with merging conflicts between conflicting branches. Something like that. – smart May 04 '17 at 21:04
  • @MarkAdelsberger, @torek, btw guys. One additional question.During `git checkout ; git pull origin ` I have one more issue: `here are no candidates for merging among the refs that you just fetched. Generally this means that you provided a wildcard refspec which had no matches on the remote end.` May it be related somehow to previous issue? – smart May 04 '17 at 23:10
  • `git pull` = `git fetch` + `git merge`, more or less, except that you can configure `git pull` to behave differently and hence make it highly unpredictable. You don't have a `git pull` in your original post. I'd need more information to be sure what is happening here, but in anything you are automating, you should avoid `git pull` anyway, lest it behave unpredictably. – torek May 04 '17 at 23:20
  • @smart - I agree with torek that we don't have enough information to *know*, but what it *sounds like* is that you're trying to pull a branch that has no upstream. I don't think it's related to the previous issue, because git should not enter a merging state when that error occurs. – Mark Adelsberger May 05 '17 at 14:12
  • @MarkAdelsberger, @torek I've updated an original post. So, `git merge --abort` didn't help me :) – smart May 05 '17 at 16:39
  • As @MarkAdelsberger said, `git merge --abort` dies (with error message `fatal: There is no merge to abort (MERGE_HEAD missing).`) if there is no in-progress merge to abort. You can't just call it unconditionally unless you're prepared to discard error messages and ignore failures. – torek May 05 '17 at 20:11
0

git merge --abort command after failed merging for some reason

Git 2.38 (Q3 2022) hould help: when "git merge"(man) finds that it cannot perform a merge, it should restore the working tree to the state before the command was initiated, but in some corner cases it didn't (before 2.38).

See commit c23fc07, commit 034195e, commit aa77ce8, commit 1369f14, commit 8f240b8, commit e4cdfe8, commit 24ba8b7, commit 11f4290 (23 Jul 2022) by Elijah Newren (newren).
(Merged by Junio C Hamano -- gitster -- in commit 966ff64, 03 Aug 2022)

merge: do not exit restore_state() prematurely

Reported-by: ZheNing Hu
Signed-off-by: Elijah Newren

Previously, if the user:

  • Had no local changes before starting the merge
  • A merge strategy makes changes to the working tree/index but returns with exit status 2

Then we'd call restore_state() to clean up the changes and either let the next merge strategy run (if there is one), or exit telling the user that no merge strategy could handle the merge.

Unfortunately, restore_state() did not clean up the changes as expected; that function was a no-op if the stash was a null, and the stash would be null if there were no local changes before starting the merge.

So, instead of "Rewinding the tree to pristine..." as the code claimed, restore_state() would leave garbage around in the index and working tree (possibly including conflicts) for either the next merge strategy or for the user after aborting the merge.

And in the case of aborting the merge, the user would be unable to run "git merge --abort"(man) to get rid of the unintended leftover conflicts, because the merge control files were not written as it was presumed that we had restored to a clean state already.

Fix the main problem by making sure that restore_state() only skips the stash application if the stash is null rather than skipping the whole function.

However, there is a secondary problem -- since merge.c forks subprocesses to do the cleanup, the in-memory index is left out-of-sync.
While there was a refresh_cache(REFRESH_QUIET) call that attempted to correct that, that function would not handle cases where the previous merge strategy added conflicted entries.
We need to drop the index and re-read it to handle such cases.

(Alternatively, we could stop forking subprocesses and instead call some appropriate function to do the work which would update the in-memory index automatically.
For now, just do the simple fix.)

Also, add a testcase checking this, one for which the octopus strategy fails on the first commit it attempts to merge, and thus which it cannot handle at all and must completely bail on (as per the "exit 2" code path of commit 98efc8f ("octopus: allow manual resolve on the last round.", 2006-01-13, Git v1.2.0 -- merge)).

VonC
  • 1,262,500
  • 529
  • 4,410
  • 5,250