Skip to content

refactor: depend on bazelisk rather than directly on Bazel #35153

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

refactor: depend on bazelisk rather than directly on Bazel #35153

wants to merge 1 commit into from

Conversation

alexeagle
Copy link
Contributor

This has a couple benefits:

  • we now use a .bazelversion file rather than package.json to pin the version of bazel we want. This means even if you install bazel on your computer rather than via yarn, you'll still get a warning if your bazel version is wrong.
  • you no longer end up downloading three copies of bazel due to bugs in both npm and yarn where they download all tarballs before checking the metadata to see which are usable on the local platform.
  • bazelisk correctly handles the tools/bazel trick for wrapping functionality, which we want to use to instrument developer build latencies
  • it should fix the strange issue with Windows CI seeing bazel being unlinked out from under the binary; I removed the workaround

Note that @angular/bazel package isn't updated in this PR; I'm working on a plan to remove the opt-in preview bits with schematics/builder for running bazel this way.

@pullapprove pullapprove bot requested review from filipesilva and kyliau February 5, 2020 00:16
@filipesilva
Copy link
Contributor

CI is failing because integration/bazel is still trying to run bazel:

$ bazel build ... --noshow_progress && bazel test ...
/bin/sh: 1: bazel: not found

@alexeagle alexeagle force-pushed the bazelisk branch 4 times, most recently from 6178866 to 7fc2555 Compare February 12, 2020 22:02
@kara kara added the area: bazel Issues related to the published `@angular/bazel` build rules label Feb 13, 2020
@ngbot ngbot bot added this to the needsTriage milestone Feb 13, 2020
@philwo
Copy link

philwo commented Feb 25, 2020

@alexeagle What's necessary to get this in and can I help in some way?

@alexeagle
Copy link
Contributor Author

thanks @philwo I think I just need to work through misconfigurations of the integration tests wrt. how they find bazel binary

@alexeagle alexeagle added the target: major This PR is targeted for the next major release label Feb 26, 2020
@alexeagle
refactor: depend on bazelisk rather than directly on Bazel
eee20f5 
This has a couple benefits:
- we now use a .bazelversion file rather than package.json to pin the version of bazel we want. This means even if you install bazel on your computer rather than via yarn, you'll still get a warning if your bazel version is wrong.
- you no longer end up downloading three copies of bazel due to bugs in both npm and yarn where they download all tarballs before checking the metadata to see which are usable on the local platform.
- bazelisk correctly handles the tools/bazel trick for wrapping functionality, which we want to use to instrument developer build latencies
- it should fix the strange issue with Windows CI seeing bazel being unlinked out from under the binary; I removed the workaround
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels Mar 3, 2020
@alexeagle
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Mar 3, 2020
@gregmagolan gregmagolan mentioned this pull request Mar 16, 2020
Closed
14 tasks
@gregmagolan
Copy link
Contributor

Replaced by #36078

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@filipesilva filipesilva Awaiting requested review from filipesilva

@kyliau kyliau Awaiting requested review from kyliau

@gregmagolan gregmagolan Awaiting requested review from gregmagolan

@josephperrott josephperrott Awaiting requested review from josephperrott

Assignees
No one assigned
Labels
area: bazel Issues related to the published `@angular/bazel` build rules cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Milestone
needsTriage
Development

Successfully merging this pull request may close these issues.
6 participants
@alexeagle @filipesilva @philwo @googlebot @gregmagolan @kara