<div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jun 5, 2020 at 3:38 AM MyDeveloper Day via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">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.<div><br></div><div>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.</div><div><br></div><div>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</div><div><br></div><div>1) check using the last release of clang-format (v10 at time of writing)</div><div>2) If 1) passes (then we are good)</div><div>3) If 1) fails, then check the patch again with the last trunk version of clang-format (v11
at time of writing
)</div><div>4) if 3) passes then we are good </div><div>5) if 3) also fails report the diffs from 1) as the failures</div><div><br></div><div>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.</div></div></blockquote><div><br></div><div>If pre-merge is using the latest release, then why would developers need to build it themselves?</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>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.</div></div></blockquote><div><br></div><div>I think we're only running clang-format on the diff, not on complete files anyway. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>I hope this kind of approach seems sensible and reasonable in order to prevent false negatives from the pre-merge checking.</div><div><br></div><div>MyDeveloeprDay.</div><div><br></div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 4, 2020 at 11:09 AM Mikhail Goncharov <<a href="mailto:goncharov@google.com" target="_blank">goncharov@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi MyDeveloperDay,<div><br></div><div>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: <a href="https://github.com/google/llvm-premerge-checks/issues/196" target="_blank">https://github.com/google/llvm-premerge-checks/issues/196</a> </div><div><br clear="all"><div><div dir="ltr"><div dir="ltr">Kind regards,<div>Mikhail</div></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 3, 2020 at 3:40 PM MyDeveloper Day <<a href="mailto:mydeveloperday@gmail.com" target="_blank">mydeveloperday@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">Mikhail<div><br></div><div>Firstly let me say that I love the pre-merge checks...but I recently saw something a little odd<div><br></div><div>A recent change I made to clang-format failed the pre-merge checks<div><br></div><div><a href="https://results.llvm-merge-guard.org/BETA_amd64_debian_testing_clang8-1980/summary.html" target="_blank">https://results.llvm-merge-guard.org/BETA_amd64_debian_testing_clang8-1980/summary.html</a> </div><div><br></div><div>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.</div><div><br></div><div><a href="https://reviews.llvm.org/D80950" target="_blank">https://reviews.llvm.org/D80950</a> </div><div><br></div><div>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)</div><div><br></div><div>The failure from the pre-merge check was: (clang-format.patch)</div><div><br></div><div>diff --git clang/lib/Format/UnwrappedLineParser.cpp clang/lib/Format/UnwrappedLineParser.cpp<br>index 9c25e107d44..b8da2c23b55 100644<br>--- clang/lib/Format/UnwrappedLineParser.cpp<br>+++ clang/lib/Format/UnwrappedLineParser.cpp<br>@@ -2744 +2744,2 @@ LLVM_ATTRIBUTE_UNUSED static void printDebugInfo(const UnwrappedLine &Line,<br>- llvm::dbgs() << I->Tok->Tok.getName() << "[" << "T=" << I->Tok->getType()<br>+ llvm::dbgs() << I->Tok->Tok.getName() << "["<br>+ << "T=" << I->Tok->getType()<br></div><div><br></div><div> Reading the documentation for the pre-merge checks it says this...</div><div><p style="box-sizing:border-box;margin-top:16px;margin-bottom:16px;color:rgb(36,41,46);font-family:-apple-system,BlinkMacSystemFont,"Segoe UI",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji";font-size:16px">Linux</p><ol style="box-sizing:border-box;padding-left:2em;margin-top:0px;margin-bottom:0px;list-style-type:lower-roman;color:rgb(36,41,46);font-family:-apple-system,BlinkMacSystemFont,"Segoe UI",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji";font-size:16px"><li style="box-sizing:border-box">Checkout of the branch (from apply patch)</li><li style="box-sizing:border-box;margin-top:0.25em">Guess which projects were modified, run Cmake for those.</li><li style="box-sizing:border-box;margin-top:0.25em">Build the binaries -- <code style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:13.6px;padding:0.2em 0.4em;margin:0px;background-color:rgba(27,31,35,0.05);border-top-left-radius:3px;border-top-right-radius:3px;border-bottom-right-radius:3px;border-bottom-left-radius:3px">ninja all</code></li><li style="box-sizing:border-box;margin-top:0.25em">Run the test suite -- <code style="box-sizing:border-box;font-family:SFMono-Regular,Consolas,"Liberation Mono",Menlo,monospace;font-size:13.6px;padding:0.2em 0.4em;margin:0px;background-color:rgba(27,31,35,0.05);border-top-left-radius:3px;border-top-right-radius:3px;border-bottom-right-radius:3px;border-bottom-left-radius:3px">ninja check-all</code></li><li style="box-sizing:border-box;margin-top:0.25em">Run clang-format and clang-tidy on the diff.</li><li style="box-sizing:border-box;margin-top:0.25em">Upload build results to Phabricator</li></ol></div><div><br></div><div>However could you clarify: if step v.</div><div><br></div><div>> Run clang-format and clang-tidy on the diff. <br></div><div><br></div><div>Uses the clang-format/clang-tidy binaries either</div><div><br></div><div>a) built at step iii. or </div><div>b) if you use a pre-existing version? </div><div><br></div></div><div>If b) which version do you use?</div><div><br></div><div>a) last successful built</div><div>b) tip of existing committed master</div><div>c) last released version.</div><div><br></div><div>Many thank in advance</div><div><br></div><div>MyDeveloperDay.</div><div><br></div><div><br></div><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 3, 2020 at 1:40 PM Mikhail Goncharov via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Hello friends,</div><div><br></div><div>We are switching the pre-merge test build system from Jenkins to Buildkite.</div><div>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.</div><div><br></div><div>As usual, please tell us if something is off.</div><div><br></div><div>[0] <a href="https://reviews.llvm.org/project/view/78/" target="_blank">https://reviews.llvm.org/project/view/78/</a></div> <br clear="all"><div><div dir="ltr"><div dir="ltr">Kind regards,<div>Mikhail</div></div></div></div></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div></div>