[PATCH] D120305: [Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON

Sat Feb 26 00:26:29 PST 2022

MaskRay added a comment.

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.
And one pointed out that your "nemanjai is correct" message was made a bit arbitrary, without evidence of considering the full context.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120305/new/

https://reviews.llvm.org/D120305