Skip to content

Fix select with condition on darwin.#2274

Merged
alexeagle merged 1 commit intobazel-contrib:stablefrom
comius:fix-darwin-condition
Nov 9, 2020
Merged

Fix select with condition on darwin.#2274
alexeagle merged 1 commit intobazel-contrib:stablefrom
comius:fix-darwin-condition

Conversation

@comius
Copy link
Copy Markdown
Contributor

@comius comius commented Nov 9, 2020

Bazel is changing how @bazel_tools/conditions:darwin is implemented.
With old implementation based on flags, it was ok to have darwin and
darwin_x86_64 in a select. This was based on flag --cpu=darwin and
--cpu=dawin_x86_64.

New implementation is based on constraints, where "darwin" means OS (and
any CPU), while "darwin_x86_64" mean only specific CPU. As such they
cannot be used in the same select, because the selection would be
ambiguous.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Bazel is changing how @bazel_tools/conditions:darwin is implemented.
With old implementation based on flags, it was ok to have darwin and
darwin_x86_64 in a select. This was based on flag --cpu=darwin and
--cpu=dawin_x86_64.

New implementation is based on constraints, where "darwin" means OS (and
any CPU), while "darwin_x86_64" mean only specific CPU. As such they
cannot be used in the same select, because the selection would be
ambiguous.
Copy link
Copy Markdown
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

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

thanks!

@Wyverald
Copy link
Copy Markdown
Contributor

@comius
Copy link
Copy Markdown
Contributor Author

comius commented Nov 12, 2020

rules_nodejs still seems to be failing: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1761#3e9ef6b7-1c7c-490e-b51d-85777d2f8c2f @comius

The buildkite is using master branch, whereas the patch has been merged into stable.
Since stable is more recent and designated as default, I guess we should be using it?

@alexeagle could you confirm?

@alexeagle
Copy link
Copy Markdown
Collaborator

That's right, we no longer use master. I would delete it but not all incoming refs get redirected.

@comius
Copy link
Copy Markdown
Contributor Author

comius commented Nov 18, 2020

That's right, we no longer use master. I would delete it but not all incoming refs get redirected.

I proposed a change in CI to use stable branch - bazelbuild/continuous-integration#1064

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants