Opened 9 years ago

Closed 7 years ago

Last modified 7 years ago

#48937 closed submission (fixed)

New port: Bazel

Reported by: ahh Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc: Schamschula (Marius Schamschula), maehne (Torsten Maehne)
Port: bazel

Description

Bazel is a build system written by Google for open-source use (based on their internal system): http://bazel.io/

http://bazel.io/docs/install.html contains simple distributables, and instructions for building from source off Github.

Attachments (6)

Portfile.2 (3.4 KB) - added by raimue (Rainer Müller) 7 years ago.
extract.mkdir, no distpath deletion
Portfile (3.4 KB) - added by RootFunction 7 years ago.
Initial Portfile for Bazel
Portfile.3 (3.4 KB) - added by Schamschula (Marius Schamschula) 7 years ago.
Portfile.4 (3.9 KB) - added by Schamschula (Marius Schamschula) 7 years ago.
Added basel-complete bash
Portfile.5 (4.0 KB) - added by RootFunction 7 years ago.
Merged missed changes with previous bash completion update
Portfile.6 (4.6 KB) - added by RootFunction 7 years ago.
Updated to version 0.5.2 and fixed completion script issues

Download all attachments as: .zip

Change History (30)

comment:1 in reply to:  description Changed 7 years ago by RootFunction

Replying to ahh:

Bazel is a build system written by Google for open-source use (based on their internal system): http://bazel.io/

http://bazel.io/docs/install.html contains simple distributables, and instructions for building from source off Github.


This post provides updated links.
Home page: https://bazel.build
Installation and build instruction page: https://bazel.build/versions/master/docs/install.html
Release page containing archives and scripts: https://github.com/bazelbuild/bazel/releases

comment:2 Changed 7 years ago by Schamschula (Marius Schamschula)

Port: bazel added
Version: 2.3.32.4.1

comment:3 Changed 7 years ago by Schamschula (Marius Schamschula)

Type: requestsubmission

comment:4 Changed 7 years ago by mf2k (Frank Schima)

Summary: Request: BazelNew port: Bazel
Version: 2.4.1

The "Version" field is not needed in Submission tickets.

Notes on your Portfile:

  • Use the GitHub portgroup to simplify the Portfile.
  • You specify supported_archs but do not have a build phase. That implies that nothing is compiled. So it should say:
    supported_archs    noarch
    

comment:5 in reply to:  4 Changed 7 years ago by RootFunction

Replying to mf2k:

The "Version" field is not needed in Submission tickets.

Notes on your Portfile:

  • Use the GitHub portgroup to simplify the Portfile.
  • You specify supported_archs but do not have a build phase. That implies that nothing is compiled. So it should say:
    supported_archs    noarch
    


Thanks for the suggestions. I've updated the Portfile attached to this ticket.

comment:6 Changed 7 years ago by raimue (Rainer Müller)

Quoting the linked web page: "The installer only contains Bazel binary, some additional libraries are required to be installed on the machine to work."

Are you sure that running the installer is the right thing to do here?

Examples from other distributions use the -dist.zip and compile it locally:

Also there should be some kind of dependency on Java. As I said before on #52730: I personally have no confidence in reviewing any Java related port. For example, I don't know if it is wise to use compiled Java class files and whether these will work on all available Java runtimes on the various versions of OS X.

comment:7 in reply to:  6 Changed 7 years ago by RootFunction

I'm not really aware of any big reasons why compiling from source would be better than running the installer, so I went with the installer method because it was somewhat easier. Perhaps it would be slightly more prudent to compile from source in order to ensure that the binary runs on the user's machine, so for that reason I wouldn't mind changing approaches. The source code archive is also just over half the size of the installer script, so compiling from source makes sense if you also want to save bandwidth.

I agree with the suggestion of adding a dependency on Java. Could we not just add a check for JDK 8 or greater without worrying about which of its Java libraries are actually being used? On OSX, Bazel requires JDK 8 and the Xcode command line tools - assuming you're not using Homebrew. MacPorts already requires the Xcode command line tools. If there are particular Java runtime + OS X version combinations that don't work when installing the port with MacPorts, then wouldn't it be the case that those combinations would also fail when installing the port manually? I think in these situations, the correct course of action would be to file tickets with the port developers.

comment:8 Changed 7 years ago by mf2k (Frank Schima)

Macports is not for downloading binaries. It is for compiling software. So definitely compile the project from source. Then the line:

supported_archs    noarch

Is no longer correct and should be removed.

comment:9 Changed 7 years ago by kencu (Ken)

this port would likely be handy, as it appears to be a build dependency of tensorflow <https://www.tensorflow.org/install/install_sources> and probably others.

Here's a link to the homebrew formula for bazel as well, if that is of any use to the submitter/reviewer <https://github.com/Homebrew/homebrew-core/blob/master/Formula/bazel.rb>

comment:10 Changed 7 years ago by RootFunction

I re-wrote the Portfile to compile Bazel from source instead of copying the binary. I've also added a check in the pre-fetch stage to make sure an appropriate JDK is installed.

Last edited 7 years ago by RootFunction (previous) (diff)

comment:11 Changed 7 years ago by mf2k (Frank Schima)

Comments:

  • Can openmaintainer be added?
  • In general, we do not set distfiles directly, it is best practice to set distname, worksrcdir and extract.suffix separately. extract.suffix is covered by use_zip yes

