[Suggestion] swift/utils/update-checkout should keep llvm/clang up-to-date


(Lily Ballard) #1

I was surprised to discover that swift/utils/update-checkout doesn't update LLVM/clang by default. Looking at the script, it supports a --all flag to update those (and llbuild too), but that led to the second surprise, which is that the script doesn't make any attempt to ensure it's checking out a compatible version of llvm/clang, it just runs `git fetch` and `git rebase FETCH_HEAD`.

Given that we're targeting specific stable snapshots of llvm/clang, and that we shouldn't be updating our local versions for new stable versions until it's actually been validated and local changes made if necessary, it seems to me that the script should actually know what specific versions of llvm/clang we want to be using, and should always update to those versions. It could have a separate flag to skip llvm/clang (instead of a flag to include them) for use by people who are intentionally testing newer versions, but the default behavior should update it.

-Kevin Ballard


(Michael Gottesman) #2

I was surprised to discover that swift/utils/update-checkout doesn't update LLVM/clang by default. Looking at the script, it supports a --all flag to update those (and llbuild too), but that led to the second surprise, which is that the script doesn't make any attempt to ensure it's checking out a compatible version of llvm/clang, it just runs `git fetch` and `git rebase FETCH_HEAD`.

We ensure that the trunk stable clang/llvm branches are always in sync. So just rebasing head should be sufficient.

···

On Feb 8, 2016, at 3:09 PM, Kevin Ballard via swift-dev <swift-dev@swift.org> wrote:

Given that we're targeting specific stable snapshots of llvm/clang, and that we shouldn't be updating our local versions for new stable versions until it's actually been validated and local changes made if necessary, it seems to me that the script should actually know what specific versions of llvm/clang we want to be using, and should always update to those versions. It could have a separate flag to skip llvm/clang (instead of a flag to include them) for use by people who are intentionally testing newer versions, but the default behavior should update it.

-Kevin Ballard
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Lily Ballard) #3

>
> I was surprised to discover that swift/utils/update-checkout doesn't update LLVM/clang by default. Looking at the script, it supports a --all flag to update those (and llbuild too), but that led to the second surprise, which is that the script doesn't make any attempt to ensure it's checking out a compatible version of llvm/clang, it just runs `git fetch` and `git rebase FETCH_HEAD`.

We ensure that the trunk stable clang/llvm branches are always in sync. So just rebasing head should be sufficient.

Ah right, the llvm/clang checkouts are to copies of llvm/clang instead of to the original.

In that case, why doesn't the update-checkout script update llvm/clang by default? The only reason I can think of to not update it (besides for when you're intentionally targeting a non-stable version of llvm/clang because you're working on migrating it) is to avoid having to recompile llvm/clang if a backwards-compatible change happens to llvm/clang, but I assume the llvm/clang repos aren't updated very often, and when they are updated we all should probably update our checkouts of it even if we don't get compilation errors.

-Kevin Ballard

···

On Mon, Feb 8, 2016, at 03:33 PM, Michael Gottesman wrote:

> On Feb 8, 2016, at 3:09 PM, Kevin Ballard via swift-dev <swift-dev@swift.org> wrote:

> Given that we're targeting specific stable snapshots of llvm/clang, and that we shouldn't be updating our local versions for new stable versions until it's actually been validated and local changes made if necessary, it seems to me that the script should actually know what specific versions of llvm/clang we want to be using, and should always update to those versions. It could have a separate flag to skip llvm/clang (instead of a flag to include them) for use by people who are intentionally testing newer versions, but the default behavior should update it.
>
> -Kevin Ballard
> _______________________________________________
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev


(Michael Gottesman) #4

I was surprised to discover that swift/utils/update-checkout doesn't update LLVM/clang by default. Looking at the script, it supports a --all flag to update those (and llbuild too), but that led to the second surprise, which is that the script doesn't make any attempt to ensure it's checking out a compatible version of llvm/clang, it just runs `git fetch` and `git rebase FETCH_HEAD`.

We ensure that the trunk stable clang/llvm branches are always in sync. So just rebasing head should be sufficient.

Ah right, the llvm/clang checkouts are to copies of llvm/clang instead of to the original.

In that case, why doesn't the update-checkout script update llvm/clang by default? The only reason I can think of to not update it (besides for when you're intentionally targeting a non-stable version of llvm/clang because you're working on migrating it) is to avoid having to recompile llvm/clang if a backwards-compatible change happens to llvm/clang, but I assume the llvm/clang repos aren't updated very often, and when they are updated we all should probably update our checkouts of it even if we don't get compilation errors.

