[llvm-dev] pre-merge checks are switching to buildkite build system

[Mikhail Goncharov via llvm-dev]

Hello friends,

We are switching the pre-merge test build system from Jenkins to Buildkite.

That will give authors and reviewers more transparency on what's going on during the build process. For now only members of "pre-merge beta testing" [0] group are affected.

As usual, please tell us if something is off.

[0] https://reviews.llvm.org/project/view/78/

 

Kind regards,

Mikhail

[MyDeveloper Day via llvm-dev]

Mikhail

Firstly let me say that I love the pre-merge checks...but I recently saw something a little odd

A recent change I made  to clang-format failed the pre-merge checks

https://results.llvm-merge-guard.org/BETA_amd64_debian_testing_clang8-1980/summary.html 

This was because as part of the revision I clang-formatted one of the files with a build of clang-format that contained the fix I was making.

https://reviews.llvm.org/D80950 

i.e. I was making a change to not just break between   "XXX" << "XXX" just because it was 2 strings either side of "<<" and included as way of a demonstration the one other file in lib/Format that violated that rule (because we keep lib/Format 100% clang-format clean)

The failure from the pre-merge check was: (clang-format.patch)

diff --git clang/lib/Format/UnwrappedLineParser.cpp clang/lib/Format/UnwrappedLineParser.cpp
index 9c25e107d44..b8da2c23b55 100644
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2744 +2744,2 @@ LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine &Line,
-    llvm::dbgs() << I->Tok->Tok.getName() << "[" << "T=" << I->Tok->getType()
+    llvm::dbgs() << I->Tok->Tok.getName() << "["
+                 << "T=" << I->Tok->getType()

 Reading the documentation for the pre-merge checks it says this...

Linux

1. Checkout of the branch (from apply patch)
2. Guess which projects were modified, run Cmake for those.
3. Build the binaries -- ninja all
4. Run the test suite -- ninja check-all
5. Run clang-format and clang-tidy on the diff.
6. Upload build results to Phabricator

However could you clarify: if step v.

>  Run clang-format and clang-tidy on the diff.  

Uses the clang-format/clang-tidy binaries either

a) built at step iii. or 

b) if you use a pre-existing version? 

If b) which version do you use?

a) last successful built

b) tip of existing committed master

c) last released version.

Many thank in advance

MyDeveloperDay.

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Mikhail Goncharov via llvm-dev]

Hi MyDeveloperDay,

We are using the released version of clang-format / clang-tidy (not necessarily the latest release). I think it makes sense to use most recent versions of the tools: https://github.com/google/llvm-premerge-checks/issues/196 

Kind regards,

Mikhail

[MyDeveloper Day via llvm-dev]

Actually I've been thinking about this more, I wouldn't expect the rest of the developers who are working in LLVM to be living at Trunk of the clang-format binary, using the last release say v10 at the time of writing seems reasonable.

However for those of us actually developing clang-format we are more than likely going to be using the latest and greatest and as such we'll submit code (as I did here) which utilizes the latest changes/bug fixes.

It would be great if both worlds could exist, where we support the current tip of trunk for clang-format and the last version, for that to happen I feel the pre merge check would need to

1) check using the last release of clang-format (v10 at time of writing)

2) If 1) passes (then we are good)

3) If 1) fails, then check the patch again with the last trunk version of clang-format (v11 at time of writing )

4) if 3) passes then we are good 

5) if 3) also fails report the diffs from 1) as the failures

Otherwise I feel developers outside of clang-format will start getting clang-format warnings for a clang-format that they may not have access to, unless they build it themselves.

But also clang-format developers or those that like to use the latest clang-format will be able to ensure they are keeping clang-format area clean with the trunk version, if we don't do this we can find that come a new release, there is a bunch of files that just start failing clang-format, and people don't like a big clang-format only commits.

I hope this kind of approach seems sensible and reasonable in order to prevent false negatives from the pre-merge checking.

MyDeveloeprDay.

[Mehdi AMINI via llvm-dev]

On Fri, Jun 5, 2020 at 3:38 AM MyDeveloper Day via llvm-dev <llvm...@lists.llvm.org> wrote:

Actually I've been thinking about this more, I wouldn't expect the rest of the developers who are working in LLVM to be living at Trunk of the clang-format binary, using the last release say v10 at the time of writing seems reasonable.

However for those of us actually developing clang-format we are more than likely going to be using the latest and greatest and as such we'll submit code (as I did here) which utilizes the latest changes/bug fixes.

It would be great if both worlds could exist, where we support the current tip of trunk for clang-format and the last version, for that to happen I feel the pre merge check would need to

1) check using the last release of clang-format (v10 at time of writing)

2) If 1) passes (then we are good)

3) If 1) fails, then check the patch again with the last trunk version of clang-format (v11 at time of writing )

4) if 3) passes then we are good 

5) if 3) also fails report the diffs from 1) as the failures

Otherwise I feel developers outside of clang-format will start getting clang-format warnings for a clang-format that they may not have access to, unless they build it themselves.

If pre-merge is using the latest release, then why would developers need to build it themselves?

 

But also clang-format developers or those that like to use the latest clang-format will be able to ensure they are keeping clang-format area clean with the trunk version, if we don't do this we can find that come a new release, there is a bunch of files that just start failing clang-format, and people don't like a big clang-format only commits.

I think we're only running clang-format on the diff, not on complete files anyway. 

[Mikhail Goncharov via llvm-dev]

We have not noticed any significant issues with Buildkite migration and now ALL pre-merge checks for diff reviews are running on Buildkite!

Kind regards,

Mikhail