-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Profile
object to track which profile was used, - and
profiles.bench
is 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
--release
flag 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
ProfileSelection
object (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--release
flag, subcommand, target, etc. -
Remove the
test
,doc
andcheck
fields ofProfile
and turn it into a "dumb" anonymous collection of settings for rustc. Probably rename it toProfileSettings
. -
Make
ProfileSelection
know how to create appropriately-configured instance ofProfileSettings
for 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 ofProfileSelection
orProfileSettings
as 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?