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

How to verify individual crates build (and ideally pass tests) for distro packaging #3732

Closed
ignatenkobrain opened this issue Feb 18, 2017 · 18 comments
Assignees
Labels
A-crate-dependencies Area: [dependencies] of any kind C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-build Command-test Z-build-plan Nightly: --build-plan feature

Comments

@ignatenkobrain
Copy link
Contributor

In Fedora we're packaging each crate and there are definitely circular dependencies in dev-dependencies (memchr -> quickcheck(dev) -> regex -> memchr), so we first need to build memchr, then regex, then quickcheck and then we can rebuild memchr with enabled cargo-test.

We override crates.io registry, but cargo-build still tries to find quickcheck:

+ /usr/bin/cargo build --release -j8
error: no matching package named `quickcheck` found (required by `memchr`)
location searched: registry https://github.com/rust-lang/crates.io-index
version required: ^0.4.1

But build doesn't really need that dependency to be present...

@ignatenkobrain
Copy link
Contributor Author

cargo install is affected as well.

@alexcrichton
Copy link
Member

I think that cargo build may not be what you want to use here, as a critical part of its operation is indeed verifying dev-dependencies exist. Could you describe in a bit more detail what's happening here? I agree that packaging typically shouldn't worry with dev-dependencies, but that means you're going to need to take a bit of a different route through Cargo that's not the default one.

@ignatenkobrain
Copy link
Contributor Author

@alexcrichton we are trying to verify that crate works itself (for libs) and to build real thing (for bins). While it can be partially lying on first category, second one is definitely the case.

@alexcrichton
Copy link
Member

@ignatenkobrain can you give me some more information? What does it mean "verify crate works itself"? Why/where are you validating libraries? How is this specific to fedora? etc...

@ignatenkobrain
Copy link
Contributor Author

/usr/bin/mkdir -p .cargo 
cat > .cargo/config << EOF 
[build]
rustc = "/usr/bin/rustc"
rustflags = ["-Copt-level=3", "-g", "-Clink-arg=-Wl,-z,relro,-z,now", ]

[term]
verbose = true

[source]

[source.local-registry]
directory = "/usr/src/rust"

[source.crates-io]
registry = "https://crates.io"
replace-with = "local-registry"
EOF

/usr/bin/cargo build --release -j8

if /usr/bin/cargo-inspector --target-kinds Cargo.toml | grep -q -F "$(printf 'lib\nproc-macro')"; then                                                  
  CRATE_NAME=$(/usr/bin/cargo-inspector --name Cargo.toml)                      
  CRATE_VERSION=$(/usr/bin/cargo-inspector --version Cargo.toml)                
  REG_DIR=/home/brain/rpmbuild/BUILDROOT/aho-corasick-0.6.2-1.fc26.x86_64/usr/src/rust/$CRATE_NAME-$CRATE_VERSION        
  /usr/bin/mkdir -p $REG_DIR                                                  
  /usr/bin/cargo package -l | xargs cp --parents -p -t $REG_DIR                 
  /usr/bin/install -p Cargo.toml.orig $REG_DIR/Cargo.toml                     
  echo '{"files":{},"package":""}' > $REG_DIR/.cargo-checksum.json        
fi 
if /usr/bin/cargo-inspector --target-kinds Cargo.toml | grep -q -F bin; then                                                  
  /usr/bin/cargo install -j8 --path . --root /home/brain/rpmbuild/BUILDROOT/aho-corasick-0.6.2-1.fc26.x86_64/usr 
  /usr/bin/rm /home/brain/rpmbuild/BUILDROOT/aho-corasick-0.6.2-1.fc26.x86_64/usr/.crates.toml                             
fi 

/usr/bin/cargo test --release -j8

That's what we do.

  1. Replace registry with our custom one
  2. Run cargo build (to ensure that at least thing builds)
  3. If crate is library - copy all files into registry
  4. If crate is binary - run cargo install
  5. Run cargo test

Why do we "verify crate works itself"? Because we don't want to package "cat in box", if crate can't do build either it's nightly or it's broken. That's the reason why we also run cargo test.