That is most likely an oversight. Pull request and assign to me?

Michael

···

On Feb 8, 2016, at 3:54 PM, Kevin Ballard <kevin@sb.org> wrote:
On Mon, Feb 8, 2016, at 03:33 PM, Michael Gottesman wrote:

On Feb 8, 2016, at 3:09 PM, Kevin Ballard via swift-dev <swift-dev@swift.org> wrote:

-Kevin Ballard

Given that we're targeting specific stable snapshots of llvm/clang, and that we shouldn't be updating our local versions for new stable versions until it's actually been validated and local changes made if necessary, it seems to me that the script should actually know what specific versions of llvm/clang we want to be using, and should always update to those versions. It could have a separate flag to skip llvm/clang (instead of a flag to include them) for use by people who are intentionally testing newer versions, but the default behavior should update it.

-Kevin Ballard
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev


(Jordan Rose) #5

IIRC Dmitri was concerned about doing more serialized network requests on slow connections.

Jordan

···

On Feb 8, 2016, at 16:34, Michael Gottesman via swift-dev <swift-dev@swift.org> wrote:

On Feb 8, 2016, at 3:54 PM, Kevin Ballard <kevin@sb.org> wrote:

On Mon, Feb 8, 2016, at 03:33 PM, Michael Gottesman wrote:

On Feb 8, 2016, at 3:09 PM, Kevin Ballard via swift-dev <swift-dev@swift.org> wrote:

I was surprised to discover that swift/utils/update-checkout doesn't update LLVM/clang by default. Looking at the script, it supports a --all flag to update those (and llbuild too), but that led to the second surprise, which is that the script doesn't make any attempt to ensure it's checking out a compatible version of llvm/clang, it just runs `git fetch` and `git rebase FETCH_HEAD`.

We ensure that the trunk stable clang/llvm branches are always in sync. So just rebasing head should be sufficient.

Ah right, the llvm/clang checkouts are to copies of llvm/clang instead of to the original.

In that case, why doesn't the update-checkout script update llvm/clang by default? The only reason I can think of to not update it (besides for when you're intentionally targeting a non-stable version of llvm/clang because you're working on migrating it) is to avoid having to recompile llvm/clang if a backwards-compatible change happens to llvm/clang, but I assume the llvm/clang repos aren't updated very often, and when they are updated we all should probably update our checkouts of it even if we don't get compilation errors.

That is most likely an oversight. Pull request and assign to me?


(Lily Ballard) #6

The script already updates 9 repositories by default, including the
various corelibs repos. I'd say it's worth updating llvm and clang too,
since having an out-of-date llvm or clang can prevent swift from
building. I'm not sure about llbuild, since I don't really know much
about it or how often it updates, but I'm tempted to just get rid of the
--all flag entirely. I'll submit a PR.

-Kevin Ballard

···

On Mon, Feb 8, 2016, at 05:07 PM, Jordan Rose wrote:

On Feb 8, 2016, at 16:34, Michael Gottesman via swift-dev <swift- >> dev@swift.org> wrote:

On Feb 8, 2016, at 3:54 PM, Kevin Ballard <kevin@sb.org> wrote:

On Mon, Feb 8, 2016, at 03:33 PM, Michael Gottesman wrote:

On Feb 8, 2016, at 3:09 PM, Kevin Ballard via swift-dev <swift- >>>>> dev@swift.org> wrote:

I was surprised to discover that swift/utils/update-checkout
doesn't update LLVM/clang by default. Looking at the script, it
supports a --all flag to update those (and llbuild too), but that
led to the second surprise, which is that the script doesn't make
any attempt to ensure it's checking out a compatible version of
llvm/clang, it just runs `git fetch` and `git rebase FETCH_HEAD`.

We ensure that the trunk stable clang/llvm branches are always in
sync. So just rebasing head should be sufficient.

Ah right, the llvm/clang checkouts are to copies of llvm/clang
instead of to the original.

In that case, why doesn't the update-checkout script update
llvm/clang by default? The only reason I can think of to not update
it (besides for when you're intentionally targeting a non-stable
version of llvm/clang because you're working on migrating it) is to
avoid having to recompile llvm/clang if a backwards-compatible
change happens to llvm/clang, but I assume the llvm/clang repos
aren't updated very often, and when they are updated we all should
probably update our checkouts of it even if we don't get compilation
errors.

That is most likely an oversight. Pull request and assign to me?

IIRC Dmitri was concerned about doing more serialized network requests
on slow connections.

Jordan