PR branch merge is not triggered on fresh clone

[bilderbuchi]

I noticed that our builds were sometimes not building the merge commits for a PR update event.

After quite some troubleshooting, I realised that, if the Git repo that we are building a PR for does not yet exist on the Worker, the Buildbot Git step does a full clone/clobber (of course).
In all logic paths I could find (for any combination of mode and method), this resulted in the Git._fetch()` call being skipped, so all the logic in https://github.com/lab132/buildbot-gitea/blob/1d10fc0e119b66e66e777a43fabc888ded5d0a9a/buildbot_gitea/step_source.py is never executed, and the merge of the PR branch into the base branch never happens.

Ultimately, the base branch head will be built, but there are no error messages or somesuch that alert you to that fact.

Additional context:
We are using a Dockerfile committed to the repo to recreate a Worker from, to improve reproducibility. So, we have a "wrapping" builder that checks out the repo, rebuilds the worker if needed, and uses Trigger to start the actual CI build on the worker.
This means that more often than usual, we start with a fresh worker that does not have the Git repo to be built, yet.

We worked around this for now by just having two identical Gitea steps in our CI builder. This way we ensure that the Git repo exists at least for the second try, so the conditions are such that the _fetch code path is executed.

Buildbot 2.7.0
buildbot-gitea 1.7.2

[pampersrocker]

Thanks for the report!
This all makes sense, I think this has ever been tested on existing repos and cloning works entirely different from the fetch. (Also the whole update process for this steps works entirely different as you would do on a command line).

Our use case is the exact opposite since we are using this to build large video games with it and re-cloning a repo would mean downloading and regenerating hundreds of GB of data, so we specifically avoid creating new workers if not necessary.

Your workaround is probably part of the fix. As in the clone case we would need to do the merge tasks after the clone.

However, I have sadly no time myself to implement this fix at the moment (The fix itself is probably easily done, but testing and verifying is more complex). Any PR is greatly appreciated.

[bilderbuchi]

I'm not familiar enough to judge the full implications (e.g. if this avoids doing work twice), but another fix approach could be to add the gitea code not only on the self._fetch callback, but also to self._clone https://github.com/buildbot/buildbot/blob/54185067e187b55d296f03dbdfd76f42ba2a5bc0/master/buildbot/steps/source/git.py#L369.

[bilderbuchi]

I think I am hitting the same problem (buildbot-gitea step logic not being executed) if I use a Trigger step with alwaysUseLatest=False. Instead of the base branch head, it now builds the PR branch head (instead of the merge commit).

The problem is that I need that argument because otherwise it merges the wrong commit in situations where the PR branch already exists on the worker with later commits (observed this when force-pushing).

No idea how to work around this ATM, the approach with two Git steps does not work.

[bilderbuchi]

If that might be useful for someone else, when using a Trigger step withalwaysUseLatest=False, the following was needed to unbreak as much functionality as I could observe/test:

    steps.Trigger(
        name="Execute actual CI",
        schedulerNames=[f"{project_slug} CI"],
        waitForFinish=True,
        alwaysUseLatest=False,
        set_properties={
            'pr_id': util.Property('pr_id'),
            'base_git_ssh_url': util.Property('base_git_ssh_url'),
            'head_sha': util.Property('head_sha'),
            'head_git_ssh_url': util.Property('head_git_ssh_url'),
            'head_reponame': util.Property('head_reponame'),
            'head_owner': util.Property('head_owner'),
            'repository_name': util.Property('repository_name'),
            'owner': util.Property('owner'),
            'event': util.Property('event'),
            'revision': util.Property('revision'),
        }

I cannot confirm that this set is exhaustive, probably I'll later find out that something else is not working.

It seems like with alwaysUseLatest=False, too few of these properties get forwarded to the triggered step, which broke ~everything in buildbot-gitea.

[pampersrocker]

Well the fresh checkout problem can be fixed, but nothing can really be done from this plugin to fix the isue with a trigger step, since there is no way to tell a property that it should be auto copied or similar in a trigger step.

[bilderbuchi]

nothing can really be done from this plugin to fix the isue with a trigger step, since there is no way to tell a property that it should be auto copied or similar in a trigger step.

Yeah, I understand, that's why I tried to document my workaround here.
I tried to find out why the Trigger step does not forward those properties anymore with the alwaysUseLatest=False setting, but was not really successful. I feel like it should, though, but did not manage to collect enough detail/insight for a bug report.