While running test suite sometimes broken (#3642), at least cargo build should work (we don't package windows-only crates and nightly ones, we don't want to package everything, only real and needed things).

@ignatenkobrain
Copy link
Contributor Author

P.S. cargo-inspector is our own thing which runs cargo metadata and parses json for our needs. In case above it prints all kind: [...] values

@ignatenkobrain
Copy link
Contributor Author

What is our workaround now:

%if ! %{with check}
# https://github.com/rust-lang/cargo/issues/3732
gawk -i inplace -v INPLACE_SUFFIX=.orig '/^\[dev-dependencies\]$/{f=1;next} /^\[/{f=0}; !f' Cargo.toml
%endif

Basically if %check is disabled in RPM, then we remove [dev-dependencies] section completely, so cargo build builds thing instead of complaining about missing dependencies from dev section (because they are not in our registry).

@alexcrichton
Copy link
Member

@ignatenkobrain ok thanks for the info!

The tl;dr; I believe is that unfortunately Cargo just doesn't support this use case today. I don't currently know the best way to doing so as well.

@cuviper
Copy link
Member

cuviper commented Mar 2, 2017

With a broader view, I think the basic concept of what we'd want is already there in the "ephemeral" flags in cargo. That is, when dealing with packaging, we want to manage a single standalone crate much the same way that cargo package and cargo install do it.

Maybe something like an --ephemeral option could be added to build/test/metadata/etc.? If I understand the current ephemeral code, in this mode it would not consider workspaces or path deps, and dev-deps would only be pulled in if actually running tests.

#3369 is related too, I think.

@awalcutt
Copy link

awalcutt commented Mar 3, 2017

Lending my experiences in case it's useful for drafting/prioritizing features.

We import third-party packages as source to our internal repository for various security, legal, and operational reasons (patching, license compliance, etc.).

Our import tool finds a specified package on crates.io, recursively determines what dependencies need to be imported at the same time, and generates configuration to glue it to our in-house build system. When source code is updated (or initially imported) we automatically build it and store the artifacts corresponding to that version/target/etc.

This currently breaks on certain crates like libc which defines workspace packages that aren't included in the crate itself. It seems like our options are:

  • Remove stuff from Cargo.toml on import to allow for building
  • Modify our build tool to use a temporary Cargo.toml while keeping the source "prisitine"
  • Abandon cargo magic and just use straight rustc
  • Try to import from the source repository (e.g. github) where releases may not be well defined

Rust tooling at my company is community maintained and some of it is old so I'm open to hearing alternative approaches to this problem. As interest increases, removing these hurdles will go a long way towards encouraging more adoption in real projects.

P.S. This doesn't reflect the views of my employer.

@joshtriplett
Copy link
Member

joshtriplett commented Mar 3, 2017

@cuviper @awalcutt I think the patch in #3369 may solve your problem, along with one other change.

For Debian packaging, we construct a directory registry, which includes the sources of all dependencies (recursively), including the current crate. We then run cargo install cratename-version to install the crate from that registry. On the advice of @alexcrichton, we don't directly run cargo install from the current directory.

That should help you solve the bootstrapping problem. Once you've bootstrapped, you may be able to opt into running cargo test on a per-package basis, for crates that can support running the testsuite. However, some crates don't support that except as part of a workspace, or with additional dependencies installed that you won't want, or even with additional material not included on crates.io. In the general case, you can only count on cargo install cratename-version from a directory registry loaded with the sources.

@cuviper
Copy link
Member

cuviper commented Mar 3, 2017

Yes, we are making a system-wide directory registry in Fedora. As I understand Debian's scheme, you have the system-wide location too, which you then symlink into the builddir along with the current crate. But even if we follow the same scheme, how can you build/test a library-only crate within a local directory registry? We'd like to have some measure of confidence that the crate is working before we get all the way to a leaf crate to cargo install. (Setting aside the issue of those that can't be tested due to missing assets, etc.)

The fact that cargo install and cargo package can do this tells me we're almost there. It has the logic to build things in a standalone way, so I hope with some option we can make that more widely available. If we can just run cargo build --ephemeral/standalone/w.e. in the current directory, leaving all the dependencies in the system-wide registry, then I think that will be a much cleaner configuration.

@brson
Copy link
Contributor

brson commented Mar 3, 2017

No context here, but I see that people are discussing how to make published Cargo.toml's buildable. FWIW here is how I modify toml files in cargobomb.

@joshtriplett
Copy link
Member

@cuviper At the moment, we don't. I'd like to run not just cargo build but a full cargo test, as both a test on the target architecture and a test that the build using system libraries works correctly. However, that doesn't reliably work, not just because of dev-dependencies (which you'd actually need for tests) but because of workspaces and similar.

I'd welcome an option that made a build from a local directory equivalent to one from the registry, but even with that, cargo test wouldn't work reliably. And if I can't run cargo test, I don't really care that much about build-only testing. Partly because I expect the developer to build-test before uploading, and partly because I expect crates on crates.io to build.

@cuviper
Copy link
Member

cuviper commented Mar 3, 2017

@brson It's cool that you've automated this frobbing for cargobomb, but I think we'll all be better served if cargo gains a direct option to ignore those things.

@joshtriplett I do think it's still worth doing build-only testing, because said distro developers may still make mistakes, e.g. forgetting to declare a system dependency in the spec. And of course the crates.io author may be building on a totally different platform.

@carols10cents carols10cents added Z-build-plan Nightly: --build-plan feature A-crate-dependencies Area: [dependencies] of any kind C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-build Command-test labels Oct 2, 2017
@carols10cents carols10cents changed the title cargo-build should not enforce dev-dependencies to be present How to verify individual crates build (and ideally pass tests) for distro packaging Oct 2, 2017
@alexcrichton
Copy link
Member

I believe a number of issues here were fixed with #4030, but are there more remaining?

AndersBlomdell pushed a commit to AndersBlomdell/rust2rpm that referenced this issue Apr 17, 2018
All background written in upstream cargo GitHub issue[0].

In short, cargo build/install enforces us to have all dev-dependencies
even they are not used for building/installed.

[0] rust-lang/cargo#3732

Signed-off-by: Igor Gnatenko <ignatenkobrain@fedoraproject.org>
@dwijnand
Copy link
Member

I believe a number of issues here were fixed with #4030, but are there more remaining?

Optimistically close, and reopen if need be?

@alexcrichton
Copy link
Member

Sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-crate-dependencies Area: [dependencies] of any kind C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-build Command-test Z-build-plan Nightly: --build-plan feature
Projects
None yet
Development

No branches or pull requests

8 participants