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

WIP: Initial work to add modifiers to Cargo.toml and env variable options #117

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

deg4uss3r
Copy link

@deg4uss3r deg4uss3r commented Jan 19, 2025

This PR adds the ability to use modifiers in Cargo.toml and Environment variable overrides.

resolves: #118

Copy link
Owner

@gdesmott gdesmott left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

I have no objection adding such feature but would like a bit more context as I never used those kinds of modifiers.

Adding @xclaesse and @nirbheek in case they know more about modifiers and have some input.

Once we agree on the API we'll need it properly documented in the general documentation at the top of lib.rs.


[package.metadata.system-deps]
testdata = "4"
teststaticlib = { version = "1", feature = "test-feature", modifiers = "+whole-archive"}
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not familiar with +whole-archive and modifiers in general. If they are used only during static linking maybe they should be called static_modifiers instead?

What other modifiers could be used there for example?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you so much for taking a look so quickly and for pulling in some more people, I wouldn't consider myself a deep expert either!

In rustc documentation the three specified are [+|-] whole-archive, verbatim, and bundle. This is where I wouldn't consider myself a deep expert and I'm not sure if more are supported and these 3 are just the documented ones because they have defaults.

The terminology I used here matches rustc and build script documentation. However, I am happy to rename if that's the consensus.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it makes sense to use the same terminology as rustc then. We should also link this page in our documentation of this feature.

Copy link

codecov bot commented Jan 20, 2025

Codecov Report

Attention: Patch coverage is 96.31902% with 6 lines in your changes missing coverage. Please review.

Project coverage is 91.30%. Comparing base (38acdbc) to head (dfff668).

Files with missing lines Patch % Lines
src/lib.rs 90.56% 5 Missing ⚠️
src/test.rs 98.95% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
+ Coverage   90.91%   91.30%   +0.38%     
==========================================
  Files           3        3              
  Lines        2257     2403     +146     
==========================================
+ Hits         2052     2194     +142     
- Misses        205      209       +4     

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

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.

Allow Modifiers For Statically Linked Libraries
2 participants