Protobuf in Bazel: let's use a submodule

[Carmi Grushko]

Hi,

Currently we have a patched 3.0.0 protobuf in //third_party/protobuf/, with some redirection aliases that were required when we had checked-in binaries.

Since https://github.com/google/protobuf now builds with Bazel, we can have it as a submodule in Bazel's repo. 

This will make it easy to upgrade it to a newer version - we won't need the instructions in https://github.com/bazelbuild/bazel/blob/master/third_party/protobuf/README.md, just a simple pull.

What do you think?

[Brian Silverman]

I dislike submodules, but I think git-subtree sounds like a good idea. It still gives you the "single command pull" functionality and automatically tracks what came from upstream, and it provides a much simpler workflow for everybody besides the person doing the updates.

If we go with git-subtree, I'm not sure whether it makes sense to use --squash or not here. Using --squash means upstream protobuf commits aren't recorded in the bazel repository's history, which can be good or bad depending on what you want to do.

Why your company shouldn’t use Git submodules goes over a lot of the reasons I dislike submodules (although it misses how easy `git subtree split` makes pushing changes back upstream). This list from here has some reasons too:

• conflicts on submodules
• complex handling in scripts
• submodules stopping tracking branches
• bunch of useless bump commits for updating a submodule
• complex removal of a submodule
• GUI tools not handling submodules properly
• more complex github & jenkins hooks
• commiting a not yet merged submodule branch ref
• teaching everyone in the team about submodules

It's possible to work around all of them, but I found it really annoying and not worth it. The only real criticism I've seen of git-subtree is having two copies of the files, which seems more like an advantage to me (don't have to worry about coordinate two copies).

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Damien Martin-guillerez]

Hi,

git subtree seems like the ideal option if we squash commit. IIUC we actually have to do nothing (except maybe change a bit the layout to avoid encoding the version of the protobuf library and updating the README file on how to update the dependency).

If we do that I would be in favor of using a local_repository from the parent workspace that point to that directory now that it works just fine.

Definitely +100 for reducing the patches to protobuf sources and have the same version as upstream.

On Tue, Feb 7, 2017 at 5:23 AM Brian Silverman <bsilve...@gmail.com> wrote:

I dislike submodules, but I think git-subtree sounds like a good idea. It still gives you the "single command pull" functionality and automatically tracks what came from upstream, and it provides a much simpler workflow for everybody besides the person doing the updates.

If we go with git-subtree, I'm not sure whether it makes sense to use --squash or not here. Using --squash means upstream protobuf commits aren't recorded in the bazel repository's history, which can be good or bad depending on what you want to do.

Why your company shouldn’t use Git submodules goes over a lot of the reasons I dislike submodules (although it misses how easy `git subtree split` makes pushing changes back upstream). This list from here has some reasons too:

• conflicts on submodules
• complex handling in scripts
• submodules stopping tracking branches
• bunch of useless bump commits for updating a submodule
• complex removal of a submodule
• GUI tools not handling submodules properly
• more complex github & jenkins hooks
• commiting a not yet merged submodule branch ref
• teaching everyone in the team about submodules

