Page MenuHomePhabricator

[CMake] Add a warning message to prepare the upcoming upgrade to CMake 3.13.4
ClosedPublic

Authored by ldionne on Apr 8 2020, 9:35 AM.

Details

Summary

As discussed in http://lists.llvm.org/pipermail/llvm-dev/2020-March/140349.html,
the minimum version of CMake required to build LLVM will be upgraded to
3.13.4 right after we create the release branch for LLVM 11.0.0.

As part of this effort, this commit adds a warning to give a heads up
to folks regarding the upcoming upgrade. This should allow users to
upgrade their CMake in advance so that the upgrade can sail right
through when the time comes.

Diff Detail

Event Timeline

ldionne created this revision.Apr 8 2020, 9:35 AM

Not sure who to add -- I'm sending an email to the list with a link to this review.

I don't think there's any point in delaying until after the 11 release. We just branched LLVM 10. I believe we should make the change between now and the September LLVM 11 release, rather than afterwards.

I'd propose instead:

  1. Add a warning like this -- but saying the minimum will be upgraded SOON, not in 11.0.0.
  2. Keep the warning for a month -- meanwhile work on getting all buildbots upgraded
  3. Promote the warning to an error, make sure buildbots are still working. Potentially revert and re-apply later, if there are issues.
  4. Start cleaning up cruft for compat with old versions, yay!
hubert.reinterpretcast added inline comments.
llvm/CMakeLists.txt
8

This message refers to "internal process" concepts such as creation of a release branch. Can the message be made to be clear for "downstream consumers"?

This looks good to me. As I mentioned in the thread, it would probably make sense to at least upgrade to 3.8 now in order to use C++17 without hacking the build system.

mehdi_amini added inline comments.
llvm/CMakeLists.txt
8

I would write "Starting with 11.0, the minimum version of CMake required ...."

I don't think there's any point in delaying until after the 11 release. We just branched LLVM 10. I believe we should make the change between now and the September LLVM 11 release, rather than afterwards.

We actually just *released* LLVM 10, we branched in January I believe and the next branching point is July (I think?). So if we can wait three months, we'd have the warnings in a released version.

Second to what jyknight has said.

Add a warning like this -- but saying the minimum will be upgraded SOON, not in 11.0.0.

ldionne marked an inline comment as done.Apr 8 2020, 11:53 AM

I don't think there's any point in delaying until after the 11 release. We just branched LLVM 10. I believe we should make the change between now and the September LLVM 11 release, rather than afterwards.

I'd strongly support that, however can you please mention that in the llvm-dev thread? This isn't what we agreed on. I think this might get consensus as well, but I can't really implement a resolution that's different from what has been agreed on the list.

So, as it stands, I'll have to keep the warning as-is except for mentioning an approximate branching date and other minor rewordings.

llvm/CMakeLists.txt
8

@hubert.reinterpretcast

This message refers to "internal process" concepts such as creation of a release branch. Can the message be made to be clear for "downstream consumers"?

The reason why I didn't say a date is that I wasn't able to find the (approximate) date at which we'd branch off LLVM 11.0.0. It looks like this is around September, however, so I'll mention that if we keep the warning similar (see other threads).

@mehdi_amini

I would write "Starting with 11.0, the minimum version of CMake required ...."

It does read better, however it's not really what we mean. We'd have to say "Starting with 12.0, the minimum ...", which would be clear to downstream users of LLVM, but not to developers who will want to know that we're going to bump as soon as 11.0 is branched, even though 12.0 hasn't been released yet. I guess we could also lie and say "Starting with 11.0, ..." -- this would be clear to developers, and downstream users would think they need to upgrade when they could actually afford to wait one more release (which is probably OK).

smeenai added a comment.EditedApr 8 2020, 11:58 AM

I don't think there's any point in delaying until after the 11 release. We just branched LLVM 10. I believe we should make the change between now and the September LLVM 11 release, rather than afterwards.

I'd propose instead:

  1. Add a warning like this -- but saying the minimum will be upgraded SOON, not in 11.0.0.
  2. Keep the warning for a month -- meanwhile work on getting all buildbots upgraded
  3. Promote the warning to an error, make sure buildbots are still working. Potentially revert and re-apply later, if there are issues.
  4. Start cleaning up cruft for compat with old versions, yay!

I believe the 3.13.4 minimum is predicated on the Ubuntu 20.04 LTS release, which will have CMake 3.16.3 available, at which point Debian 10 (which has CMake 3.13.4) will be our limiting OS. (The current Ubuntu LTS, 18.04, has CMake 3.10.2.)

smeenai added inline comments.Apr 8 2020, 12:00 PM
llvm/CMakeLists.txt
8

The branch date is usually mid-July, and the release is late August/early September. I assume the branch date is what's relevant here.

Adding more CMake people, but I'm all for this.

llvm/CMakeLists.txt
8

The branch for 10.0 was created in January. The branch for 11.0 is likely to be created in July (six months since the last branch). The usual delay between branching and release is expected to put the release at September.

I believe the "12.0" version of the wording is less likely to cause confusion if the 11.0 release is expected to be an opportunity for downstream users to learn of the planned change. If that's the direction, then we might want to put something into the 11.0 release notes to highlight the plan.

smeenai added inline comments.Apr 8 2020, 12:02 PM
llvm/CMakeLists.txt
7

How about "Your CMake version" or "The CMake version you're using" or something like that, instead of "The current CMake version"? To me, "The current CMake version" is ambiguous – it could mean *your* current CMake version, or it could mean the minimum CMake version currently required by LLVM.

