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

Remove Intel mac CircleCI configuration #1409

Merged
merged 6 commits into from
Jun 20, 2024
Merged

Remove Intel mac CircleCI configuration #1409

merged 6 commits into from
Jun 20, 2024

Conversation

dschuff
Copy link
Member

@dschuff dschuff commented Jun 20, 2024

Also switch bazel-mac to the arm64 config.
The CircleCI Intel mac config will be turned down soon.

@dschuff
Copy link
Member Author

dschuff commented Jun 20, 2024

@walkingeyerobot will there need to be anything interesting done to bazel because of the architecture switch?

@dschuff
Copy link
Member Author

dschuff commented Jun 20, 2024

Looks like the bazel test is failing on arm64 mac. The grep/sed pipeline that tries to set VER seems to be messing up (since VER ends up empty).

I wonder if the ggrep tool isn't on the bot. I don't have it on my arm64 macbook.

@dschuff
Copy link
Member Author

dschuff commented Jun 20, 2024

So the ggrep version of grep is present and found... the pipeline just isn't correctly selecting the version.

@dschuff
Copy link
Member Author

dschuff commented Jun 20, 2024

OK, just getting the version info from the file as JSON in Python seemed easier than debugging what was going wrong with grep. PTAL.
I decided against doing a bunch of error checking in the python file because in all the error cases I thought of, the messages I was thinking of writing were no better than the errors python itself would give directly, and would make the program 2x as long.

@dschuff
Copy link
Member Author

dschuff commented Jun 20, 2024

also: do you have any idea why the "test-mac" and "test-bazel-mac" jobs are still showing up on the GH UI as "expected"? I thought I correctly got rid of them everywhere.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 20, 2024

The "expected" list of bots is set in the github branch settings for main.

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

This is where the jq tool would be awesome, but I'm not sure how hard it would be to ensure it is installed on all the bot.

@dschuff dschuff merged commit 4b9e83d into main Jun 20, 2024
9 checks passed
@dschuff dschuff deleted the macarm branch June 20, 2024 23:16
@dschuff
Copy link
Member Author

dschuff commented Jun 20, 2024

jq appears to be available in Debian; if it's also in homebrew, I guess we could install it the same way we install other packages. Not sure if that's better or worse than a tiny python script. I guess at least with the python script, we can skip the homebrew step on the bazel bots entirely.

@walkingeyerobot
Copy link
Collaborator

I don't expect this to cause any problems for bazel.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 21, 2024

jq appears to be available in Debian; if it's also in homebrew, I guess we could install it the same way we install other packages. Not sure if that's better or worse than a tiny python script. I guess at least with the python script, we can skip the homebrew step on the bazel bots entirely.

jq is like a really nice way to extract data from a json file. I learned about it a couple of years back and I think if you work with json a lot its amazing. I would totally make this python script redundant. But not rush, either way is fine for now.

@allsey87
Copy link
Contributor

@dschuff any idea why all PRs now have this pending test on test-bazel-mac-arm64 that never starts?

@dschuff
Copy link
Member Author

dschuff commented Jun 24, 2024

Hm, test-bazel-mac-arm64 should be running on every PR now. Did you see this this on new PRs or for existing PRs that haven't had commits pushed since this PR landed?

@dschuff
Copy link
Member Author

dschuff commented Jun 24, 2024

I guess you must be looking at #1405. I think if you push another commit it should work as expected with the new config. Otherwise if we just end up accepting the most recent commit there, we can just bypass the branch protections to land it, so no big deal either way.

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

Successfully merging this pull request may close these issues.

4 participants