Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove bazel.rb Formula #31396

Closed
wants to merge 2 commits into from
Closed

Remove bazel.rb Formula #31396

wants to merge 2 commits into from

Conversation

buchgr
Copy link

@buchgr buchgr commented Aug 23, 2018

The Bazel team has decided to move to their own Homebrew tap,
as outlined on our blog https://blog.bazel.build/2018/08/22/bazel-homebrew.html

We aren't able to maintain the Homebrew core formula any longer
and would thus prefer to have it removed.

cc @JCount

The Bazel team has decided to move to their own Homebrew tap,
as outlined on our blog https://blog.bazel.build/2018/08/22/bazel-homebrew.html

We aren't able to maintain the Homebrew core formula any longer
and would thus prefer to have it removed.
@commitay
Copy link
Contributor

bazel is a build dep of libtensorflow

depends_on "bazel" => :build

@commitay commitay added the maintainer feedback Additional maintainers' opinions may be needed label Aug 23, 2018
@DomT4
Copy link
Member

DomT4 commented Aug 24, 2018

To be honest I'm not wild about the way this has been handled by y'all upstream. By moving unilaterally rather than cooperatively with us at Homebrew you've created a pretty messy situation that now has to be a priority for the core team. I completely & wholly appreciate you've got priorities upstream as well but I can't say I appreciate how little you've valued the time of Homebrew's maintainers by dumping this on us essentially as an instruction rather than a discussion.

In the last 90 days there have been 28,153 installations of bazel and 3,501 of libtensorflow, which depends on bazel. These are not insignificant numbers and unilateral moves are the kind of things that can start to rapidly generate support requests/bug reports/confused tweets/etc for us. I will move onto the practical steps we can take here, but I really wish there had been more of an effort to handle this with Homebrew.

So. We can actually make the transition to a new tap a pretty painless experience for existing users, and indeed, anyone who types in brew install bazel without having your tap installed at a later date. That's a process we did just the other day for Amber after they requested to take ownership of the formula. That negates the need for blog posts which only a slim portion of people are ever likely to go looking for.

libtensorflow, assuming you don't want to adopt it, will just have to be deleted. That's a sucky experience for existing users but we don't have a lot of choice at this point.

Those are the steps this PR should take; formal migration & the deletion of libtensorflow.

@@ -9,6 +9,7 @@
"auctex": "homebrew/tex",
"avidemux": "homebrew/cask",
"awsenv": "Luzifer/tools",
"bazel": "bazelbuild/tap"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this line is missing a comma at the end.

@buchgr
Copy link
Author

buchgr commented Aug 24, 2018

Hi @DomT4,

I'd like to apologize for any confusion / pain that I have caused. It was certainly not my intention. I admittedly have been in firefighting mode for the past three weeks and didn't have any experience with homebrew when I started to work on this. I am not entirely sure what we could have done differently but if you are still open to it I am happy to discuss how we can remain in homebrew core or do a smooth migration and not cause any pain to homebrew users / maintainers.

How is maintaining libtensorflow and how can I reach out to that person?

Best,
Jakob

@buchgr
Copy link
Author

buchgr commented Aug 24, 2018

@commitay are all build deps required to be in homebrew core? Could we instead update the formula to download the Bazel binary?

@DomT4
Copy link
Member

DomT4 commented Aug 24, 2018

I am not entirely sure what we could have done differently

Just a discussion in advance to try and work out a path forwards, or at least ensure a smooth transition, really.

As things stand I can tell you that there have been nearly 5x more installations of bazel from homebrew/core in the last two days than from bazelbuild/tap, and the longer the status quo exists the more the "bazel via Homebrew" userbase is going to be split, which isn't an ideal experience for anyone involved.

We support tap migrations, where necessary/desirable, but we try to have this kind of formal process to it because otherwise these splits happen & it gets... messy, for lack of a more technical phrase.

I am happy to discuss how we can remain in homebrew core or do a smooth migration and not cause any pain to homebrew users / maintainers.

I don't think we can bend as far as supporting the full binary installation. We'd accept something like using the binary to bootstrap a build, we do something similar with things like go, crystal and sbcl already, but I think that's about as far as we could stretch on that, which I assume is a stretch not quite far enough for upstream. If that is the case, formal migration is the only path available.

How is maintaining libtensorflow and how can I reach out to that person?

We don't have specific formulae maintainers, generally speaking. There's a team of core maintainers, a team of brew maintainers, and some who are both, myself & @commitay included. If folks are in agreement migration is the path forwards I guess for libtensorflow either we delete it or we use a binary bazel, available only during the compile process, to install it.