phosek added a comment.Apr 8 2020, 3:22 PM

The way I interpreted the email thread is that we would bump the minimum CMake version to 3.13.4 after the the 11.0 branch which will be presumably in July.

I'm also fine with updating now, but if we do that we need to decide whether we're going to upgrade to 3.10.2 immediately or wait 15 days until the Ubuntu 20.04 LTS release and then move immediately to 3.13.4. However, that might be a bit too aggressive: it's unlikely that everyone is on going to upgrade on day one.

We could also consider raising the minimum version now to 3.10.2, and then raise it again to 3.13.4 after the 11.0 branch, which gives everyone ~3 months to upgrade.

The way I interpreted the email thread is that we would bump the minimum CMake version to 3.13.4 after the the 11.0 branch which will be presumably in July.

I'm also fine with updating now, but if we do that we need to decide whether we're going to upgrade to 3.10.2 immediately or wait 15 days until the Ubuntu 20.04 LTS release and then move immediately to 3.13.4. However, that might be a bit too aggressive: it's unlikely that everyone is on going to upgrade on day one.

We could also consider raising the minimum version now to 3.10.2, and then raise it again to 3.13.4 after the 11.0 branch, which gives everyone ~3 months to upgrade.

I am in favor of doing this. Bumping now to 3.10.2 and then bump to 3.13.4 after the 11.0 branch.

mehdi_amini added inline comments.Apr 8 2020, 6:12 PM
llvm/CMakeLists.txt
8

@ldionne : I meant 12.0 here indeed.

The way I interpreted the email thread is that we would bump the minimum CMake version to 3.13.4 after the the 11.0 branch which will be presumably in July.

I'm also fine with updating now, but if we do that we need to decide whether we're going to upgrade to 3.10.2 immediately or wait 15 days until the Ubuntu 20.04 LTS release and then move immediately to 3.13.4. However, that might be a bit too aggressive: it's unlikely that everyone is on going to upgrade on day one.

We could also consider raising the minimum version now to 3.10.2, and then raise it again to 3.13.4 after the 11.0 branch, which gives everyone ~3 months to upgrade.

I am in favor of doing this. Bumping now to 3.10.2 and then bump to 3.13.4 after the 11.0 branch.

How much churn do you think two CMake upgrades, one shortly after the other, will cause, vs. a single upgrade? Is it worth the benefits? Also, what's the best way to communicate that the LLVM 11 release will up the CMake requirement?

If we decide to go down this route, we should probably do something like the toolchain version checks, where they have a minimum version and a soft error version. In our case here, the minimum would be 3.10.2 and the soft error version would be 3.13.4.

Honestly, I would either upgrade once soon, or upgrade once later after we branch for 11.0.0 (as we agreed on the list). Doing two CMake upgrades back-to-back doesn't sound fun or like a good use of everybody's time.

The way I interpreted the email thread is that we would bump the minimum CMake version to 3.13.4 after the the 11.0 branch which will be presumably in July.

I'm also fine with updating now, but if we do that we need to decide whether we're going to upgrade to 3.10.2 immediately or wait 15 days until the Ubuntu 20.04 LTS release and then move immediately to 3.13.4. However, that might be a bit too aggressive: it's unlikely that everyone is on going to upgrade on day one.

We could also consider raising the minimum version now to 3.10.2, and then raise it again to 3.13.4 after the 11.0 branch, which gives everyone ~3 months to upgrade.

I am in favor of doing this. Bumping now to 3.10.2 and then bump to 3.13.4 after the 11.0 branch.

How much churn do you think two CMake upgrades, one shortly after the other, will cause, vs. a single upgrade? Is it worth the benefits? Also, what's the best way to communicate that the LLVM 11 release will up the CMake requirement?

I didn't think it would be all that much churn. You could bump trunk to 3.10.2, but then bump cmake on all the builders to 3.13.4, so that you only need to update the builders once. That seems like the hardest part to me. But really if you think it's too much extra work, I'm fine just updating once.

If we decide to go down this route, we should probably do something like the toolchain version checks, where they have a minimum version and a soft error version. In our case here, the minimum would be 3.10.2 and the soft error version would be 3.13.4.

I'll give others a bit more to voice opinions, but I'm completely in support of this plan and delighted to see the CMake upgrade going forward.

The comments about the message need to be addressed.

smeenai accepted this revision.Apr 20 2020, 3:02 PM

LGTM with the comments about the message addressed.

This revision is now accepted and ready to land.Apr 20 2020, 3:02 PM
ldionne updated this revision to Diff 259292.Apr 22 2020, 8:00 AM

Update wording.

I didn't think it would be all that much churn. You could bump trunk to 3.10.2, but then bump cmake on all the builders to 3.13.4, so that you only need to update the builders once. That seems like the hardest part to me. But really if you think it's too much extra work, I'm fine just updating once.

The churn isn't in actually doing the upgrade, but in having a discussion on llvm-dev. Concretely, we could probably bump to 3.8 today and nobody would even notice since pretty much all build bots have been updated AFAICT. But having a 2nd discussion on llvm-dev is what I'd like to avoid.

LGTM with the comments about the message addressed.

Sorry for the delay. I've addressed the comments about the message.

I caught up on the llvm-dev thread and it seems like nobody's opposed to my conservative proposal, so I'll move forward with this patch now.

Sorry for the delay. I've addressed the comments about the message.

I caught up on the llvm-dev thread and it seems like nobody's opposed to my conservative proposal, so I'll move forward with this patch now.

I think the new wording should satisfy everyone based on the comments.

This revision was automatically updated to reflect the committed changes.