craft: Last step of publish is selecting the wrong merge target

I might have a solution to this. Depending on whether I can get to it, I will try to push up a PR and test this locally.

For background: The current approach searches the current branches history for branch annotations. This assumes that the base branch has not moved in the meanwhile, e.g. master still points to a direct ancestor.

My idea is based on https://stackoverflow.com/a/17843908. Instead, use git show-branch to search for the nearest ancestor commit (marked with a *) that resides in a different branch. Example borrowed from the stack overflow answer with the relevant ancestor commit marked:

 A---B---D <-main (e.g. on version 2.0)
      \
       \
        C---E*---I <-maintenance/1.x
             \
              \
               F---G---H <-release/1.7.0

@BYK is that something you remember considering initially, or any gotchas on your end before i go down the rabbit hole?

@jan-auer I chose the current method in https://stackoverflow.com/a/55238339/90297 due to the reasons they listed under “Why did not show-branch work for me”. I did not encounter them but the last case kind of looked possible (develop vs master for hotfixes). I think we can try the other approach and see if it actually gives us any headaches.

We just hit this same bug with sentry-native, see https://github.com/getsentry/publish/issues/694#issuecomment-997978039

The culprit here is the following:

• There’s a branch ref pointing directly to an old commit on the master branch
• The release branch has been branched off after that commit
• Craft uses git log --decorate --simplify-by-decoration --oneline, which doesn’t see the branch point
• It finds that old ref and uses it as a base branch.

I haven’t had the chance to look into this much further. We would basically need to find the branching point rather than searching for refs in the log. Same when the base branch moves on in the meanwhile, which is what happened for the Sentry release.

Thinking of this again, since the issue is master moving too quickly, I think another pull right before trying to determine the target should fix the issue mostly?

An alternative would be to add this as an option to the config file instead of a CLI argument. This way projects can easily control the behavior?

That’s more along the lines of what I was thinking. Basically any option that craft accepts on the command line should be available through the config file.