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

Reintroduce setup.py changes from #8280 erased by piper import #8902 #8966

Merged

Conversation

jtattermusch
Copy link
Contributor

This fixes the broken python aarch64 linux tests (see https://source.cloud.google.com/results/invocations/861e900c-12ad-4998-9489-18f5cfbf76ee)

For some reason (bad merge?), the changes needed for crosscompiling aarch64 python wheels have been removed in piper import https://github.com/protocolbuffers/protobuf/pull/8902/files#diff-eb8b42d9346d0a5d371facf21a8bfa2d16fb49e213ae7c21f03863accebe0fcfL161.

This is actually the second time this happened (see #8667 and the fix #8746). What can we do to make sure this won't happen again?

@haberman
Copy link
Member

Yes there does appear to be a bug in the tooling where setup.py changes aren't getting merged properly. We should take a look at at that.

I do wonder though, why wasn't this caught in Kokoro testing? Shouldn't there be a test verifying this behavior?

@jtattermusch
Copy link
Contributor Author

There is a test job and the failure was caught by it: https://fusion.corp.google.com/projectanalysis/summary/KOKORO/prod:protobuf%2Fgithub%2Fmaster%2Fubuntu%2Fpython_aarch64%2Fcontinuous

the test doesn't run on PRs though (I didn't want to add a large number of new PR test jobs) - we can add those jobs to protobuf PRs if you think it's worthwhile.

@jtattermusch
Copy link
Contributor Author

@peteboothroyd
Copy link

peteboothroyd commented Dec 14, 2021

@haberman @jtattermusch this has also affected this PR #8346 which has been reverted and lost.

@jtattermusch
Copy link
Contributor Author

FTR I filed internal b/213024295 to track the piper import problem.

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

Successfully merging this pull request may close these issues.

5 participants