tstellar added a comment. In D120305#3347148 <https://reviews.llvm.org/D120305#3347148>, @MaskRay wrote:
> In D120305#3347145 <https://reviews.llvm.org/D120305#3347145>, @tstellar > wrote: > >> In D120305#3347143 <https://reviews.llvm.org/D120305#3347143>, @MaskRay >> wrote: >> >>> In D120305#3347142 <https://reviews.llvm.org/D120305#3347142>, @tstellar >>> wrote: >>> >>>> In D120305#3347139 <https://reviews.llvm.org/D120305#3347139>, @MaskRay >>>> wrote: >>>> >>>>> In D120305#3347109 <https://reviews.llvm.org/D120305#3347109>, @tstellar >>>>> wrote: >>>>> >>>>>> [...] >>>>>> The issue here has nothing to do with the technical merits of the patch >>>>>> or what the root cause of the problem is. The policy for this project >>>>>> is that if you commit a patch that breaks someone's configuration >>>>>> (especially a buildbot), then it needs to be fixed quickly or reverted. >>>>>> I get that this policy can be frustrating as a committer when you feel >>>>>> your patch is correct, and the real problem is elsewhere, but this is >>>>>> still the policy and it should be followed. >>>>> >>>>> 7 hours ago my >>>>> https://github.com/llvm/llvm-zorg/commit/b6ddf02ce3a54da2df29e7e599b1838167e0e3ad >>>>> was sufficient to fix the issue and was the suggested fix in my opinion. >>>>> Unfortunately nobody on the PowerPC side made the change effective in the >>>>> build bot. Rather, I received such a heated message >>>>> (https://reviews.llvm.org/D120305#3347058). >>>>> It was another way to fix the redness (revert) but IMO not justified. >>>> >>>> I feel like we are talking past each other at this point, but in general >>>> changing the buildbot configuration is not an acceptable solution to a >>>> broken bot. Did the bot owner approve that change? >>> >>> Unsure why it isn't acceptable. There is fairly strong evidence that that >>> specific ppc64le buildbot has a stability issue and >>> `-DCLANG_DEFAULT_PIE_ON_LINUX=OFF` keeps it as if before this CMake patch. >>> We can always discuss this with @nemanjai in another place, e.g. in IRC if >>> we want to stop bothering others subscribing to this thread. >> >> It's not acceptable, because buildbots are meant to represent a specific >> configuration that the buildbot owner cares about. Changing the buildbot >> configuration makes the buildbot no longer useful to them since it is now >> testing something different. For example, if someone is running production >> builds that used the same configuration as the buildbot, those production >> builds are still broken even though the buildbot is green. Configuration >> changes like this need to have approval of the bot owner. > > If clang-ppc64le-rhel works with `-DCLANG_DEFAULT_PIE_ON_LINUX=off` but not > with `-DCLANG_DEFAULT_PIE_ON_LINUX=on`, adding > `-DCLANG_DEFAULT_PIE_ON_LINUX=off` makes the intention explicit. I am not > sure why this isn't acceptable. > It's a tech debt, though, as we are making configurations fragmented. > >> I still don't understand why you can't revert the patch. I've encountered >> this same situation numerous times while working on this project and no one >> has ever objected this much to doing a revert. The fact that this is the >> second time this has happened is concerning to me. > > I have stated my reasoning. Configuration churn can also be a problem to > users. > Consider what if the DWARF v5 patch got reverted and relanded back and forth. > Downstream users would keep observing changing behaviors. > > In this case, really even normal ppc64le machines (including some bots) were > happy and just that one was picky. > Given that the bot does not have a high success rate (track record) for at > least the past month, I am unsure I am supposed to revert my change. > > It is more concerning to me that a bot maintainer leaves an unstable bot for > so long and is not even willing to spend very little time to make a llvm-zorg > change live (perhaps just restart the bot software), but rather is more > willing to **write such a long reply taking it very personally**. > (Sorry, I know when I write this, I was a bit in a mood. I felt quite > frustrated at this point, so I could not keep using the tune when I made the > reply https://reviews.llvm.org/D120305#3347094) > > I can response to you that I have shared the thread to some folks and at > least two agree with me that nemanjai's reply made the discussion less > technical but more personal. Can these folks provide their feedback on the thread? > And one pointed out that your "nemanjai is correct" message was made a bit > arbitrary, without evidence of considering the full context. The Reversion Policy is documented here: https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120305/new/ https://reviews.llvm.org/D120305 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
