Skip to content

Comments

build: saves ~11 seconds build time#1498

Closed
helio-frota wants to merge 1 commit intoguacsec:mainfrom
helio-frota:issue-1323
Closed

build: saves ~11 seconds build time#1498
helio-frota wants to merge 1 commit intoguacsec:mainfrom
helio-frota:issue-1323

Conversation

@helio-frota
Copy link
Contributor

Please let me know with 👍 or 👎 or PR review rejection/approval if this worth to have a bit verbose Cargo.toml to save ~15 seconds during build.

More details you can see here: #1323 (comment)

@helio-frota
Copy link
Contributor Author

Update: now 170.3s (2m 50.3s) ~11 seconds got tests broken , interestingly the build pass and we have a runtime error in tests...

@helio-frota helio-frota changed the title build: saves ~15 seconds build time build: saves ~11 seconds build time Mar 31, 2025
@JimFuller-RedHat
Copy link
Contributor

I would first fix tests and whatever runtime error (maybe good to raise issue for each as well).

@ctron
Copy link
Contributor

ctron commented Apr 1, 2025

I'm not sure saving 11 seconds build time really is that important, adding that extra complexity of maintaining all those feature flags. On the other side, it may be more for a --release build. And it may have a positive impact on the release binary? I think those things should be checked as well.

@helio-frota
Copy link
Contributor Author

thanks for the feedback folks

I would first fix tests and whatever runtime error (maybe good to raise issue for each as well).

yeah that was fixed yesterday. It was caused by:

- git2 = { version = "0.20.0", default-features = false, features = ["ssh"] }
+ git2 = { version = "0.20.0", features = ["ssh"] }

probably related to the missing https feature and the runtime error in tests fixed yesterday removing default-features = false

I'm not sure saving 11 seconds build time really is that important, adding that extra complexity of maintaining all those feature flags.

yeah that's why I 👎

On the other side, it may be more for a --release build. And it may have a positive impact on the release binary? I think those things should be checked as well.

We have the same result without --release

Before:
2025-04-01_07-34

After:
2025-04-01_07-44

@JimFuller-RedHat @ctron let's close the the PR and the issue then?

@ctron
Copy link
Contributor

ctron commented Apr 2, 2025

Yea, if the size of the release binary doesn't show any significant different either, let's close it.

@helio-frota
Copy link
Contributor Author

ok closing 👍

➜  trustify git:(issue-1323) ls -sh target/release/trustd
114M target/release/trustd*
➜  trustify git:(issue-1323) cargo build --no-default-features --release

@helio-frota helio-frota closed this Apr 2, 2025
@helio-frota helio-frota deleted the issue-1323 branch April 2, 2025 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Would be good to make sure we are using Cargo-feature-inheritance properly

3 participants