It's possible to work around all of them, but I found it really annoying and not worth it. The only real criticism I've seen of git-subtree is having two copies of the files, which seems more like an advantage to me (don't have to worry about coordinate two copies).

On Tue, Feb 7, 2017 at 3:26 AM, 'Carmi Grushko' via bazel-discuss <bazel-...@googlegroups.com> wrote:

Hi,

Currently we have a patched 3.0.0 protobuf in //third_party/protobuf/, with some redirection aliases that were required when we had checked-in binaries.

Since https://github.com/google/protobuf now builds with Bazel, we can have it as a submodule in Bazel's repo. 

This will make it easy to upgrade it to a newer version - we won't need the instructions in https://github.com/bazelbuild/bazel/blob/master/third_party/protobuf/README.md, just a simple pull.

What do you think?

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CAP01z6LxC2x51B9MkOb7n_cDYapz1WX6XFh2ELj-vDoN2VY3pw%40mail.gmail.com.

[Carmi Grushko]

+aehlig

Ok, subtrees it is.
I have 2 questions -

1. //third_party:srcs includes //third_party/protobuf:srcs, which is problematic because third_party/protobuf/BUILD now comes from protobuf. I can add it, but it’ll make future pulls from protobuf harder. Do we need it?
2. The URL of the upstream protobuf repo isn’t written anywhere, so a future maintainer will have to remember the URL when pulling from upstream. An easy solution is to put the pull command in a script. Damien, where do you want the script to be?

​

On Tue, Feb 7, 2017 at 2:21 AM, Damien Martin-guillerez <dmar...@google.com> wrote:

Hi,

git subtree seems like the ideal option if we squash commit. IIUC we actually have to do nothing (except maybe change a bit the layout to avoid encoding the version of the protobuf library and updating the README file on how to update the dependency).

If we do that I would be in favor of using a local_repository from the parent workspace that point to that directory now that it works just fine.

Definitely +100 for reducing the patches to protobuf sources and have the same version as upstream.

On Tue, Feb 7, 2017 at 5:23 AM Brian Silverman <bsilve...@gmail.com> wrote:

I dislike submodules, but I think git-subtree sounds like a good idea. It still gives you the "single command pull" functionality and automatically tracks what came from upstream, and it provides a much simpler workflow for everybody besides the person doing the updates.

If we go with git-subtree, I'm not sure whether it makes sense to use --squash or not here. Using --squash means upstream protobuf commits aren't recorded in the bazel repository's history, which can be good or bad depending on what you want to do.

Why your company shouldn’t use Git submodules goes over a lot of the reasons I dislike submodules (although it misses how easy `git subtree split` makes pushing changes back upstream). This list from here has some reasons too:

• conflicts on submodules
• complex handling in scripts
• submodules stopping tracking branches
• bunch of useless bump commits for updating a submodule
• complex removal of a submodule
• GUI tools not handling submodules properly
• more complex github & jenkins hooks
• commiting a not yet merged submodule branch ref
• teaching everyone in the team about submodules

It's possible to work around all of them, but I found it really annoying and not worth it. The only real criticism I've seen of git-subtree is having two copies of the files, which seems more like an advantage to me (don't have to worry about coordinate two copies).

On Tue, Feb 7, 2017 at 3:26 AM, 'Carmi Grushko' via bazel-discuss <bazel-discuss@googlegroups.com> wrote:

Hi,

Currently we have a patched 3.0.0 protobuf in //third_party/protobuf/, with some redirection aliases that were required when we had checked-in binaries.

Since https://github.com/google/protobuf now builds with Bazel, we can have it as a submodule in Bazel's repo. 

This will make it easy to upgrade it to a newer version - we won't need the instructions in https://github.com/bazelbuild/bazel/blob/master/third_party/protobuf/README.md, just a simple pull.

What do you think?

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.

[Damien Martin-guillerez]

1. Yes we need it to do integration tests and to generate a distribution artifact, can we just upstream it?

2. We put those kind of scripts in scripts/ (so surprising :))

On Tue, Feb 7, 2017 at 3:26 AM, 'Carmi Grushko' via bazel-discuss <bazel-...@googlegroups.com> wrote:

Hi,

Currently we have a patched 3.0.0 protobuf in //third_party/protobuf/, with some redirection aliases that were required when we had checked-in binaries.

Since https://github.com/google/protobuf now builds with Bazel, we can have it as a submodule in Bazel's repo. 

This will make it easy to upgrade it to a newer version - we won't need the instructions in https://github.com/bazelbuild/bazel/blob/master/third_party/protobuf/README.md, just a simple pull.

What do you think?

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

[Carmi Grushko]

I doubt we can upstream it, what's their interest in it?

How well does pull from upstream work if we have our local additions to their files?

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.

[Damien Martin-guillerez]

Well their interest is supporting our use case :) Klaus also suggested that instead of using local_repository we use new_local_repository using a custom build file so we can patch it on the fly.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

[Kamal Marhubi]

On Tue, Feb 7, 2017, 02:21 'Damien Martin-guillerez' via bazel-discuss <bazel-...@googlegroups.com> wrote:

