Fix cargo always treating nss crate as dirty by adombeck · Pull Request #1042 · canonical/authd

adombeck · 2025-08-25T20:15:46Z

I couldn't figure out exactly why, but cargo build always treated the nss crate as dirty and re-compiled it even when nothing changed in between two runs:

cargo build --verbose
[...]
Dirty nss v0.1.0 (/home/user/projects/authd/nss): the file `nss/..` has changed (1756139002.017575102s, 2s after last build at 1756139000.627582074s)
Compiling nss v0.1.0 (/home/user/projects/authd/nss)

Using "../internal/proto" instead of "../" as the includes directory in the compile_protos call fixes that.

UDENG-7825

codecov-commenter · 2025-08-25T20:29:11Z

Codecov Report

❌ Patch coverage is 61.53846% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.04%. Comparing base (21e891f) to head (3d2b788).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/fileutils/fileutils.go	61.53%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1042      +/-   ##
==========================================
+ Coverage   88.00%   88.04%   +0.04%     
==========================================
  Files          85       85              
  Lines        6026     6039      +13     
  Branches      111      111              
==========================================
+ Hits         5303     5317      +14     
- Misses        663      666       +3     
+ Partials       60       56       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

adombeck · 2025-08-26T08:46:17Z

internal/testutils/rust.go

-	// Note that for developing purposes and avoiding keeping building the rust program dependencies,
-	// TEST_RUST_TARGET environment variable can be set to an absolute path to keep iterative
-	// build artifacts.
-	target := os.Getenv("TEST_RUST_TARGET")
-	if target == "" {
-		target = t.TempDir()
-	}
+	// Store the build artifacts in a common temp directory, so that they can be reused between tests.
+	target := filepath.Join(os.TempDir(), "authd-tests-rust-build-artifacts")


@3v1n0 I would like to avoid re-building Rust dependencies by default, because I keep forgetting to set TEST_RUST_TARGET when I run the tests. Do you see any cases where we shouldn't re-use build artifacts between tests?

So, the tests use different library builds, as per different features.

For example, SSH tests use different flags compared to the NSS library, and doing this we may end up creating problems between them (races as one library may being rebuilt while used).

One thing you can do is adding to the the target path the features too, so that it will be unique.

Ideally I still would like to cleanup the path by default though, unless requested by the user, as leaving artifacts around is not nice in general.

So, the tests use different library builds, as per different features.

For example, SSH tests use different flags compared to the NSS library, and doing this we may end up creating problems between them (races as one library may being rebuilt while used).

cargo is smart enough to rebuild the affected artifacts when different features are selected. You can try running

cargo build --verbose --features integration_tests,custom_socket

in the authd repo and afterwards

cargo build --verbose --features integration_tests

you should see:

Dirty nss v0.1.0 (/home/adrian.dombeck@gmail.com/projects/authd/nss): the list of features changed Compiling nss v0.1.0 (/home/adrian.dombeck@gmail.com/projects/authd/nss)

Ideally I still would like to cleanup the path by default though, unless requested by the user, as leaving artifacts around is not nice in general.

As a user of this test, I very much prefer a few MB of artifacts lying around in /tmp over having to wait ~30s longer each time I run one of the affected tests, so I'm in favor of keeping the artifacts by default. We could add an environment variable to delete them if you think that's useful.

cargo is smart enough to rebuild the affected artifacts when different features are selected.

Sure, I'm aware of it, but we should not use the same build directory for different artifacts, no?

Otherwise we may have a test using features that another test is concurrently not in the need for

Ah, now I get what you mean. I think we could avoid that by:

Ensuring that only one build runs at a time, and

Copying the resulting .so to a directory that includes the features and other relevant options in its name

done in commit tests: Avoid races when building/using the NSS library

internal/fileutils/fileutils.go

internal/fileutils/fileutils_test.go

internal/testutils/cmd.go

internal/testutils/rust.go

3v1n0 · 2025-08-27T22:01:01Z

internal/testutils/rust.go

