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 support for installing on NixOS #1298

Merged
merged 8 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/kani.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,10 @@ jobs:
- name: Build container test
if: ${{ matrix.os == 'ubuntu-18.04' }}
run: |
docker build -t kani-20-04 -f scripts/ci/Dockerfile.bundle-test-20-04 .
docker build -t kani-18-04 -f scripts/ci/Dockerfile.bundle-test-18-04 .
docker build -t kani-20-04 -f scripts/ci/Dockerfile.bundle-test-ubuntu-20-04 .
docker build -t kani-18-04 -f scripts/ci/Dockerfile.bundle-test-ubuntu-18-04 .
# this one does its tests automatically, for reasons documented in the file:
docker build -t kani-nixos -f scripts/ci/Dockerfile.bundle-test-nixos .

- name: Run installed tests
if: ${{ matrix.os == 'ubuntu-18.04' }}
Expand Down
19 changes: 19 additions & 0 deletions scripts/ci/Dockerfile.bundle-test-al2
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright Kani Contributors
# SPDX-License-Identifier: Apache-2.0 OR MIT

# Note: this file is intended only for testing the kani release bundle

FROM amazonlinux:2
RUN yum install -y gcc python3 python3-pip curl ctags tar gzip && \
curl -sSf https://sh.rustup.rs | sh -s -- -y
ENV PATH="/root/.cargo/bin:${PATH}"

WORKDIR /tmp/kani
COPY ./tests ./tests
COPY ./kani-latest-x86_64-unknown-linux-gnu.tar.gz ./
# Very awkward glob (not regex!) to get `kani-verifier-*` and not `kani-verifier-*.crate`
COPY ./target/package/kani-verifier-*[^e] ./kani-verifier
#RUN cargo install --path ./kani-verifier
#RUN cargo-kani setup --use-local-bundle ./kani-latest-x86_64-unknown-linux-gnu.tar.gz
RUN cargo install kani-verifier
RUN cargo-kani setup
44 changes: 44 additions & 0 deletions scripts/ci/Dockerfile.bundle-test-nixos
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Copyright Kani Contributors
# SPDX-License-Identifier: Apache-2.0 OR MIT

# Note: this file is intended only for testing the kani release bundle

FROM nixos/nix
RUN nix-channel --update
WORKDIR /tmp/kani
RUN echo $' \n\
with import <nixpkgs> {}; \n\
mkShell { \n\
packages = [ \n\
ctags \n\
curl \n\
gcc \n\
patchelf \n\
python310 \n\
python310Packages.pip \n\
rustup \n\
]; \n\
}' >> ./default.nix
# we need to switch to nix-shell to get proper support for e.g. pip
SHELL ["nix-shell", "--command"]
ENTRYPOINT ["nix-shell"]
RUN rustup toolchain add stable
ENV PATH="/root/.cargo/bin:${PATH}"

WORKDIR /tmp/kani
COPY ./tests ./tests
COPY ./kani-latest-x86_64-unknown-linux-gnu.tar.gz ./
# Very awkward glob (not regex!) to get `kani-verifier-*` and not `kani-verifier-*.crate`
COPY ./target/package/kani-verifier-*[^e] ./kani-verifier
RUN cargo install --path ./kani-verifier
RUN cargo-kani setup --use-local-bundle ./kani-latest-x86_64-unknown-linux-gnu.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan on running a quick smoke test to make sure everything was correctly patched as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I hope we'll eventually have exactly that: cargo-kani self-test #1246
  2. Right now the "smoke tests" we do can be seen here: https://github.com/model-checking/kani/blob/main/.github/workflows/kani.yml#L126-L132
  3. I have not added Nix to our CI, so the above are what I tested manually. Once 1 is resolved, I expect all these docker files to add cargo kani self-test to the end of them. Then the test would be complete just by successfully building the container with no extra steps.

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a test for standalone kani in the CI?


# Temporary hack: nix-shell causes problems when trying to run these with 'docker run'
# like we do for other tests, so we've imported these into the dockerfile for now
# until everything can be replaced with 'self-test':
# https://github.com/model-checking/kani/issues/1246
RUN cargo kani --version
RUN (cd /tmp/kani/tests/cargo-kani/simple-lib && cargo kani)
RUN (cd /tmp/kani/tests/cargo-kani/simple-visualize && cargo kani)
RUN (cd /tmp/kani/tests/cargo-kani/build-rs-works && cargo kani)
RUN cargo-kani setup --use-local-bundle ./kani-latest-x86_64-unknown-linux-gnu.tar.gz
20 changes: 20 additions & 0 deletions scripts/ci/Dockerfile.bundle-test-ubuntu-22-04
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright Kani Contributors
# SPDX-License-Identifier: Apache-2.0 OR MIT