are all build deps required to be in homebrew core?

Yes. All mandatory dependencies are required to exist in core.

@buchgr
Copy link
Author

buchgr commented Aug 24, 2018

Just a discussion in advance to try and work out a path forwards, or at least ensure a smooth transition, really.

I am sorry about that. I clearly fucked up here. Please accept my apology.

I don't think we can bend as far as supporting the full binary installation.

For us the issue is really that we need to ship a (minimal) embedded JDK that Bazel itself runs on. We'd like to make the fact that Bazel is written in Java an implementation detail and by controlling the JDK also ensure proper testing.

So I think at this point the only path forward is to do a formal migration. I'd prefer not to break libtensorflow users and so in this PR can I update the libtensorflow formula to download a Bazel binary during the compile process and remove the build dependency on Bazel?

Thanks!

@MikeMcQuaid
Copy link
Member

In the last 90 days there have been 28,153 installations of bazel

Given this I'm strongly 👎 on migrating this formula to a tap for discovery reasons. If you're unable to maintain it in this repository that's fine; the community will continue to do so.

@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented Aug 24, 2018

To be more explicit: this would make the 104th most popular formula in Homebrew (in the last 90 days) harder for users to discover and install for no benefit to us.

For us the issue is really that we need to ship a (minimal) embedded JDK that Bazel itself runs on. We'd like to make the fact that Bazel is written in Java an implementation detail and by controlling the JDK also ensure proper testing.

As a side note I'm seeing more and more projects taking this approach and I'd like to say as a long-time package manager maintainer (and even longer package manager user): it's a mistake. Imagine if every tool that used the JVM insisted on shipping their own version. This wastes time downloading, compiling and storing it and means every individual project needs to bump the JVM version on a security update. Homebrew is considerably more liberal than most package managers on this front and if we're saying it's a bad idea: good luck convincing e.g. Debian/Ubuntu to do it.

@buchgr
Copy link
Author

buchgr commented Aug 24, 2018

Given this I'm strongly -1 on migrating this formula to a tap for discovery reasons.

It's exactly for this reason that we would like to ensure a great experience for Bazel in Homebrew and also ensure that we can release a properly tested version to Homebrew.

If you're unable to maintain it in this repository that's fine; the community will continue to do so.

The package hasn't been updated for the past three releases, any attempt to do so failed and IIUC the previous maintainer (ilovzfs) quit. I'd be more than happy to start maintaining the homebrew core package if you can point out a path forward.

As a side note I'm seeing more and more projects taking this approach and I'd like to say as a long-time package manager maintainer (and even longer package manager user): it's a mistake. Imagine if every tool that used the JVM insisted on shipping their own version. This wastes time downloading, compiling and storing it and means every individual project needs to bump the JVM version on a security update. Homebrew is considerably more liberal than most package managers on this front and if we're saying it's a bad idea: good luck convincing e.g. Debian/Ubuntu to do it.

I generally agree and we are very much aware of that. However, there are reasons why projects are taking this approach:

  • The introduction of modules in Java made it possible to build a minimal JDK that only contains what your project needs (i.e. 13MiB for Bazel vs. 200MiB for the full JDK).
  • Homebrew does not ship a Java formula, so there's a manual step to install Java involved. The majority of Bazel users use it to build a language other than Java so they might ask themselves why they need to install a full JDK.
  • Homebrew cask has a Java formula but only ships the latest version and seems to immediately move to a newer version. That was fine in the past were JDK releases happened every 3 years, but now that they happen every 6 months everyone is just on different versions of the JDK and there have been major breaking changes in the past few JDK releases.

Additionally, Bazel is special in that Bazel runs on the JDK and is used to build and run Java applications. These two JDKs are often not the same (i.e. Bazel runs on JDK10, but the user would like to build a JDK8 code base).

@MikeMcQuaid
Copy link
Member

The package hasn't been updated for the past three releases, any attempt to do so failed and IIUC the previous maintainer (ilovzfs) quit. I'd be more than happy to start maintaining the homebrew core package if you can point out a path forward.

We don't have individual package maintainers in Homebrew. The path forward would be to reopen #31066 and address the comments and build failures.

The introduction of modules in Java made it possible to build a minimal JDK that only contains what your project needs (i.e. 13MiB for Bazel vs. 200MiB for the full JDK).

Again, the problem here is what every project needs is different and relying on a system JDK means that all projects have what all projects need.

That said, if bazel adopts a fairly traditional build system (e.g. make, make install) and that ends up installing a version of a JDK that you build inside the formula: we can consider that.

