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

bazel: Update to version 0.9.0 #1098

Closed
wants to merge 2 commits into from
Closed

bazel: Update to version 0.9.0 #1098

wants to merge 2 commits into from

Conversation

missa-prime
Copy link
Contributor

@missa-prime missa-prime commented Dec 10, 2017

Description

Closes: https://trac.macports.org/ticket/55407
Closes: https://trac.macports.org/ticket/55659

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 10.12.6
Xcode 9.2

Verification

Have you

  • followed our Commit Message Guidelines?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint?
  • tested basic functionality of all binary files?

file delete -force /var/tmp/_${name}_root
exec -ignorestderr -- git init ${worksrcpath}
exec -ignorestderr -- git add ${worksrcpath}
exec -ignorestderr -- git commit -m GitRepo
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to make a dummy commit during the build phase?

It also looks a bit suspicious that a build would fail if you don't remove a file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oddly enough, I can't build the bash completion script without deleting that temporary directory and having a local git repo in that state. I suspect it's a bug that popped up after 0.5.2. So essentially these changes work around that problem.

Copy link
Member

Choose a reason for hiding this comment

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

Never use exec, only system.

I don't understand why this would be necessary, though. If a git repo is assumed in any build rule, better patch that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why only system rather than exec? Also, could you point me to a good document for system? I'm not finding one online.

Copy link
Member

Choose a reason for hiding this comment

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

From man portfile:

 system [-W dir] commandline
     Execute a program. See system(3).  For calls to install(1) please use xinstall.  For calls to mv(1), cp(1), rm(1) or
     similar, please use the built-in commands or file if they don't meet your requirements.

     -W      Change to dir before working.

Copy link
Contributor

Choose a reason for hiding this comment

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

Without that bit of code, the bazel build errors out like this

Build successful! Binary is here: /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_bazel/bazel/work/bazel-0.9.0-dist/output/bazel
xinstall: chdir(/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_bazel/bazel/work/bazel-0.9.0-dist/output)
xinstall: bazel -> /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_bazel/bazel/work/bazel-0.9.0-dist/bazel
Extracting Bazel installation...
.....................
Loading: 
Loading: 0 packages loaded
Analyzing: target //scripts:bazel-complete.bash (3 packages loaded)
Analyzing: target //scripts:bazel-complete.bash (4 packages loaded)
Analyzing: target //scripts:bazel-complete.bash (73 packages loaded)
Analyzing: target //scripts:bazel-complete.bash (75 packages loaded)
Analyzing: target //scripts:bazel-complete.bash (195 packages loaded)
INFO: Analysed target //scripts:bazel-complete.bash (195 packages loaded).
INFO: Found 1 target...
[0 / 2] [-----] BazelWorkspaceStatusAction stable-status.txt
ERROR: Process exited with status 128: Process exited with status 128
fatal: Not a git repository (or any of the parent directories): .git
Target //scripts:bazel-complete.bash failed to build
INFO: Elapsed time: 25.182s, Critical Path: 0.14s
FAILED: Build did NOT complete successfully
FAILED: Build did NOT complete successfully
Error: Failed to build bazel: child process exited abnormally
Error: See /opt/local/var/macports/logs/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_bazel/bazel/main.log for details.
Error: Follow https://guide.macports.org/#project.tickets to report a bug.
Error: Processing of port bazel failed

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyway, with that bit of code in, 0.9.0 builds without intervention

$ port -v installed bazel
The following ports are currently installed:
  bazel @0.9.0_0 (active) platform='darwin 17' archs='x86_64' date='2017-12-30T17:32:13-0800'

Thanks!

@kencu kencu mentioned this pull request Jan 9, 2018
10 tasks
@emcrisostomo
Copy link
Member

@RootFunction, if you take a look at #1132 you'll see that @kencu contributed an updated 0.9.0 Portfile inspired to yours. Would you please bump this PR to 0.9.0?

@missa-prime missa-prime changed the title bazel: Update to version 0.8.1 bazel: Update to version 0.9.0 Jan 14, 2018
@missa-prime
Copy link
Contributor Author

@emcrisostomo, I have bumped the PR to 0.9.0

@kencu, Switching from exec to system causes the installation to hang. I suspect the issue is in the bash completion script portion again. I think there is some gotcha I'm missing when using system in this case, but I'm not sure what.

@emcrisostomo
Copy link
Member

Thank you @RootFunction.

@kencu
Copy link
Contributor

kencu commented Jan 15, 2018

Personally I could accept this as it is. There are other ports that use exec in the Ports tree. But there are requests to either find a way to use system or find a way to patch out the scripting that is calling for this in the first place. Patching out the scripting is over my head at present. When I get a minute I'll see if I can figure out a system -W call. Doesn't help that building bazel takes forever...

@pmetzger
Copy link
Member

