[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

Mark de Wever via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 19 09:17:34 PDT 2023


Mordante added a comment.

In D144509#4356160 <https://reviews.llvm.org/D144509#4356160>, @hans wrote:

> In D144509#4350052 <https://reviews.llvm.org/D144509#4350052>, @Mordante wrote:
>
>> In D144509#4349921 <https://reviews.llvm.org/D144509#4349921>, @thakis wrote:
>>
>>> Reverted this and follow-ups in d763c6e5e2d0a6b34097aa7dabca31e9aff9b0b6 <https://reviews.llvm.org/rGd763c6e5e2d0a6b34097aa7dabca31e9aff9b0b6> for now.
>>>
>>> Sorry this is such a pain to land :(
>>>
>>> (See also discussion over in D150688 <https://reviews.llvm.org/D150688>)
>>
>> I'm not happy that the patch needs to be reverted again.
>>
>> It has taken me a lot of time to contact all buildbots maintainers to get all buildbots updated to the minimal CMake requirement.
>
> I sympathize with this, but I still believe reverting in these situations is the right thing to do. It reduces disruption for everyone who needs their builds to keep working, while allowing the failures to be investigated without the pressure of knowing that head is currently broken. That this patch has been hard to land is in the nature of the change itself.

I'm caught quite off-guard that it seems the LLVM buildbots don't cover our Windows support that well. It would also be great to know how we can improve the process to update dependency versions. The current way does not really encourage me to propose LLVM wide updates again.

>> Now that they are updated it turned out that one of the two last updated bots has an issue with this patch and that has been fixed. But now it seems to break Chromium. I don't have access to Windows so I don't know how I can test patches.
>
> I'm happy to test patches on my Windows machine.

Thanks!

>> Do you have a suggestion how we can move this patch forward?
>
> IIRC, D150688 <https://reviews.llvm.org/D150688> + the diff in https://github.com/llvm/llvm-project/issues/62719#issuecomment-1552903385 + upgrading the pre-merge linux bot should take care of all known issues.

Would it make sense to put all these patches in one new review and then test that on Chromium and ask @glandium to test that too. Then we know whether it solves the issues. Do you want me to make a patch or do you want to do it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144509



More information about the cfe-commits mailing list