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

Test that symbol version names are correct #8

Open
joshlf opened this Issue Nov 14, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@joshlf
Member

joshlf commented Nov 14, 2018

Currently, there's no way to automatically test that the version number in link and link_name attributes in boringssl/boringssl.rs match the version number in Cargo.toml. This has already led to an issue: we published 0.2.1 with the version numbers in boringssl/boringssl.rs still at 0.2.0, forcing us to fix and release 0.2.2, and yank 0.2.1. We should add an automated test so this sort of issue can't slip through in the future.

@codeworm96

This comment has been minimized.

codeworm96 commented Nov 29, 2018

I would like to look at this problem.

@joshlf

This comment has been minimized.

Member

joshlf commented Nov 29, 2018

Nobody else is working on it to my knowledge, so it's all yours!

@joshlf

This comment has been minimized.

Member

joshlf commented Nov 29, 2018

Our existing test_symbol_conflict.sh script should make sure that all symbols are prefixed. So all that's left to do is to test to make sure it's the right prefix. This involves two things, both of which can be checked in boringssl.rs:

  • Each link_name attribute should start with __RUST_MUNDANE_X_Y_Z_, where X.Y.Z is the current crate version
  • The #[link(name = "crypto_X_Y_Z")] attribute at the top of the file should have the correct version

It's up to you where you want to put this test. It could be a separate script, or it could go in test_symbol_conflict.sh. If you want to do something really cool, you could make it run as part of cargo test.

@codeworm96

This comment has been minimized.

codeworm96 commented Dec 3, 2018

I see. I prefer to make it part of cargo test because it is convenient and cool.
Is regex matching enough for this check, or we need to do some kind of parsing of the source code?
(With the help of crates like syn, it seems not too hard, but may be too heavy for this check...)

@joshlf

This comment has been minimized.

Member

joshlf commented Dec 3, 2018

I think it should be sufficient to find every instance of a link_name attribute and ensure that it matches the regex #\[link_name = "__RUST_MUNDANE_X_Y_Z_[A-Za-z0-9_]+"\]. For the link attribute, it should be sufficient to ensure that the exact string #[link(name = "crypto_X_Y_Z")] appears exactly once in the file.

As for how to do that, I'd advise a test in the tests directory that calls out to a shell script. I would expect it to be much easier to write the test itself using a shell script than in pure Rust.

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