-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Description
I wanted to fix a pet peeve of mine (cargo bench reports it finished building a "release" profile when it actually used a "bench" profile), and found that it's not straightforward to do:
- Cargo doesn't use a reference to the
Profileobject to track which profile was used, - and
profiles.benchis not returned bylib_profile()used in the end condition ofdrain_the_queue()(that seems like a bug stemming from lack of (1).)
The profile setting appears to be spread over booleans in Context, BuildConfig and global-ish Profile objects. In some places Cargo uses a set of flags to "guess" what profile was used (Context::lib_profile()) instead of keeping a reference to an actual Profile object used. In other places all profile information is reduced to a single boolean flag if self.is_release { "release" } else { "dev" }.
I've got a hunch that these multiple boolean flags spread throughout the code are a source of needless complexity and quirks (like the "test" and "bench" profiles vaguely overlapping with identity of "dev" and "release" profiles).
I would like to refactor it, but I'd like your guidance whether it makes sense and how it should be implemented.
Existence of lib_profile()/lib_or_check_profile()/build_script_profile() makes me think that there are actually two distinct concepts of a "profile" in Cargo:
-
One that describes a high-level user choice (a combination of
--releaseflag andTargetKind) — the "what are we building" part -
and the other kind that just describes low-level compiler settings (for compiling lib, deps, build.rs) — the "how" part.
Currently the first kind is represented by booleans in Context, BuildConfig (release,is_release,check) and Profile (test/check/doc), and the second kind is the remainder of the Profile struct (opt_level/rustc_args, etc.).
I'd like to replace most code in form let foo = if profile.foo { this } else { that } with let foo = profile.foo().
I'm thinking about refactoring it this way:
-
Make a
ProfileSelectionobject (it's not a great name, bikeshedding welcome) that represents the high-level profile chosen by the user (one that corresponds to profile names from Cargo.toml: "dev","release","bench", etc.). It would be a single object for the whole build that encapsulates all the booleans based on the--releaseflag, subcommand, target, etc. -
Remove the
test,docandcheckfields ofProfileand turn it into a "dumb" anonymous collection of settings for rustc. Probably rename it toProfileSettings. -
Make
ProfileSelectionknow how to create appropriately-configured instance ofProfileSettingsfor each part of the build (lib_profile()/lib_or_check_profile()/build_script_profile()). It'll probably replace or encapsulate theProfiles(plural) type as well. -
Replace all uses
BuildConfig.release/.test,fn generate_targets(release: bool)with an instance ofProfileSelectionorProfileSettingsas appropriate.
Does that make sense? Will that work?
I haven't looked closely at Workspaces yet. Are there any complications there?
Anything else that I'm missing?