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.

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

Fix #3653: Bazel Build Fail for MacOS #3654

Closed
wants to merge 1 commit into from
Closed

Fix #3653: Bazel Build Fail for MacOS #3654

wants to merge 1 commit into from

Conversation

anandwana001
Copy link
Contributor

@anandwana001 anandwana001 commented Aug 7, 2021 •
edited
Loading

Explanation

Fix #3653

Original Issue - protocolbuffers/protobuf#8863

Screenshot 2021-08-07 at 09 59 23

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

BenHenning
BenHenning reviewed Aug 9, 2021
View reviewed changes
Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @anandwana001. Nice find. I wish that we would understand why this change works, but it might be something as simple as the old dependencies aren't built against the latest runtime libraries on OS X.

Is it possible for us to add an OS X flavor to https://github.com/oppia/oppia-android/blob/develop/.github/workflows/build_tests.yml#L19 so that we can verify that this works? I think we need to verify that it first fails without the fixes, but then that the fixes fix the OS X build (which is specifically convenient since it provides a regression test).

third_party/versions.bzl
@@ -106,12 +106,16 @@ MAVEN_TEST_DEPENDENCY_VERSIONS = {

# Note to developers: Please keep this dict sorted by key to make it easier to find dependencies.
HTTP_DEPENDENCY_VERSIONS = {
"bazel_skylib": {
"sha": "97e70364e9249702246c0e9444bccdc4b847bed1eb03c5a3ece4f83dfe6abc44",
"version": "1.0.2",
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the latest release?

third_party/versions.bzl
"dagger": {
"sha": "9e69ab2f9a47e0f74e71fe49098bea908c528aa02fa0c5995334447b310d0cdd",
"version": "2.28.1",
},
"protobuf_tools": {
"version": "3.11.0",
"version": "3.12.0",
Copy link
Member

Choose a reason for hiding this comment

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

Could this be the latest protoc?

@BenHenning BenHenning assigned anandwana001 and unassigned BenHenning Aug 9, 2021
@oppiabot
Copy link

oppiabot bot commented Aug 16, 2021

Hi @anandwana001, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 3 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 16, 2021
@anandwana001
Copy link
Contributor Author

Putting this PR on less priority for now, will check again in next week.

@oppiabot oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 18, 2021
@oppiabot
Copy link

oppiabot bot commented Aug 25, 2021

Hi @anandwana001, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 3 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 25, 2021
@oppiabot oppiabot bot closed this Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenHenning BenHenning BenHenning left review comments

Assignees

@anandwana001 anandwana001

Labels
stale Corresponds to items that haven't seen a recent update and may be automatically closed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bazel Build Fail for MacOS
2 participants
@anandwana001 @BenHenning