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

Fri Jun 5 03:38:13 PDT 2020

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.

On Thu, Jun 4, 2020 at 11:09 AM Mikhail Goncharov <goncharov at google.com>
wrote:

> 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
>
>
> On Wed, Jun 3, 2020 at 3:40 PM MyDeveloper Day <mydeveloperday at gmail.com>
> wrote:
>
>> 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.
>>
>>
>>
>>
>> On Wed, Jun 3, 2020 at 1:40 PM Mikhail Goncharov via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> 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
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200605/13354661/attachment.html>