Skip to content

ci: commit lint correctly handles merge trains & squashing

Tomas Vik requested to merge tv/2024-01/merge-train-commit-lint-squash into main

Summary

This MR re-introduces the ability to squash commits within an MR.

We disabled squashing to be able to use Merge Trains (see #820 (closed)).

The problem

GitLab squash behaves differently with and without Merge Trains.

The difference shows in the following scenario: If MR has only one commit, and the MR is set to squash.

  • Without merge trains: GitLab uses the original commit message in the commit it adds to main
  • With merge trains: GitLab uses the MR title in the commit it adds to main

Example

MR with one commit:

  • MR Title: 'feat: add great new feature'
  • Commit message: 'adding foo to bar'

MR is set to squash. Depending on the merge train feature, the resulting commit message in main branch after merge is different:

merge train enabled resulting commit message in main
false adding foo to bar
true feat: add great new feature

Our logic was not expecting that merge trains replace commit message for a single commit and that caused invalid commit messages making it to main (see #820 (closed))

The solution

We build in the expectation that the project has merge trains enabled. The new lint.js script works only for merge trains.

We then use the CI_MERGE_REQUEST_SQUASH_ON_MERGE and CI_MERGE_REQUEST_TITLE CI environment variables to correctly decide if we should lint MR title or all commit messages.

The MR pipeline always runs on the "Merge result", which means that it always runs on merge commit when one parent comes from the target branch (usually main) and the other parent comes from the MR. We introduce a shell script, lint.sh, that finds the last MR commit and always uses that for linting.

This results in always testing exactly the commit(s) that will be added by the MR.

Also, I removed the local linting since invalid commits in MR don't block merging. The maintainer can change the MR title and merge.

Related Issues

Resolves #820 (closed)

How has this been tested?

I tested several scenarios:

  1. Merge MR without squashing with one invalid commit
  2. Merge MR with squashing, invalid MR title and not editing the commit message
  3. Merge MR with squashing and editing the commit message to an invalid mesasge
  4. Add a commit to main before merging to make sure that the way we find the main branch HEAD is correct (we use CI_MERGE_REQUEST_TARGET_BRANCH_SHA)

Testing MR viktomas/gitlab-vscode-extension!9

Old testing scenarios

I tested this extensively.

MR with one commit

✅ check that non-squash MR Train with one commit is linted properly (checking commit message, not MR description)

  1. create MR with 1 commit and a bad message
  2. set MR title to be valid and set to squash
  3. let the pipeline pass
  4. uncheck the squash and try to merge
  5. see merge train fail
  6. force push amended commit with OK message
  7. uncheck the squash and merge
  8. see success

Test MR: viktomas/gitlab-vscode-extension!4 (merged)

✅ check that merging squash MR with one commit discards the commit message in favour of the MR Title

  1. create MR with 1 commit and a bad message

  2. set MR title to be valid and MR to squash

  3. see the pipeline succeed

  4. start merge train

  5. see merge success

  6. validate that the new commit in the main has a message from the MR title and not the invalid message

  7. image

    ~/w/g/gitlab-vscode-extension (main↑12|✔) ❯❯❯ git ll
*   01629e81 (HEAD -> main, mine/main) Merge branch 'test-merge-train-4' into 'main'
|\  
| * 51a75cb9 chore: good MR message
|/  
*   c29a8ba7 Merge branch 'test-merge-train-3' into 'main'

Test MR: viktomas/gitlab-vscode-extension!5 (merged)

MR with multiple commits

✅ check that non-squash MR train with multiple commits is linted properly (check all commits)

  1. create MR with two commits, one bad

  2. set MR to not squash

  3. see lint fail (https://gitlab.com/viktomas/gitlab-vscode-extension/-/jobs/6001841570)

  4. amend the bad commit

  5. se lint succeed

  6. start the merge train and see the two valid commits in the main

    ~/w/g/gitlab-vscode-extension (main↑15|✔) ❯❯❯ git ll
*   536e4e5b (HEAD -> main, mine/main) Merge branch 'test-merge-train-5' into 'main'
|\  
| * 00810201 (mine/test-merge-train-5, test-merge-train-5) ci: improve commit lint logging
| * 16d3912f chore: good message
|/  
*   01629e81 Merge branch 'test-merge-train-4' into 'main'

Test MR: viktomas/gitlab-vscode-extension!6 (merged)

✅ check that merging squash MR with multiple commits will use the MR title

  1. create MR with two commits, both bad

  2. set MR to squash

  3. merge MR

  4. see that the commit in the main has the message from the MR title

    ~/w/g/gitlab-vscode-extension (main↑17|✔) ❯❯❯ git ll
*   05210219 (HEAD -> main, mine/main) Merge branch 'test-merge-train-6' into 'main'
|\  
| * 0a367354 chore: valid MR message for multiple bad commits
|/  
*   536e4e5b Merge branch 'test-merge-train-5' into 'main'

Test MR: viktomas/gitlab-vscode-extension!7 (merged)

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation
  • Chore (Related to CI or Packaging to platforms)
  • Test gap
Edited by Tomas Vik

Merge request reports