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

run basic C++ aarch64 tests under qemu emulator #8391

Merged

Conversation

jtattermusch
Copy link
Contributor

@jtattermusch jtattermusch commented Mar 8, 2021

Add a script that crosscompiles C++ under a crosscompiler (and uses dockcross docker image for that) and then runs C++ test suite under QEMU emulator.

  • the tests pass and the the whole build + tests only takes a few minutes (= faster than if we were compiled everything under and emulator, which would be quite slow).
  • for now there's no kokoro job to actually run the tests, but setting up one is easy (I plan to do that as a followup PR)
  • dockcross docker image is used since it makes everything much easier (see comments in the code)
  • for manual run, just run kokoro/linux/aarch64/test_cpp_aarch64.sh

What the kokoro test output would look like if the kokoro job was already created (I used a hack to obtain an adhoc run):
sponge/988880f9-7e8f-465b-8ff9-9483d6ce2f0d

@google-cla google-cla bot added the cla: yes label Mar 8, 2021
@jtattermusch jtattermusch changed the title run basic C++ tests under qemu emulator run basic C++ aarch64 tests under qemu emulator Mar 8, 2021
@jtattermusch
Copy link
Contributor Author

CC @janaknat

@jtattermusch
Copy link
Contributor Author

@dlj-NaN can you please reassign as needed?

@jtattermusch
Copy link
Contributor Author

@dlj-NaN I verified that the tests are passing if run on kokoro CI. Please review.

@jtattermusch
Copy link
Contributor Author

@fowles please also review this PR.

@jtattermusch jtattermusch assigned fowles and unassigned dlj-NaN Apr 23, 2021

# the wrapper script has CRLF line endings and bash doesn't like that
# so we change CRLF line endings into LF.
sed -i 's/\r//g' dockcross-linux-arm64.sh
Copy link
Member

Choose a reason for hiding this comment

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

does windows require this, or can we buy the gnome and fix the wrapper script?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the "dockcross-linux-arm64.sh" is distributed as part of the dockcross/linux-arm64 image, so "fixing" the wrapper script would required opening a pull request against https://github.com/dockcross/dockcross, getting it accepted (maybe it does, maybe it doesn't) and then waiting for a new release of dockcross/linux-arm64 to actually get a fix. That seems as a way to lengthy process (and with uncertain outcome) to be able to rely on it.
Let's keep the sed command as is. I'll look into filing an issue with https://github.com/dockcross/dockcross so that hopefully this will get fixed in the future. But I really don't want to delay this PR because of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the "dockcross-linux-arm64.sh" is distributed as part of the dockcross/linux-arm64 image, so "fixing" the wrapper script would required opening a pull request against https://github.com/dockcross/dockcross, getting it accepted (maybe it does, maybe it doesn't) and then waiting for a new release of dockcross/linux-arm64 to actually get a fix. That seems as a way to lengthy process (and with uncertain outcome) to be able to rely on it.
Let's keep the sed command as is. I'll look into filing an issue with https://github.com/dockcross/dockcross so that hopefully this will get fixed in the future. But I really don't want to delay this PR because of it.


# Location of the build script in repository
build_file: "protobuf/kokoro/linux/cpp_aarch64/build.sh"
timeout_mins: 120
Copy link
Member

Choose a reason for hiding this comment

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

2 hours is a really long time... Can we tighten this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, 120 is the lowest value that protobuf's kokoro .cfg files use (and it's used by many jobs including python, ruby etc.) and it's the most commonly used timeout value, so I simply reused it. (Btw, for some jobs, the timeout value is even much higher)

I can reduce this to e.g. 60 (in reality it takes only about 10mins, but it's useful to have some buffer) if you want, but basically I feel this should be done in a separate effort to set all the protobuf's kokoro jobs to a reasonable timeout (which is unrelated to this PR).

Copy link
Contributor Author

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

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

Responded to all the concerns.


# the wrapper script has CRLF line endings and bash doesn't like that
# so we change CRLF line endings into LF.
sed -i 's/\r//g' dockcross-linux-arm64.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the "dockcross-linux-arm64.sh" is distributed as part of the dockcross/linux-arm64 image, so "fixing" the wrapper script would required opening a pull request against https://github.com/dockcross/dockcross, getting it accepted (maybe it does, maybe it doesn't) and then waiting for a new release of dockcross/linux-arm64 to actually get a fix. That seems as a way to lengthy process (and with uncertain outcome) to be able to rely on it.
Let's keep the sed command as is. I'll look into filing an issue with https://github.com/dockcross/dockcross so that hopefully this will get fixed in the future. But I really don't want to delay this PR because of it.


# Location of the build script in repository
build_file: "protobuf/kokoro/linux/cpp_aarch64/build.sh"
timeout_mins: 120
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, 120 is the lowest value that protobuf's kokoro .cfg files use (and it's used by many jobs including python, ruby etc.) and it's the most commonly used timeout value, so I simply reused it. (Btw, for some jobs, the timeout value is even much higher)

I can reduce this to e.g. 60 (in reality it takes only about 10mins, but it's useful to have some buffer) if you want, but basically I feel this should be done in a separate effort to set all the protobuf's kokoro jobs to a reasonable timeout (which is unrelated to this PR).


# the wrapper script has CRLF line endings and bash doesn't like that
# so we change CRLF line endings into LF.
sed -i 's/\r//g' dockcross-linux-arm64.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the "dockcross-linux-arm64.sh" is distributed as part of the dockcross/linux-arm64 image, so "fixing" the wrapper script would required opening a pull request against https://github.com/dockcross/dockcross, getting it accepted (maybe it does, maybe it doesn't) and then waiting for a new release of dockcross/linux-arm64 to actually get a fix. That seems as a way to lengthy process (and with uncertain outcome) to be able to rely on it.
Let's keep the sed command as is. I'll look into filing an issue with https://github.com/dockcross/dockcross so that hopefully this will get fixed in the future. But I really don't want to delay this PR because of it.

@jtattermusch jtattermusch merged commit 37ba0bc into protocolbuffers:master Apr 29, 2021
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.

None yet

4 participants