If we do that I would be in favor of using a local_repository from the parent workspace that point to that directory now that it works just fine.

As an aside, does this mean that relative paths work for local repositories now?

[Kristina Chodorow]

On Thu, Feb 9, 2017 at 12:43 PM, Kamal Marhubi <ka...@marhubi.com> wrote:

On Tue, Feb 7, 2017, 02:21 'Damien Martin-guillerez' via bazel-discuss <bazel-discuss@googlegroups.com> wrote:

If we do that I would be in favor of using a local_repository from the parent workspace that point to that directory now that it works just fine.

As an aside, does this mean that relative paths work for local repositories now?

Yes, they have for several releases. 

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CAK-ZPek3FrCQOYR5kdXoa6HDLeTvM%3Da3LMa31JxMEZ4adUDuTw%40mail.gmail.com.

[Kamal Marhubi]

Thanks, I'll check them out! I really should read the changelog since ~0.3 which is when I last properly used bazel.

</aside> :-)

On Thu, Feb 9, 2017 at 12:45 PM Kristina Chodorow <kcho...@google.com> wrote:

On Thu, Feb 9, 2017 at 12:43 PM, Kamal Marhubi <ka...@marhubi.com> wrote:

On Tue, Feb 7, 2017, 02:21 'Damien Martin-guillerez' via bazel-discuss <bazel-...@googlegroups.com> wrote:

If we do that I would be in favor of using a local_repository from the parent workspace that point to that directory now that it works just fine.

As an aside, does this mean that relative paths work for local repositories now?

Yes, they have for several releases. 

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

[Carmi Grushko]

+kchodorow

I ran into some trouble pushing my commits to Gerrit.
After deleting the original third_party/protobuf/ directory, I ran

git subtree add --prefix third_party/protobuf https://github.com/google/protobuf master --squash

which resulted in 2 extra automatic commits:

commit 6e55bc9e0ef6204380d6556a4019b3c3a3abd3c4
Merge: 5bf837d75 f6f499fcc
Author: Carmi Grushko <ca...@google.com>
Date:   Wed Feb 22 11:25:49 2017 -0500

    Merge commit 'f6f499fcc25af8b0337cce3e4fcde6c071b3867e' as 'third_party/protobuf'

commit f6f499fcc25af8b0337cce3e4fcde6c071b3867e
Author: Carmi Grushko <ca...@google.com>
Date:   Wed Feb 22 11:25:49 2017 -0500

    Squashed 'third_party/protobuf/' content from commit 1a8cbfd

    git-subtree-dir: third_party/protobuf
    git-subtree-split: 1a8cbfd355603e094858cbfdf5999b860dbab13f

commit 5bf837d759f6bd1bcda797f8cc117db210afa20f
Author: Carmi Grushko <ca...@google.com>
Date:   Wed Feb 22 11:25:35 2017 -0500

    Delete third_party/protobuf/, in preparation for adding it as a subtree of https://github.com/google/protobuf

    Change-Id: I06885207980212de8e5bbc1e6f8bda35c4816e36

The problem is that the 2 top commits don’t have a Change-Id, so I can’t push them to Gerrit for review.

Usually you can change the commit message using git rebase, but in this case I’m getting weird merge conflicts.
Looks like I’m not the only one: http://stackoverflow.com/q/40297499/1071136

Any ideas?
I can simply clone the protobuf repo and then our update process will be “delete the directory, clone again”.

​

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.

[Damien Martin-guillerez]

That's because git subtree generate a merge commit (so rebasing is tricky). That's shouldn't be a problem for merging but that will be a problem for sending the change for review.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

[Carmi Grushko]

Ok, what do I do?

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discuss+unsubscribe@googlegroups.com.

[Damien Martin-guillerez]

Let me try it out on my machine

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CANdYFJ7HkUW4BEnaZDXQe4Fjs4gWBNLbhZj_GOvHbSBxjNKcdw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.

To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

[Damien Martin-guillerez]

I am also a bit at lost, I need to look a bit more at git subtree but it seems like it add a full branch all the time...