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

Fri Jun 5 13:33:01 PDT 2020

On Fri, Jun 5, 2020 at 3:38 AM MyDeveloper Day via llvm-dev <
llvm-dev at 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.

>
> 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
>>>>
>>> _______________________________________________
> 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/2e8b3732/attachment.html>