Should this be merged? @kencu ?

@kencu
Copy link
Contributor

kencu commented Feb 4, 2018

trying to find an afternoon to work on this...

@kencu
Copy link
Contributor

kencu commented Feb 8, 2018

I think I have the system -W calls all worked out.

There is still a huge pause in the build (think hour-long or more) while I-have-no-idea-what is happening.

INFO: From Executing genrule //scripts:bash_completion:
Extracting Bazel installation...
Target //scripts:bazel-complete.bash up-to-date:
  bazel-bin/scripts/bazel-complete.bash
INFO: Elapsed time: 747.894s, Critical Path: 85.57s
INFO: Build completed successfully, 1723 total actions
INFO: Build completed successfully, 1723 total actions

LONG PAUSE HERE

Seems this is something that happens sometimes when building things with bazel bazelbuild/bazel#748. No idea if it's fixable or not.

It may well cause the buildbots to all time out, tho, as there is no output during this pause, and top shows nothing much is doing anything.

I'll fix up the PR once I can confirm it all works as expected.

@kencu
Copy link
Contributor

kencu commented Feb 8, 2018

Still waiting. But I know from previous bazel builds it will likely finish. I have sampled all the current processes that have macports as a user, and I'll put these samples here in case some genius out there can see why this build is stuck

Sample of cfprefsd.txt
Sample of distnoted.txt
Sample of java1.txt
Sample of java2.txt
Sample of java3.txt
Sample of java4.txt
Sample of lsd.txt
Sample of tclsh8.5.txt

@pmetzger
Copy link
Member

pmetzger commented Feb 8, 2018

@kencu Have you tried tracing the process while the pause happens?

@kencu
Copy link
Contributor

kencu commented Feb 9, 2018

OK. As before, it sat paused for about two hours, and then finished successfully.

@pmetzger -- the traces are in the post just above yours.

I'll fix up the PR, and we can commit it. We'll see how many people cancel out the build and post tickets about the build failure :>

@kencu
Copy link
Contributor

kencu commented Feb 9, 2018

@RootFunction -- your PR is against your master branch, and I can't update it due to permissions.

Here is the commit message I had used (squashed):

commit 439fae2be6375e2014559de383e83136e4229bb3 (HEAD -> master)
Author: RootFunction <RootFunction@users.noreply.github.com>
Date:   Sun Dec 10 16:26:35 2017 -0500

    bazel: Update to version 0.9.0
    
    closes: https://trac.macports.org/ticket/55407
    closes: https://trac.macports.org/ticket/55731
    closes: https://trac.macports.org/ticket/55736

and here is the diff against the current MacPorts master branch:

bazelfix.diff.txt

@kencu
Copy link
Contributor

kencu commented Feb 12, 2018

So this PR looks good to go to me, if those changes are made in @RootFunction 's PR. I can't do it like I usually might, due to the PR being made off the master branch. So you'll have to do it in your PR, and then squash the commits down to one commit.

@kencu
Copy link
Contributor

kencu commented Feb 14, 2018

@RootFunction -- would you like me to just push this from my own repo, and close this PR?

@pmetzger
Copy link
Member

@kencu Why don't you just do that? It would be quick.

@kencu
Copy link
Contributor

kencu commented Feb 14, 2018

committed separately c47cd8d

@kencu kencu closed this Feb 14, 2018
@kencu
Copy link
Contributor

kencu commented Feb 15, 2018

Hmmm. That didn't work out as planned. Build failed on every system....oops.

The error is

DEBUG: Executing org.macports.build (bazel)
DEBUG: system -W /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_devel_bazel/bazel/work/bazel-0.9.0-dist: ./compile.sh

ERROR: Could not find JAVA_HOME, please ensure a JDK (version 1.8+) is installed.
Command failed: ./compile.sh
Exit code: 1

@kencu
Copy link
Contributor

kencu commented Feb 15, 2018

And that would be because Java isn't installed on any of the buildbots, I understand. So all is likely OK after all.

@missa-prime
Copy link
Contributor Author

Thanks @kencu for all the work to close this. After giving this a try, I was a little worried when it didn't finish after 2 hours. I decided to stick with it though, and everything finished after 4 hours on my machine.

@kencu
Copy link
Contributor

kencu commented Feb 19, 2018

It's a totally nutty long delay. Just for my knowledge, is that bash completion script so useful that it has to be built? Things would go much smoother without it!!

Maybe somebody will see something in those traces I posted and come up with the proper fix for this...

@missa-prime
Copy link
Contributor Author

If this was a new port, I would suggest leaving the completion script out to keep the installation time sane. It could then be re-introduced during a later update or the user could build and install manually if truly needed.

Anyway, I've opened bazelbuild/bazel#4670 and bazelbuild/bazel#4671 to track all the problems encountered in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

9 participants