# Note: this file is intended only for testing the kani release bundle

FROM ubuntu:22.04
ENV DEBIAN_FRONTEND=noninteractive \
DEBCONF_NONINTERACTIVE_SEEN=true
RUN apt-get update && \
apt-get install -y python3 python3-pip curl universal-ctags && \
curl -sSf https://sh.rustup.rs | sh -s -- -y
ENV PATH="/root/.cargo/bin:${PATH}"

WORKDIR /tmp/kani
COPY ./tests ./tests
COPY ./kani-latest-x86_64-unknown-linux-gnu.tar.gz ./
# Very awkward glob (not regex!) to get `kani-verifier-*` and not `kani-verifier-*.crate`
COPY ./target/package/kani-verifier-*[^e] ./kani-verifier
RUN cargo install --path ./kani-verifier
RUN cargo-kani setup --use-local-bundle ./kani-latest-x86_64-unknown-linux-gnu.tar.gz
78 changes: 77 additions & 1 deletion src/os_hacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ use std::ffi::OsString;
use std::path::Path;
use std::process::Command;

use anyhow::Result;
use anyhow::{bail, Context, Result};
use os_info::Info;

use crate::cmd::AutoRun;

Expand Down Expand Up @@ -45,3 +46,78 @@ pub fn setup_python_deps_on_ubuntu_18_04(pyroot: &Path, pkg_versions: &[&str]) -

Ok(())
}

/// This is the final step of setup, where we look for OSes that require additional setup steps
/// beyond the usual ones that we have done already.
pub fn setup_os_hacks(kani_dir: &Path, os: &Info) -> Result<()> {
match os.os_type() {
tedinski marked this conversation as resolved.
Show resolved Hide resolved
os_info::Type::NixOS => setup_nixos_patchelf(kani_dir),
os_info::Type::Linux => {
// NixOs containers are detected as Unknown Linux, so use a fallback hack:
if std::env::var_os("NIX_CC").is_some() && Path::new("/etc/nix").exists() {
return setup_nixos_patchelf(kani_dir);
}
Ok(())
}
_ => Ok(()),
}
}

/// On NixOS, the dynamic linker does not live at the standard path, and so our downloaded
/// pre-built binaries need patching.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to fix that while pre-building them to NixOS instead of patching here or do you mean that there is standard path and it is only available at runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't pre-build anything specifically for NixOS (well, except that the kani-verifier is downloaded and built locally), so this is taking the generic linux binaries and making them link properly on NixOS.

There are no standard paths, it has to be discovered at runtime. Nix seems to hack things into sorta working by injecting flags so that the linker would add rpaths to binaries. This basically replicates the effect for downloaded binaries built elsewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, that's great. :(

/// In addition, the C++ standard library (needed by the CBMC binaries we ship) also does not
/// have a standard path, and so we need to inject an rpath into those binaries to get them
/// to successfully link at runtime.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming LD_LIBRARY_PATH didn't do the trick, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An LD_LIBRARY_PATH would do the trick I think. But (1) that has to be done in a different place and it seems nicer to concentrate "os hacks" to install time things. (2) Hacking up binaries to add rpath seems to be the way this problem is fixed in the nix community's answers, and they built the patchelf tool specifically for it.

And I guess (3) this means users can add it to PATH and run cbmc manually successfully, I suppose.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah patchelf is nice since it makes it "just work" every time you invoke the command, rather than having to remember to correctly set up the LD_LIBRARY_PATH

Copy link
Contributor

Choose a reason for hiding this comment

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

If the path never changes, I agree that this is a more permanent solution, although a bit invasive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well the problem is it does change whenever the hash of the contents of the package changes. So this will work initially for when kani is set up, but if the hash of stdenv changes and the old version is garbage collected, then that path will point to a non-existent directory 😃. I think the most permanent solution is to publish kani to nixpkgs and that'll "just work" by automatically rebuilding kani anytime dependent packages change.

Copy link
Contributor

Choose a reason for hiding this comment

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

cargo-bolero was actually recently added (not even by me which is cool) so it's pretty easy to get packages in there: https://github.com/NixOs/nixpkgs/blob/master/pkgs/development/tools/rust/cargo-bolero/default.nix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see now what the question was.

Yeah, the path can change as the system updates, but we're using basically the same solution for finding the libraries as would be done for normally built binaries (for instance, for ANY binary that was cargo install'd would suffer the same fate.)

So I think when a base system update like that happens, users of things cargo install'd on NixOS are probably already familiar with needing to reinstall them?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's cool!

