Compile all commits of a PR branch

[MiguelWeezardo]

Description

To make automated git bisect work better, we need to make sure all commits in a PR are at least compiling.
This change calls Maven install for all commits between master and the PR branch.

Is this change a fix, improvement, new feature, refactoring, or other?

This is an improvement of build stability.

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

This is a change in the CI pipeline.

How would you describe this change to a non-technical end user or system administrator?

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

[nineinchnick]

Can you show an example of failure for a non-head commit? Would it be easy enough to identify which commit failed?

Also, I don't think we want to run all checks on all commits except the head. We only want to make sure other commits compile.

[electrum]

Why?

[MiguelWeezardo]

Git rebase needs an identity to assign to commits. I'm using the identity from the top commit in the PR.

[hashhar]

Provide any random identity - doesn't matter since we won't use the results.

[hashhar]

Rebasing on origin/master instead of PR head means that the commit hashes for the rebased result may change (since other PRs could've been merged in meantime) and users will have harder time to figure out which commit caused compilation failure.

Also not every PR is open against master - they can target different branches as well.

[nineinchnick]

There are examples elsewhere in this workflow how to get current PRs head

[martint]

What's the purpose of this?

BTW, compile-all-commits@trino.org is not a valid email.

[MiguelWeezardo]

git rebase needs an identity to tag rebased commits with.
These commits will be discarded at the end, but the important part is that git rebase will run whatever is passed in --exec between all commits

[martint]

Does it need to be a real email? Can it be a string such as "dummy", "nobody", "empty"?

[MiguelWeezardo]

I'm not sure if git validates this, but I finally noticed you probably meant that trino.org is not the Trino domain trino.io

[hashhar]

git doesn't validate it - any non-empty value works.

[martint]

Is this guaranteed?

[MiguelWeezardo]

The ordering depends on which branch is merged into which, but I think I found a less complicated way to do this.

[hashhar]

You can count the commits from merge-base to HEAD and if there is just 1 skip the rebase?

git rev-list --count start~..end

[MiguelWeezardo]

The version you saw doesn't contain a git checkout HEAD~ which drops the top PR commit.
By doing so, if the PR branch is only 1 commit, HEAD ends up pointing to a commit that is already on the base branch and rebase turns into a no-op.

[hashhar]

Nice, this is much simpler now.

[MiguelWeezardo]

@martint Could you take another look? Is this merge-worthy?

[MiguelWeezardo]

@martint Could I request some feedback on this PR?

[hashhar]

LGTM % 1 nit

[hashhar]

Do you have some CI runs with this within your fork? master behaves a bit differently with GIB so we can't verify it before it's merged otherwise.

[MiguelWeezardo]

Do you have some CI runs with this within your fork? master behaves a bit differently with GIB so we can't verify it before it's merged otherwise

I've pushed changes in 3 commits so we can see it in action. We can squash everything before merge.

[hashhar]

LGTM % logging improvements

[hashhar]

(remember to squash before merge)

[hashhar]

One this which verbose is showing is that it seems we are running compilation for alternate commits only?

i.e. I see maven log output only for some steps of the rebase. Is there some way to verify it is indeed running compilation for each commit?

[MiguelWeezardo]

One this which verbose is showing is that it seems we are running compilation for alternate commits only?

i.e. I see maven log output only for some steps of the rebase. Is there some way to verify it is indeed running compilation for each commit?

With rebase --exec you have twice as many rebase steps than commits, because the rebase log looks like this:

pick X commit 1
exec cmd
pick Y commit 2
exec cmd

Compilation happens on even-numbered steps.

[hashhar]

Ah, that clears it up for me. I did not realise the numbers are for each "action" instead of commit - it makes total sense though why it looks like it does. Thankfully with --versbose it's less confusing since it says which commit has been checked out.

[hashhar]

@MiguelWeezardo Please squash after you have a green run with the latest changes. It's good to be merged now.

[MiguelWeezardo]

@hashhar I think we're all set. Could you merge?

[hashhar]

@MiguelWeezardo Sorry for one more comment. Do you think it's be useful to print git log --graph --reverse instead of in forward direction? That way the commits on this branch would be listed at top of output instead after long list of commits on master?