+	// Store the build artifacts in a common temp directory, so that they can be reused between tests.
+	target := filepath.Join(os.TempDir(), "authd-tests-rust-build-artifacts")
+	if v := os.Getenv("TEST_RUST_TARGET"); v != "" {
+		target = v


Suggested change

target = v

target := os.Getenv("TEST_RUST_TARGET")

if target == "" {

target = filepath.Join(os.TempDir(), "authd-tests-rust-build-artifacts")

Otherwise we create a dir for no eason

internal/testutils/rust.go

Rebuilding the NSS library takes an unexpectedly long time, even if the code hasn't changed. Let's print the output of the cargo build command to get a better picture of what's going on.

I couldn't figure out exactly why, but `cargo build` always treated the nss crate as dirty and re-compiled it even when nothing changed in between two runs: cargo build --verbose [...] Dirty nss v0.1.0 (/home/user/projects/authd/nss): the file `nss/..` has changed (1756139002.017575102s, 2s after last build at 1756139000.627582074s) Compiling nss v0.1.0 (/home/user/projects/authd/nss) Using "../internal/proto" instead of "../" as the includes directory in the compile_protos call fixes that.

Now that the issue that the library was rebuilt each time has been solved, we don't the verbose output anymore.

Speeds up the tests if the developer did not set TEST_RUST_TARGET in the test environment.

* Lock the target directory to avoid multiple tests running in parallel trying to build the library at the same time (possibly with different set of features or other options). * Copy the resulting library to a temporary directory only used by the current test, to allow other tests to reuse the target directory (so that they don't have to rebuild it from scratch).

adombeck force-pushed the improve-integration-test-runtime branch from 969a327 to 2a47fd8 Compare August 25, 2025 20:16

adombeck force-pushed the improve-integration-test-runtime branch 3 times, most recently from e2e27bc to dfd11fb Compare August 25, 2025 22:03

adombeck commented Aug 26, 2025

View reviewed changes

adombeck force-pushed the improve-integration-test-runtime branch 2 times, most recently from 6c238af to 18842d4 Compare August 27, 2025 11:29

3v1n0 reviewed Aug 27, 2025

View reviewed changes

adombeck force-pushed the improve-integration-test-runtime branch from d0e51b1 to 1c1422d Compare August 27, 2025 14:55

adombeck requested a review from 3v1n0 August 27, 2025 14:59

adombeck marked this pull request as ready for review August 27, 2025 17:25

adombeck requested a review from a team as a code owner August 27, 2025 17:25

adombeck force-pushed the improve-integration-test-runtime branch from 1c1422d to d137268 Compare August 27, 2025 17:25

3v1n0 reviewed Aug 27, 2025

View reviewed changes

adombeck force-pushed the improve-integration-test-runtime branch from d137268 to 0609f97 Compare August 27, 2025 22:13

adombeck requested a review from 3v1n0 August 27, 2025 22:14

adombeck mentioned this pull request Aug 28, 2025

Make SSH integration tests easier to debug #1046

Merged

adombeck force-pushed the improve-integration-test-runtime branch from 0609f97 to 46793c9 Compare August 29, 2025 10:01

3v1n0 approved these changes Aug 29, 2025

View reviewed changes

adombeck added 7 commits September 1, 2025 15:10

tests: Print output of cargo build command

8e48857

Rebuilding the NSS library takes an unexpectedly long time, even if the code hasn't changed. Let's print the output of the cargo build command to get a better picture of what's going on.

tests: Build NSS library without verbose flag

d2545da

Now that the issue that the library was rebuilt each time has been solved, we don't the verbose output anymore.

tests: Re-use Rust build artifacts by default

b990c4d

Speeds up the tests if the developer did not set TEST_RUST_TARGET in the test environment.

fileutils: Add LockDir

ff494b8

tests: Run cargo with --verbose flag if AUTHD_TEST_VERBOSITY is set

3d2b788

adombeck force-pushed the improve-integration-test-runtime branch from 46793c9 to 3d2b788 Compare September 1, 2025 13:10

adombeck merged commit a30d2fd into main Sep 1, 2025
13 of 14 checks passed

adombeck deleted the improve-integration-test-runtime branch September 1, 2025 13:14

Conversation

adombeck commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

adombeck Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

3v1n0 Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adombeck Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

adombeck Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

3v1n0 Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

adombeck Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adombeck Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

3v1n0 Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

adombeck Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adombeck commented Aug 25, 2025 •

edited

Loading

codecov-commenter commented Aug 25, 2025 •

edited

Loading

3v1n0 Aug 26, 2025 •

edited

Loading

adombeck Aug 26, 2025 •

edited

Loading