Skip to content

Internally storing profiles as special-cased booleans #4140

@kornelski

Description

@kornelski

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:

  1. Cargo doesn't use a reference to the Profile object to track which profile was used,
  2. and profiles.bench is not returned by lib_profile() used in the end condition of drain_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 and TargetKind) — 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 and check fields of Profile and turn it into a "dumb" anonymous collection of settings for rustc. Probably rename it to ProfileSettings.

  • Make ProfileSelection know how to create appropriately-configured instance of ProfileSettings for each part of the build (lib_profile()/lib_or_check_profile()/build_script_profile()). It'll probably replace or encapsulate the Profiles (plural) type as well.

  • Replace all uses BuildConfig.release/.test, fn generate_targets(release: bool) with an instance of ProfileSelection or ProfileSettings 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?

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-profilesArea: profilesC-cleanupCategory: cleanup within the codebase

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions