Teach CircleCI to build Rust wrapper #353
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We change the build system in the next commit. This removal is done separately because some older versions of git do not handle symlinks well during rebases and I got tired of having to manually resolve nonexistent merge conflicts every time I update the branch.
libthemis-src needs to embed the source code of core Themis library. Previously, we did this by using git submodules when rust-themis was developed in a separate repository. It was... satisfactory. (Not great because "cargo package" refused to package "themis" directory as it contained a Cargo.toml file.) git submodule has been converted into a symlink to the root directory when rust-themis was merged into the main repository. This should have worked well, but it didn't. First of all, the "copy_dir" crate does not copy symlinks and this breaks the build. Second, the root directory now contains Cargo's target directory which is huge and copying it as well significantly increases build times. In order to reduce build time let's copy only the essential files. Replace the quick-and-dirty "themis" symlink with a more refined selection of files necessary to compile core Themis library. We don't really need all the wrappers, documentation, etc. Just copy the source code, the build system, and some other files which are unconditionally included by the top-level makefile. (This should also significantly reduce the size of the crate.) Next, drop the "copy_dir" crate and replace it with our ad-hoc utility for copying files that handles symlinks the way we need it. It's not a general-purpose utility but it does its job well enough. And while we're here, redirect output of "make install" to /dev/null. It's annoying to look at when we run tests. Ideally we would like to redirect it into stderr, but that's not possible with Command's API. (We keep the original stderr though, so we'll see compilation errors just fine.) Unfortunately, running "cargo package" for libthemis-src is still kinda broken because Cargo does not handle symlinks. This is a known issue. Currently we will have to manually copy the source directory to resolve all the symlinks and just get a distributable files. Annoying, yes.
We install Rust using rustup which is the most common way to do this. This gets us the current stable version (updated if necessary). I would like to use the Debian package for this, but Rust is not stable enough for that yet. The language is evolving, the libraries adopt new compiler features, and the Rust team supports only the latest stable version. Therefore we go for downloading stuff from the Internet via rustup.rs. In addition to core Rust toolchain we install a couple of tools that we use for quality assurance: code formatter, static analyzer, and dead link checker. "deadlinks" take a while to compile and install, sadly. In general, Rust tooling *looooves* to pull lots of stuff from the net so we cache the downloaded packages and installed binaries if possible. This cuts down compilation time considerably, but there's still way to go. We do a clean build of Themis so we take our time to compile all the dependencies of dependencies... Aside from rustc we install a couple of native dependencies. pkg-config is required to locate Themis after installation. Clang is a dependency of bindgen crate which is used to generate bindings for Themis.
Add "test_rust" target to Makefile which runs rust-themis tests similar to other language wrappers. We skip the tests if Rust does not seem to be installed on the system. The test suite may be a bit large. We'll see how much time it takes to run on the CI. It replicates the current setup used by the original rust-themis on TravisCI [1] - Disallow any warning by any tool (the compiler and alike) - Ensure consistent code formatting with rustfmt - Avoid common mistakes by running static analysis with Clippy - Check both dynamic and static linkage to core Themis library - Compile and run automated test suite for all packages - Check that API documentation is likely to build on docs.rs - Check that documentation does not contain dead URLs [1]: https://github.com/ilammy/rust-themis/blob/master/.travis.yml
ilammy
added
infrastructure
CircleCI runs each build step in a separate shell. Therefore we lose environment variables set by "source ~/.cargo/env" during rustup installation. Append the environment configuration file to $BASH_ENV in order to set the PATH correctly for next commands. This is a special file read by CircleCI's bash.
It's actually not alright. This test is flaky and is not really safe to run. It happens to behave itself in single-threaded environment, but in reality the memory freed from the keys is instantly reused by the allocator and does not contain zeros anymore. It's not even guaranteed to be actually allocated. I guess it's fine to trust zeroize to do its thing and do not verify that memory is indeed zeroed out. Let's drop this test because flaky CI builds are bad.
Writing some random value at arbitrary offset sometimes is not enough to break Secure Cell. Internally, Secure Cell uses randomized keys so the ciphertext may actually happen to contain "42" at exactly that offset. Or so it seems to be when the tests are run by CircleCI. Try flipping the bits instead of writing a constant. We do so in documentation tests and this should be a better way to simulate corrupted data.
|
The last two commits do not really belong to this pull request. But we have to have a green build, so fix the tests here and now. I'm actually relieved that they are useful. (Nice commit hash btw, 0463666 👹) |
Lagovas
reviewed
Jan 23, 2019
I'm not a fan of this kind of installation that downloads random stuff from the Internet, but we'll have to live with it for a while, until Rust is stable enough for disto-packaged versions not becoming outdated after half a year. Let's at least add a comment about the source of the spell below so that it can be properly audited.
vixentael
requested changes
Jan 23, 2019
It turns out that we can avoid copying Themis source code. The build system supports out-of-source builds with BUILD_PATH environment variable. Just point it into OUT_DIR and Cargo should be fine. Remove all the copying utilities and blatant lies about Themis from the comments. We still keep the "optimized" symlinks instead of reverting back to a single one pointing to the root directory of the repo. This way it's more friendly towards crate packaging: "themis" directory won't have a Cargo.toml inside it and the whole crate size is smaller.
|
@ilammy please ping when this PR is ready to be reviewed again :) |
|
@vixentael @Lagovas The PR is ready for another review round. |
vixentael
approved these changes
Jan 27, 2019
|
Great job! |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Let's start checking Rust-Themis during our regular builds.
But first we have to fix a build issue with
libthemis-srccrate. There's a peculiar story there in commit messages. Anyway, this approach should work fine for now. Thanks for supporting symlinks, git! (No thanks tocargo packagefor not supporting them. I'll talk at you later.)After that we add Rust toolchain installation into the loop and actually exercise the test suite along with other automated checks. Humans can use them too, just do
tests/rust/run_tests.shbefore submitting a pull request. If that fails then it's likely to be the same on the build server.I have no idea how long this build will take on the CI. Hopefully not too long. Let's try and see... Bloated compilation times are a well-know issue with Rust. Honestly, I would not be surprised to see it taking more time than Android build. At least for the first run. Later ones may be faster due to caching, but we still have to build a metric ton of unrelated dependencies because many rustaceans seem to have been bitten by stray npm.js residents.
That's assuming the build actually works, te-he-he...