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

Fri Feb 25 20:59:34 PST 2022

nemanjai added a comment.

In D120305#3346880 <https://reviews.llvm.org/D120305#3346880>, @MaskRay wrote:

> While I feel sorry for leaving clang-ppc64le-rhel red for some time and am willing to fix issues if I have access to a ppc64 machine (especially compiler-rt ones that I care about),
> I feel uncomfortably if a group just bluntly request "please pull this patch" when apparently (a) there is a better approach (explicitly setting CLANG_DEFAULT_PIE_ON_LINUX=OFF) and (b) there is something a bot maintainer can do
> and (c) there is just some inherent stability problem (in this case, consider not enabling the testing when the target is still unstable) that is causing not only this issue, but various other reports (as I watch sanitizer failures quite closely and ppc64 often tends to be the outlier thing)

Statements like this seem to be at odds with this community's culture (or at least the way I understand it).
As long as I have been a member of this community, the guidance for patches that break bots is to fix it immediately if the fix is obvious/trivial and if it isn't, to pull the patch until a solution can be found. I am not aware of any changes to this policy. I would also like to add that this approach serves most other members of the community quite well and at least I personally don't see much opposition to this. Frankly, the only person I have ever received a response that amounts to "I would rather not" when asking them to pull a patch that breaks bots is yourself.

So I'll try to respond to the individual statements you have made here.

1. No access to a PowerPC machine - we have given you access to one before and are happy to do so again at any time.
2. "bluntly requesting to pull the patch" - This is perhaps the part of your statement that I find most surprising. In case you haven't come across this <https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy>, I encourage you to have a read through it. If you feel this needs to change, I encourage you to bring it up for discussion with the wider community. Of particular interest is this sentence: `We strongly encourage “revert to green” as opposed to “fixing forward”. We encourage reverting first, investigating offline, and then reapplying the fixed patch - possibly after another round of review if warranted.`
3. "there is a better approach" - I don't think I have to spend a lot of time explaining how subjective this statement is.
4. "there is something that a bot maintainer can do" - I can't quite decipher whether this is a disingenuous statement pretending that this is a situation where the bot maintainer (effectively myself in this case) isn't willing to help. Or perhaps it is a statement you made in the heat of the moment when I asked you to pull the patch and you missed the part where I offered help to resolve the issues when normal business resumes. I will give you the benefit of assuming the latter as I truly don't believe you have ill intentions here. I would just like to add that, as you surely realize, bot maintainers are not sitting around waiting for someone to break their bot so they can jump in immediately and work on resolving the issue. As contributors to this project, it is our responsibility to keep the tip of trunk green and to work with bot maintainers on their own time (within reason) to resolve issues we cause on their bots.
5. The rather surprising and discouraging statement you made under `(c)` - while I realize that PowerPC may not be a target on which you develop, it is really not fair to make a blanket statement that PowerPC is not stable wrt. sanitizers. If you feel there is a stability issue with a specific bot or a specific target, I encourage you to collect data about false failures and bring it up with us - we would be happy to look into it as I am sure would any other target/bot maintainer. Statements like this sow the seeds of resentment towards specific targets - you are effectively saying that PowerPC is an unstable target (at least wrt. to sanitizers) but we insist on burdening the community with our unstable target by running sanitizer tests in our bots. I am afraid that unlike number 4. above, I don't find any ambiguity in your statement here. Your statement seems to be clearly and unambiguously hostile towards PowerPC and as such, I find it at odds with the spirit of this community. Regardless of all of that, I would once again like to reiterate that I would be very happy to look into false failures you are encountering with sanitizers on PowerPC.

Fangrui, I would really like you to understand that I very much value your contribution to various LLVM projects. You are an exceptional developer and seem to be laser focused on advancing the state of LLVM and its subprojects. This is not at all lost on me. However, you need to understand that this community has all kinds of participants, many of which also care about older or somewhat esoteric targets, operating systems, toolchains, etc. As such, there will be situations such as this where it is definitely the case that `most != all` and there may have to be some nuance in applying changes. At the very least, such situations may require slowing down, pulling a patch or two, re-evaluating and finding the best way forward. And it is important to recognize that sometimes the best way forward may not be something that all parties prefer, but at least all affected parties need to be part of the discussion.

It is unfortunate that a discussion in a patch review devolved into a discussion on the philosophy of community participation, but I felt that you made strong statements that I needed to address. While I won't continue this discussion in this forum, I would be happy to meet with you virtually and discuss this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120305