In the meantime, if the path changes, we should probably use LD_LIBRARY_PATH or re-patch the binaries if the value has changed. We don't expect users to invoke kani-driver or kani-compiler directly, so we might as well ensure that they always work.

Copy link
Contributor

Choose a reason for hiding this comment

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

I could be wrong, but the annoying part is that our users will have to re-install kani-verifier crate, but that won't fix the problem since the bundle we downloaded will still be on the disk. So when they run kani, they will get another crash now because kani-driver is broken, right? At that point, they will have to run kani setup to force everything to be reinstalled.

Not a blocker since nothing works today, but we should at least document that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep that's correct. But what's here is definitely an improvement. Like I said, I think for the best user experience is to get into nixpkgs.

fn setup_nixos_patchelf(kani_dir: &Path) -> Result<()> {
// Encode our assumption that we're working on x86 here, because when we add ARM
// support, we need to look for a different path.
assert!(env!("TARGET") == "x86_64-unknown-linux-gnu");
if Path::new("/lib64/ld-linux-x86-64.so.2").exists() {
// if the expected path exists, I guess things are fine?
return Ok(());
}

println!("[NixOS detected] Applying 'patchelf' to downloaded binaries");

// Find the correct dynamic linker:
// `interp=$(cat $NIX_CC/nix-support/dynamic-linker)`
let nix_cc = std::env::var_os("NIX_CC")
.context("On NixOS but 'NIX_CC` environment variable not set, couldn't apply patchelf.")?;
let path = Path::new(&nix_cc).join("nix-support/dynamic-linker");
let interp_raw = std::fs::read_to_string(path)
.context("Couldn't read $NIX_CC/nix-support/dynamic-linker")?;
let interp = interp_raw.trim();

// Find the correct path to link C++ stdlib:
// `rpath=$(nix-instantiate --eval -E "(import <nixpkgs> {}).stdenv.cc.cc.lib.outPath")/lib`
let rpath_output = Command::new("nix-instantiate")
.args(&["--eval", "-E", "(import <nixpkgs> {}).stdenv.cc.cc.lib.outPath"])
.output()?;
if !rpath_output.status.success() {
bail!("Failed to find C++ standard library with `nix-instantiate`");
}
let rpath_raw = std::str::from_utf8(&rpath_output.stdout)?;
// The output is in quotes, remove them:
let rpath_prefix = rpath_raw.trim().trim_matches('"');
let rpath = format!("{}/lib", rpath_prefix);

let patch_interp = |file: &Path| -> Result<()> {
Command::new("patchelf").args(&["--set-interpreter", interp]).arg(file).run()
};
let patch_rpath = |file: &Path| -> Result<()> {
Command::new("patchelf").args(&["--set-rpath", &rpath]).arg(file).run()
};

let bin = kani_dir.join("bin");

for filename in &["kani-compiler", "kani-driver"] {
patch_interp(&bin.join(filename))?;
}
for filename in &["cbmc", "goto-analyzer", "goto-cc", "goto-instrument", "symtab2gb"] {
let file = bin.join(filename);
patch_interp(&file)?;
patch_rpath(&file)?;
}

Ok(())
}
5 changes: 4 additions & 1 deletion src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::process::Command;
use anyhow::{bail, Context, Result};

use crate::cmd::AutoRun;
use crate::os_hacks;

/// Comes from our Cargo.toml manifest file. Must correspond to our release verion.
const VERSION: &str = env!("CARGO_PKG_VERSION");
Expand Down Expand Up @@ -48,6 +49,8 @@ pub fn setup(use_local_bundle: Option<OsString>) -> Result<()> {

setup_build_kani_prelude(&kani_dir, toolchain_version)?;

os_hacks::setup_os_hacks(&kani_dir, &os)?;

println!("[6/6] Successfully completed Kani first-time setup.");

Ok(())
Expand Down Expand Up @@ -114,7 +117,7 @@ fn setup_python_deps(kani_dir: &Path, os: &os_info::Info) -> Result<()> {
if os.os_type() == os_info::Type::Ubuntu
&& *os.version() == os_info::Version::Semantic(18, 4, 0)
{
crate::os_hacks::setup_python_deps_on_ubuntu_18_04(&pyroot, pkg_versions)?;
os_hacks::setup_python_deps_on_ubuntu_18_04(&pyroot, pkg_versions)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this to be called inside setup_os_hacks() instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. This hack is "do this step in a different way entirely" not "do an extra thing afterwards"

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh. I would expect an else case then. From reading this code it does look like do this extra thing for ubuntu 18.4.0.

return Ok(());
}

Expand Down