comment:12 in reply to:  11 Changed 7 years ago by RootFunction

Ok, I've added openmaintainer and made the other changes in the fetch phase.

comment:13 Changed 7 years ago by RootFunction

Is there anything else required for this port to be integrated? The biggest issue seems to be the Java dependency, and I've tried to address that in the pre-fetch stage. If anyone is aware of a better solution though, I'd welcome it.

comment:14 Changed 7 years ago by raimue (Rainer Müller)

I propose the following changes:

  • What is the reason for deleting ${distpath} in post-destroot? Users are supposed to use port clean --dist or port reclaim to get rid of distfiles. I removed that command.
  • ${worksrcdir} is not supposed to be empty. Extracted files would be dumped directly into ${workpath} which might lead to name clashes (e.g. names like destroot, .home, .tmp, etc. are in use). Use extract.mkdir to create the subdirectory before extracting.
  • What exactly do you want to achieve with that sudo sh in build {}? The build phase is supposed to run as the "macports" user without super user privileges. I do not understand why stderr is a problem, but shouldn't the output be redirected (as in 2>&1) instead of being ignored completely?

Additionally, the part on getting the compiler version of javac would probably be better suited for the java port group. I'll leave that to people with more knowledge about Java on macOS.

Changed 7 years ago by raimue (Rainer Müller)

Attachment: Portfile.2 added

extract.mkdir, no distpath deletion

comment:15 in reply to:  14 Changed 7 years ago by RootFunction

Listed below are my responses to the proposed changes. I'll upload a new Portfile shortly.

  • When deleting ${distpath} in post-destroot, my intention was to free up disk space as the source and documentation files had already been copied over. If it's expected that users are supposed to use port clean --dist or port reclaim, then I'm fine with leaving the distfiles. I see MacPorts periodically reminds users to run port reclaim after running sudo port selfupdate anyway.
  • Good point. I didn't think of name clashes in ${workpath}.
  • When running the compile script without sudo, I found the build always failed due to problems with symlinks. It turns out I should have been using bash instead of sh though, so with that change sudo is no longer needed. The issue with stderr is that the compile script writes all sorts of minor warning messages there, and exec throws an error if anything is written to stderr and not subsequently redirected. This failure then gets propagated to the build phase, which causes the entire port installation to fail unnecessarily. Since I'm not interested in parsing the text written to stderr or stdout, I opted for the ignorestderr option rather than redirecting stderr. It's my understanding that every printed message from each phase gets written into the debug log anyway.

Changed 7 years ago by RootFunction

Attachment: Portfile added

Initial Portfile for Bazel

comment:16 Changed 7 years ago by Schamschula (Marius Schamschula)

Cc: Schamschula added

Changed 7 years ago by Schamschula (Marius Schamschula)

Attachment: Portfile.3 added

comment:17 Changed 7 years ago by Schamschula (Marius Schamschula)

Updated Portfile to version 0.5.1

comment:18 Changed 7 years ago by Schamschula (Marius Schamschula)

Hmm. The installed package is missing bazel-complete.bash, which is part of the pre-packaged version from Google.

Changed 7 years ago by Schamschula (Marius Schamschula)

Attachment: Portfile.4 added

Added basel-complete bash

Changed 7 years ago by RootFunction

Attachment: Portfile.5 added

Merged missed changes with previous bash completion update

comment:19 Changed 7 years ago by RootFunction

Is there anyone available to review the Java portion of the Portfile?

comment:20 Changed 7 years ago by maehne (Torsten Maehne)

Cc: maehne added

comment:21 Changed 7 years ago by raimue (Rainer Müller)

Do not install bazel-complete.bash to ${prefix}/bin, it does not make sense to have it in $PATH as it is not meant to be executed directly. Depending on how they are written, bash-completion scripts need to go into either ${prefix}/share/bash-completion/completions/ (on-demand) or ${prefix}/etc/bash_completion.d/ (always loaded).

However, this file does not look like a bash-completion script at all. Either the file should be built using bazel itself or replicate all arguments as in ${worksrcpath}/scripts/BUILD, the missing bit would be the --prepend argument. The former would be more robust to future changes. I did not test it, but the resulting file should then be good to be installed at ${prefix}/share/bash-completion/completions/bazel for on-demand loading.

Changed 7 years ago by RootFunction

Attachment: Portfile.6 added

Updated to version 0.5.2 and fixed completion script issues

comment:22 in reply to:  21 Changed 7 years ago by RootFunction

I uploaded a new Portfile that solves the bash-completion script issue and also includes the zsh-completion script.

comment:23 Changed 7 years ago by raimue (Rainer Müller)

Resolution: fixed
Status: newclosed

In 9cf7af524e0abc99734dd46f1a73908d5f360c0b/macports-ports:

bazel: New port, version 0.5.2

Closes: #48937

comment:24 Changed 7 years ago by raimue (Rainer Müller)

That looked good, but was not yet fully correct. In order not to delay the inclusion of the Portfile further, I fixed up the shell completions so that they are automatically loaded, removing the need for notes. I also added your GitHub username to the maintainers in the Portfile.

Thank you for your submission and bearing with us for so long while we sorted through all the details. For future updates as a maintainer, just submit patches as an attachment to a new Trac ticket or make a GitHub pull request. When filing tickets as a maintainer, use the maintainer haspatch keywords for faster review.

Note: See TracTickets for help on using tickets.