Skip to content

Always use Bazel-0.25 for macOS on CircleCI (release-1.2).#2283

Merged
fpesce merged 2 commits intoistio:release-1.2from
PiotrSikora:bazel_macos-release-1.2
Jul 23, 2019
Merged

Always use Bazel-0.25 for macOS on CircleCI (release-1.2).#2283
fpesce merged 2 commits intoistio:release-1.2from
PiotrSikora:bazel_macos-release-1.2

Conversation

@PiotrSikora
Copy link
Copy Markdown
Contributor

@PiotrSikora PiotrSikora commented Jun 20, 2019

Backported from #2252.

Signed-off-by: Piotr Sikora piotrsikora@google.com

@PiotrSikora
Always use Bazel-0.25 for macOS on CircleCI (release-1.2).
c4baa4f 
Backported from istio#2252.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Jun 20, 2019
@istio-testing istio-testing requested a review from rshriram June 20, 2019 03:06
@PiotrSikora PiotrSikora force-pushed the bazel_macos-release-1.2 branch from 8e8539f to c4baa4f Compare June 20, 2019 03:16
bianpengyuan
bianpengyuan reviewed Jun 20, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@bianpengyuan bianpengyuan left a comment

Choose a reason for hiding this comment

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

/lgtm

@PiotrSikora
Copy link
Copy Markdown
Contributor Author

PiotrSikora commented Jun 25, 2019

@duderino ping.

bianpengyuan
bianpengyuan reviewed Jun 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@bianpengyuan bianpengyuan left a comment

Choose a reason for hiding this comment

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

Prow bazel image is not updated. Could you use istio/ci:go1.11-bazel0.25-clang7-cmake3.8.0?

@PiotrSikora
review: update Bazel for Linux on CircleCI.
b99bf8f 
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Copy Markdown
Contributor Author

PiotrSikora commented Jun 25, 2019

@bianpengyuan sure, updated... But the point of this PR is to pin Bazel on macOS, otherwise it was installing the latest version on each CI run and completely breaking ci/circleci: macos.

@bianpengyuan
Copy link
Copy Markdown
Contributor

bianpengyuan commented Jun 25, 2019

@PiotrSikora I see. Thanks!

bianpengyuan
bianpengyuan reviewed Jun 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@bianpengyuan bianpengyuan left a comment

Choose a reason for hiding this comment

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

/lgtm

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Jun 25, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bianpengyuan, PiotrSikora
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: linsun

If they are not already assigned, you can assign the PR to them by writing /assign @linsun in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@duderino
Copy link
Copy Markdown

duderino commented Jul 23, 2019

One of the 1.2 release managers should handle this one: @fpesce @linsun

@duderino
Copy link
Copy Markdown

duderino commented Jul 23, 2019

/lgtm

@lambdai
Copy link
Copy Markdown
Contributor

lambdai commented Jul 23, 2019

istio/ci:go1.11-bazel0.25-clang7-cmake3.8.0 is accessible at linux so hopefully in macOS it is also reachable.

I don't have a mac to test but it looks good to me

/lgtm

@PiotrSikora
Copy link
Copy Markdown
Contributor Author

PiotrSikora commented Jul 23, 2019

@silentdai macOS doesn't use Docker, but always installs Bazel 0.25.0. Tests pass on CircleCi, btw.

@fpesce fpesce merged commit e522d64 into istio:release-1.2 Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@duderino duderino Awaiting requested review from duderino

@rshriram rshriram Awaiting requested review from rshriram

1 more reviewer

@bianpengyuan bianpengyuan bianpengyuan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@lambdai lambdai

@duderino duderino

@bianpengyuan bianpengyuan

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@PiotrSikora @bianpengyuan @istio-testing @duderino @lambdai @fpesce @googlebot