That was fine in the past were JDK releases happened every 3 years, but now that they happen every 6 months everyone is just on different versions of the JDK and there have been major breaking changes in the past few JDK releases.

We allow depending on specific major versions of the JDK.

Additionally, Bazel is special in that Bazel runs on the JDK and is used to build and run Java applications. These two JDKs are often not the same (i.e. Bazel runs on JDK10, but the user would like to build a JDK8 code base).

You can brew cask install (or manually install) multiple Java versions side-by-side so I'm not sure how this is relevant.

@buchgr
Copy link
Author

buchgr commented Aug 24, 2018

You can brew cask install (or manually install) multiple Java versions side-by-side so I'm not sure how this is relevant.

The problem we faced when trying to update the formula is that this Bazel release requires JDK9, was only tested on JDK9 but the homebrew CI does not have jdk9 and neither does cask.

That said, if bazel adopts a fairly traditional build system (e.g. make, make install) and that ends up installing a version of a JDK that you build inside the formula: we can consider that.

Can you elaborate on this please? Are you saying that we could build the JDK from source in the Bazel formula?

@MikeMcQuaid
Copy link
Member

The problem we faced when trying to update the formula is that this Bazel release requires JDK9, was only tested on JDK9 but the homebrew CI does not have jdk9 and neither does cask.

It may have only been tested on JDK 9 but does it build on either JDK 10 or JDK 8?

Can you elaborate on this please? Are you saying that we could build the JDK from source in the Bazel formula?

Perhaps. If it cannot work at all on JDK 10 or JDK 8 and the JDK can be built from source in the formula without it becoming a mess then it's an option we'll consider.

@buchgr
Copy link
Author

buchgr commented Aug 24, 2018

It may have only been tested on JDK 9 but does it build on either JDK 10 or JDK 8?

It does build on JDK10 but the tests don't pass. The next Bazel release (in a few weeks) will be build and tested on JDK10 so that should work again, but once JDK11 is out cask will move to JDK11, remove JDK10 and a Bazel release that is tested with JDK11 will happen completely independent of this. That's why I think it's likely that we will run into the same issue again in a few months time.

@MikeMcQuaid
Copy link
Member

The next Bazel release (in a few weeks) will be build and tested on JDK10 so that should work again,

We can always wait until then and update that.

but once JDK11 is out cask will move to JDK11, remove JDK10 and a Bazel release that is tested with JDK11 will happen completely independent of this. That's why I think it's likely that we will run into the same issue again in a few months time.

Yes, it seems likely the same issue will occur. Unfortunately this is a result of Oracle's decision to make Java 9 and 10 to not be LTS releases so I'm not sure what Homebrew can do about that, really.

@buchgr
Copy link
Author

buchgr commented Aug 24, 2018

We can always wait until then and update that.

Well yes, but that's why we had to start our own tap in the first place :\ and why Homebrew users are now on an old version of Bazel.

Yes, it seems likely the same issue will occur. Unfortunately this is a result of Oracle's decision to make Java 9 and 10 to not be LTS releases so I'm not sure what Homebrew can do about that, really.

The Java 11 LTS release [1] will also not be free and so as far as Homebrew and Bazel is concerned there will not be a LTS release for Java anymore. That's why I think maintaining our own tap will give the best user experience.

[1] http://www.oracle.com/technetwork/java/eol-135779.html

@MikeMcQuaid
Copy link
Member

Well yes, but that's why we had to start our own tap in the first place :\ and why Homebrew users are now on an old version of Bazel.

With respect, Homebrew users are on an old version of Bazel because you made updates to Bazel without considering how it would affect them.

The Java 11 LTS release [1] will also not be free and so as far as Homebrew and Bazel is concerned there will not be a LTS release for Java anymore.

From our perspective we are more likely to keep a java11 cask around given upstream's LTS status.

That's why I think maintaining our own tap will give the best user experience.

And I disagree.


I think we're going round in circles here. As far as I see the options are:

  • leave Bazel as-is until there's a new version that works with a current or LTS version of Java installed by Homebrew Cask
  • submit a Bazel formula for evaluation which builds your JVM from source as part of the formula

@buchgr buchgr closed this Aug 24, 2018
@buchgr
Copy link
Author

buchgr commented Aug 24, 2018

Ok thanks for your input. I ll try to update the homebrew core formula to build with 0.17.0 again.

@MikeMcQuaid
Copy link
Member

Thanks @buchgr, much appreciated.

@lock lock bot added the outdated PR was locked due to age label Sep 23, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Sep 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintainer feedback Additional maintainers' opinions may be needed outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants