389-ds-base

#50633 Add cargo vendor support for offline builds
Closed by spichugi. Opened by firstyear.
firstyear/389-ds-base 20191003-rust-vendor  into  master

Download 50633.patch

Bug Description: At suse/rh we need to be able to build offline. To
achieve this we need offline builds. This adds support for these in
389-ds with cargo and rust.

Fix Description:
This adds cargo vendor support for offline builds,
and shows that they work. We add a stub library for librslapd/libslapd
so that we can begin to develop features in rust.

To build normally: work as usual.

To build offline: make -f rpm.mk download-cargo-dependencies
./configure --enable-rust --enable-rust-offline

Continue to build as usual.

A note to keep in mind is cargo test does not work offline as
dev-dependencies are not vendored.

The download-cargo-dependencies has been added to dist-bz2 for
distributions.

rebased onto ee63b8a130b295ff0c8ac23eff4e4f7fcdd10dd4

For npm packages we run npm audit-ci during tarball generation to prevent including packages with known vulnerabilities. It would be nice to do the same for cargo packages using cargo audit.

I have no experience with cargo audit at the moment - one step at a time please!

I still have to fix an issue where symbols from the rust libs can't be used in ns-slapd (only inside of libslapd right now), so there is to be some more churn here I expect.

Did you want this to be a whole commit of everything or did we want to do this in steps such as this with vendor, another step with audit, and finally a step that fixes the ns-slapd linking? I'm okay either way.

1 new commit added

  • Update to support testing with --offline --locked

I have no experience with cargo audit at the moment - one step at a time please!

That's why I said it'd be nice to have, not must to have. It would be must have once we start building with rust by default in downstream, as we want to protect from supply chain attacks.

Did you want this to be a whole commit of everything or did we want to do this in steps such as this with vendor, another step with audit, and finally a step that fixes the ns-slapd linking? I'm okay either way.

I'm fine having it separately as long as it's not breaking builds with current defaults (i.e. without rust).

Testing with HEAD at 1392b272.

First, mere autoreconf -fiv && ./configure --enable-rust && make gives me this (hint: it does no Rust compilation beforehand):

ldap/servers/slapd/libglobs.c:137:10: fatal error: rust-slapi-private.h: No such file or directory
  137 | #include "rust-slapi-private.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~

Second, I would fix this, otherwise it says it does not know the option:

diff --git a/configure.ac b/configure.ac
index b6eae5b4b..4e13b776f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,7 +102,7 @@ fi
 AM_CONDITIONAL([RUST_ENABLE],[test "$enable_rust" = yes])
 AC_MSG_CHECKING(for --enable-rust-offline)
-AC_ARG_ENABLE(rust, AS_HELP_STRING([--enable-rust-offline], [Enable rust building offline. you MUST have run vendor! (default: no)]),
+AC_ARG_ENABLE(rust_offline, AS_HELP_STRING([--enable-rust-offline], [Enable rust building offline. you MUST have run vendor! (default: no)]),
               [], [ enable_rust_offline=no ])
 AC_MSG_RESULT($enable_rust_offline)
 AM_CONDITIONAL([RUST_ENABLE_OFFLINE],[test "$enable_rust_offline" = yes])

but even then, the autoreconf -fiv && make -f rpm.mk download-cargo-dependencies && ./configure --enable-rust --enable-rust-offline && make does not work failing as in the First. Am I doing something wrong?

I wonder if I forgot an ifdef around that header maybe?

So you suggest moving the --enable-rust-offline to outsideo of the enable-rust ac_arg?

Ahhhh the header is generated from build.rs, that's why it's not working. Okay need to think about how to approach this.

1 new commit added

  • Fix up issues that Matus found in build with clean env

This commit should fix your issue :)

Ping @mhonek :)

Unnecessary spacing.

Unnecessary spacing.

Sorry for the delay. Here's couple of comments.

--enable-rust fails

I think it should not rely on offline vendor present:

perl ./ldap/servers/slapd/mkDBErrStrs.pl -i /usr/include/libdb -o .
RUST_BACKTRACE=1 RUSTC_BOOTSTRAP=1 \
CARGO_TARGET_DIR=/home/vagrant/ds/rs \
SLAPD_HEADER_DIR=/home/vagrant/ds/ \
    cargo rustc  --manifest-path=./src/librslapd/Cargo.toml \
    --release --verbose -- -C debuginfo=2
error: failed to load source for a dependency on `cbindgen`
Caused by:
  Unable to update registry `https://github.com/rust-lang/crates.io-index`
Caused by:
  failed to update replaced source registry `https://github.com/rust-lang/crates.io-index`
Caused by:
  failed to read root of directory source: /home/vagrant/ds/vendor
Caused by:
  No such file or directory (os error 2)
make: *** [Makefile:14571: /home/vagrant/ds/rs/release/librslapd.a] Error 101

./configure --enable-rust-offline

Does not seem it actually builds the Rust stuff at all. This is unexpected. AFAIU this is a toggle for when already building with Rust -- in this case I would guard (i.e. fail if incorrect) the usage of this flag by a requirement to have the --enable-rust flag enabled as well.

./configure --enable-rust --enable-rust-offline

Without pre-downloading using the make -f rpm.mk download-cargo-dependencies this fails with:

CARGO_TARGET_DIR=/home/vagrant/ds/rs \
SLAPD_HEADER_DIR=/home/vagrant/ds/ \
    cargo rustc --locked --offline --manifest-path=./src/librslapd/Cargo.toml \
    --release --verbose -- -C debuginfo=2
error: can't update in the offline mode
make: *** [Makefile:14571: /home/vagrant/ds/rs/release/librslapd.a] Error 101

I think we should fail at ./configure time when building with --enable-rust-offline and do not have /vendor prerequisites already downloaded, saying "You should download the prerequisites using make -f rpm.mk download-cargo-dependencies, first."

Finally, given the prerequisites have been pre-downloaded, the things seem to build just OK. :)

Couple of other issues

.gitignore -- missing vendor folder

Please add /vendor in there.

SPEC file not updated

We should probably address this in a separate ticket. William, could you please open a simple ticket for addressing these Rust changes in the SPEC file? Also, it would be great if you could link to e.g. the SUSE's SPEC file on how to actually ship this (even though it's still just a PoC).

Sorry for the delay. Here's couple of comments.
--enable-rust fails
I think it should not rely on offline vendor present:
perl ./ldap/servers/slapd/mkDBErrStrs.pl -i /usr/include/libdb -o .
RUST_BACKTRACE=1 RUSTC_BOOTSTRAP=1 \
CARGO_TARGET_DIR=/home/vagrant/ds/rs \
SLAPD_HEADER_DIR=/home/vagrant/ds/ \
cargo rustc --manifest-path=./src/librslapd/Cargo.toml \
--release --verbose -- -C debuginfo=2
error: failed to load source for a dependency on cbindgen

Caused by:
Unable to update registry https://github.com/rust-lang/crates.io-index

Caused by:
failed to update replaced source registry https://github.com/rust-lang/crates.io-index

Caused by:
failed to read root of directory source: /home/vagrant/ds/vendor

Ahhh because the .cargo/config is there so it thinks it needs vendoring always, not just when you use --offline. Errghh. Sounds like yet another issue for me to raise on cargo.

So far I'm finding this whole vendoring process really incomplete.

Caused by:
No such file or directory (os error 2)
make: *** [Makefile:14571: /home/vagrant/ds/rs/release/librslapd.a] Error 101

./configure --enable-rust-offline
Does not seem it actually builds the Rust stuff at all. This is unexpected. AFAIU this is a toggle for when already building with Rust -- in this case I would guard (i.e. fail if incorrect) the usage of this flag by a requirement to have the --enable-rust flag enabled as well.

Yep, that's a good comment, I can fix this easily. .

./configure --enable-rust --enable-rust-offline
Without pre-downloading using the make -f rpm.mk download-cargo-dependencies this fails with:
CARGO_TARGET_DIR=/home/vagrant/ds/rs \
SLAPD_HEADER_DIR=/home/vagrant/ds/ \
cargo rustc --locked --offline --manifest-path=./src/librslapd/Cargo.toml \
--release --verbose -- -C debuginfo=2
error: can't update in the offline mode
make: *** [Makefile:14571: /home/vagrant/ds/rs/release/librslapd.a] Error 101

I think we should fail at ./configure time when building with --enable-rust-offline and do not have /vendor prerequisites already downloaded, saying "You should download the prerequisites using make -f rpm.mk download-cargo-dependencies, first."
Finally, given the prerequisites have been pre-downloaded, the things seem to build just OK. :)

Yep, we could do a test for the folder,

Couple of other issues
.gitignore -- missing vendor folder
Please add /vendor in there.
SPEC file not updated

I didn't want to update the spec yet because I didn't know how we wanted to tackle making rust a requirement for RHEL/SUSE/Others yet.

We should probably address this in a separate ticket. William, could you please open a simple ticket for addressing these Rust changes in the SPEC file? Also, it would be great if you could link to e.g. the SUSE's SPEC file on how to actually ship this (even though it's still just a PoC).

Sure. I think that ticket would be "include rust by default" but good to have a ticket + spec changes theres.

https://github.com/rust-lang/cargo/issues/7520 Related issue for upstream cargo

So it looks like there is no variables in configure.ac for the source directory, which means it's not possible to check if vendor directory exists until we get to make, and we already error in various ways there. It's not perfect, but I think I can't solve that.

I've updated so that --enable-rust-offline will also set RUST_ENABLE to true.

Finally, see https://pagure.io/389-ds-base/issue/50662

rebased onto 77219e8c76122f7ace3714f869c52b10aa624bb5

So offline works, but as your note from 7520 there is issues with online build with the .cargo/config present. For now we just have to advise that people run: cd 389-ds; make -f rpm.mk download-cargo-dependencies first

@mhonek any further comments here? I think we should merge what we have because we're waiting on upstream to improve cargo at this point.

@mhonek ?

Mate, I have not elaborated enough to prove, yet, however I think it would be posible to actually create the .cargo/config as an artifact when building with --enable-rust-offline thus not having it as an obstacle when building online. Also, maybe also moving the two targets from rpm.mk to Makefile.am to be run as dependencies when --enable-rust-offline is used, since not running them in this case does not make sense.

However, if you're blocked by this (sorry :/), then go ahead with merging. We can do adjustments later.

Well, if we put vendoring into --enable-rust-offline that would kind of create a chicken and egg problem right?

So having vendoring external to the build makes sense. If anything the .cargo/config should be generated by the rpm.mk in that case (and possibly removed by makefile if online)?.

Anyway, I'll rebase and merge this for now.

rebased onto cc8bfec02d2ae6e80f48ce8a505e312471225f00

Pull-Request has been merged by firstyear

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3688

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.