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

Add cross-compilation and testing to Arm64 on CI #6

Merged
merged 1 commit into from
Nov 1, 2019

Conversation

hug-dev
Copy link
Member

@hug-dev hug-dev commented Oct 31, 2019

Modifies the code so that the crate only writes to the OUT_DIR directory
so it can be used with cross.
Add cross-compilation and testing on the CI.

@hug-dev hug-dev added the enhancement New feature or request label Oct 31, 2019
@hug-dev hug-dev self-assigned this Oct 31, 2019
@hug-dev
Copy link
Member Author

hug-dev commented Oct 31, 2019

@ionut-arm I use Command to directly call wget and tar, maybe there is a better way to do this? Because of this, I have to add a Dockerfile that installs wget 💣

Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

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

Looks good, cleaner code overall! I'm not a fan of not being able to easily see the generated code, but this is a decision the Rust team made for us

.arg(PARSEC_OPERATIONS_VERSION)
.arg("target/parsec-operations")
.output()
if !Command::new("wget")
Copy link
Member

Choose a reason for hiding this comment

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

Curl might be easier to use. Using Rust only

Copy link
Member

Choose a reason for hiding this comment

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

Also, if you manage to use curl, you don't need the whole shenanigans of installing wget/having a special Docker image.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, I will look into using that one.

}

// Gets extracted as parsec-operations-PARSEC_OPERATIONS_VERSION directory.
if !Command::new("tar")
Copy link
Member

Choose a reason for hiding this comment

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

You have bindings to tar as a library too.

@@ -26,8 +26,28 @@ mod convert_asym_verify;
mod convert_list_providers;
mod convert_list_opcodes;

// Include the Rust generated file in its own module.
macro_rules! include_protobuf_as_module {
Copy link
Member

Choose a reason for hiding this comment

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

Nice 😎

Copy link
Member

Choose a reason for hiding this comment

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

You can move this macro definition inside the generated_ops module, don't think you'd be using it anywhere else, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, good point, will move!

build.rs Outdated

const PROTO_FOLDER: &str = "target/parsec-operations/protobuf";
const PROTO_OUT_DIR: &str = "src/operations_protobuf/generated_ops";
const PARSEC_OPERATIONS_VERSION: &str = "0.2.0";

// TODO: handle OsStrings more carefully, as .into_string() might fail

fn generate_mod_file() -> Result<()> {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need this file anymore, you've replaced it with a fully-contained module in src/operations_protobuf/mod.rs

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh very good point! I will remove the whole function.

Modifies the code so that the crate only writes to the OUT_DIR directory
so it can be used with cross.
Add cross-compilation and testing on the CI.

Signed-off-by: Hugues de Valon <hugues.devalon@arm.com>
@hug-dev
Copy link
Member Author

hug-dev commented Oct 31, 2019

I did not modify the direct command calls of wget and tar as I was facing the following error when trying to use the clang crate in the build script:

  = note: /usr/bin/ld: /target/debug/deps/libopenssl_sys-07dcfde38d575d2d.rlib(s3_clnt.o): Relocations in generic ELF (EM: 183)
          /usr/bin/ld: /target/debug/deps/libopenssl_sys-07dcfde38d575d2d.rlib(s3_clnt.o): Relocations in generic ELF (EM: 183)
          /target/debug/deps/libopenssl_sys-07dcfde38d575d2d.rlib: error adding symbols: File in wrong format
          collect2: error: ld returned 1 exit status

I think there might be a clash between the OpenSSL library on the host and the one used in the Docker container so that linking fails. This will need more investigating.

@hug-dev hug-dev merged commit 155df36 into parallaxsecond:master Nov 1, 2019
@hug-dev hug-dev deleted the cross-compilation branch November 1, 2019 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants