git-pile / git-pile

Manage a pile of patches on top of a git branch
GNU Lesser General Public License v2.1
17 stars 7 forks source link

Add git-merge-range and merge-range strategy #116

Open guludo opened 7 months ago

guludo commented 7 months ago

This PR adds a new command called git-merge-range and makes use of as a new git-pile-am strategy named merge-range. The following paragraphs are extracted from f3c1895e25b8 ("am: Introduce the merge-range strategy") to provide the motivation:

It is not uncommon to have conflicts when applying a large series to the
pile (like a rebase onto a new baseline) after some development from
other contributors has been incorporated since the creation of the
series.

Using git-merge-range has proven to be very useful in those situations,
because it reduces the number of conflicts that one would run into when
applying the series and, if conflicts happen, they are arguably easier
to work with, since we handle them at the "file level" instead of
working directly on the patches.

While one can do the steps to use git-merge-range manually, it is much
better to the user that the "am" operation automates that by having a
"merge-range" strategy, so let's implement it.
guludo commented 7 months ago

Force-pushed to make linters happy.

lucasdemarchi commented 1 week ago

Humn... tests currently failing and it seems unrelated to previous CI breakage:

https://github.com/git-pile/git-pile/actions/runs/10724118311#summary-29739028102

guludo commented 1 week ago

Looks like we are getting non-zero exit status for the command git var GIT_SEQUENCE_EDITOR. I guess I'll need to run the test on the same container to debug this.

Another error specific for git v2.20.* is that we are getting non-zero result from git branch --show-current. Maybe because that flag is not supported on that version.

guludo commented 1 week ago

Looks like we are getting non-zero exit status for the command git var GIT_SEQUENCE_EDITOR. I guess I'll need to run the test on the same container to debug this.

Yeah. I confirmed that this command simply fails in the container. The support for GIT_SEQUENCE_EDITOR in git var was added with https://github.com/git/git/commit/4c3dd9304e49402bd4ee19dfaa4c21d0217fb582, which seems to be present starting with git v2.40.

As a fallback, we could implement a python function that mimics what git does: https://github.com/git/git/blob/2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c/editor.c#L48

const char *git_sequence_editor(void)
{
    const char *editor = getenv("GIT_SEQUENCE_EDITOR");

    if (!editor)
        git_config_get_string_tmp("sequence.editor", &editor);
    if (!editor)
        editor = git_editor();

    return editor;
}
lucasdemarchi commented 1 week ago

As a fallback, we could implement a python function that mimics what git does: https://github.com/git/git/blob/2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c/editor.c#L48

sounds good to me. Probably good to just use git var GIT_SEQUENCE_EDITOR and fallback on error?

guludo commented 1 week ago

As a fallback, we could implement a python function that mimics what git does: https://github.com/git/git/blob/2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c/editor.c#L48

sounds good to me. Probably good to just use git var GIT_SEQUENCE_EDITOR and fallback on error?

Yep. That's what I ended up doing.

guludo commented 1 week ago

As a fallback, we could implement a python function that mimics what git does: https://github.com/git/git/blob/2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c/editor.c#L48

sounds good to me. Probably good to just use git var GIT_SEQUENCE_EDITOR and fallback on error?

Yep. That's what I ended up doing.

And we will probably need to do something similar for git branch --show-current.

guludo commented 1 week ago

@lucasdemarchi, looks like there are some more environment-related CI issues to solve here. Since I don't have the time to work on them right now, I do not want to block the next release of git-pile on this.

lucasdemarchi commented 1 week ago

@guludo ok... doing a release is very cheap. We can release 1.2, then fix this without rushing it and then doing a 1.3.