Skip to content

Commit

Permalink
Auto merge of #13486 - weihanglo:config-cleanup, r=epage
Browse files Browse the repository at this point in the history
refactor: clean up for `GlobalContext` rename
  • Loading branch information
bors committed Feb 23, 2024
2 parents 2b302a5 + 75dbb86 commit e975158
Show file tree
Hide file tree
Showing 16 changed files with 121 additions and 120 deletions.
4 changes: 2 additions & 2 deletions crates/mdman/src/hbs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl HelperDef for ManLinkHelper<'_> {
&self,
h: &Helper<'rc>,
_r: &'reg Handlebars<'reg>,
_gctx: &'rc Context,
_ctx: &'rc Context,
_rc: &mut RenderContext<'reg, 'rc>,
out: &mut dyn Output,
) -> HelperResult {
Expand Down Expand Up @@ -200,7 +200,7 @@ impl HelperDef for ManLinkHelper<'_> {
fn set_decorator(
d: &Decorator<'_>,
_: &Handlebars<'_>,
_gctx: &Context,
_ctx: &Context,
rc: &mut RenderContext<'_, '_>,
) -> Result<(), RenderError> {
let data_to_set = d.hash();
Expand Down
3 changes: 2 additions & 1 deletion src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult {

// Update the process-level notion of cwd
if let Some(new_cwd) = args.get_one::<std::path::PathBuf>("directory") {
// This is a temporary hack. This cannot access `Config`, so this is a bit messy.
// This is a temporary hack.
// This cannot access `GlobalContext`, so this is a bit messy.
// This does not properly parse `-Z` flags that appear after the subcommand.
// The error message is not as helpful as the standard one.
let nightly_features_allowed = matches!(&*features::channel(), "nightly" | "dev");
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
// In general, we try to avoid normalizing paths in Cargo,
// but in these particular cases we need it to fix rust-lang/cargo#10283.
// (Handle `SourceId::for_path` and `Workspace::new`,
// but not `Config::reload_rooted_at` which is always cwd)
// but not `GlobalContext::reload_rooted_at` which is always cwd)
let path = path.map(|p| paths::normalize_path(&p));

let version = args.get_one::<VersionReq>("version");
Expand Down
9 changes: 6 additions & 3 deletions src/cargo/core/compiler/job_queue/job_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@ pub struct JobState<'a, 'gctx> {
/// output messages are processed on the same thread as they are sent from. `output`
/// defines where to output in this case.
///
/// Currently the `Shell` inside `Config` is wrapped in a `RefCell` and thus can't be passed
/// between threads. This means that it isn't possible for multiple output messages to be
/// interleaved. In the future, it may be wrapped in a `Mutex` instead. In this case
/// Currently the [`Shell`] inside [`GlobalContext`] is wrapped in a `RefCell` and thus can't
/// be passed between threads. This means that it isn't possible for multiple output messages
/// to be interleaved. In the future, it may be wrapped in a `Mutex` instead. In this case
/// interleaving is still prevented as the lock would be held for the whole printing of an
/// output message.
///
/// [`Shell`]: crate::core::Shell
/// [`GlobalContext`]: crate::GlobalContext
output: Option<&'a DiagDedupe<'gctx>>,

/// The job id that this state is associated with, used when sending
Expand Down
18 changes: 7 additions & 11 deletions src/cargo/core/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ fn auto_gc_inner(gctx: &GlobalContext) -> CargoResult<()> {
debug_assert!(deferred.is_empty());
let mut global_cache_tracker = gctx.global_cache_tracker()?;
let mut gc = Gc::new(gctx, &mut global_cache_tracker)?;
let mut clean_gctx = CleanContext::new(gctx);
gc.auto(&mut clean_gctx)?;
let mut clean_ctx = CleanContext::new(gctx);
gc.auto(&mut clean_ctx)?;
Ok(())
}

Expand Down Expand Up @@ -256,7 +256,7 @@ impl<'a, 'gctx> Gc<'a, 'gctx> {
///
/// This returns immediately without doing work if garbage collection has
/// been performed recently (since `gc.auto.frequency`).
fn auto(&mut self, clean_gctx: &mut CleanContext<'gctx>) -> CargoResult<()> {
fn auto(&mut self, clean_ctx: &mut CleanContext<'gctx>) -> CargoResult<()> {
if !self.gctx.cli_unstable().gc {
return Ok(());
}
Expand All @@ -279,20 +279,16 @@ impl<'a, 'gctx> Gc<'a, 'gctx> {
}
let mut gc_opts = GcOpts::default();
gc_opts.update_for_auto_gc_config(&auto_config)?;
self.gc(clean_gctx, &gc_opts)?;
if !clean_gctx.dry_run {
self.gc(clean_ctx, &gc_opts)?;
if !clean_ctx.dry_run {
self.global_cache_tracker.set_last_auto_gc()?;
}
Ok(())
}

/// Performs garbage collection based on the given options.
pub fn gc(
&mut self,
clean_gctx: &mut CleanContext<'gctx>,
gc_opts: &GcOpts,
) -> CargoResult<()> {
self.global_cache_tracker.clean(clean_gctx, gc_opts)?;
pub fn gc(&mut self, clean_ctx: &mut CleanContext<'gctx>, gc_opts: &GcOpts) -> CargoResult<()> {
self.global_cache_tracker.clean(clean_ctx, gc_opts)?;
// In the future, other gc operations go here, such as target cleaning.
Ok(())
}
Expand Down
16 changes: 6 additions & 10 deletions src/cargo/core/global_cache_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,22 +547,18 @@ impl GlobalCacheTracker {
}

/// Deletes files from the global cache based on the given options.
pub fn clean(
&mut self,
clean_gctx: &mut CleanContext<'_>,
gc_opts: &GcOpts,
) -> CargoResult<()> {
self.clean_inner(clean_gctx, gc_opts)
pub fn clean(&mut self, clean_ctx: &mut CleanContext<'_>, gc_opts: &GcOpts) -> CargoResult<()> {
self.clean_inner(clean_ctx, gc_opts)
.with_context(|| "failed to clean entries from the global cache")
}

fn clean_inner(
&mut self,
clean_gctx: &mut CleanContext<'_>,
clean_ctx: &mut CleanContext<'_>,
gc_opts: &GcOpts,
) -> CargoResult<()> {
let _p = crate::util::profile::start("cleaning global cache files");
let gctx = clean_gctx.gctx;
let gctx = clean_ctx.gctx;
let base = BasePaths {
index: gctx.registry_index_path().into_path_unlocked(),
git_db: gctx.git_db_path().into_path_unlocked(),
Expand Down Expand Up @@ -657,9 +653,9 @@ impl GlobalCacheTracker {
Self::get_registry_items_to_clean_size_both(&tx, max_size, &base, &mut delete_paths)?;
}

clean_gctx.remove_paths(&delete_paths)?;
clean_ctx.remove_paths(&delete_paths)?;

if clean_gctx.dry_run {
if clean_ctx.dry_run {
tx.rollback()?;
} else {
tx.commit()?;
Expand Down
14 changes: 7 additions & 7 deletions src/cargo/core/resolver/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl From<(PackageId, ConflictReason)> for ActivateError {
}

pub(super) fn activation_error(
resolver_gctx: &ResolverContext,
resolver_ctx: &ResolverContext,
registry: &mut dyn Registry,
parent: &Summary,
dep: &Dependency,
Expand All @@ -81,7 +81,7 @@ pub(super) fn activation_error(
let to_resolve_err = |err| {
ResolveError::new(
err,
resolver_gctx
resolver_ctx
.parents
.path_to_bottom(&parent.package_id())
.into_iter()
Expand All @@ -95,7 +95,7 @@ pub(super) fn activation_error(
let mut msg = format!("failed to select a version for `{}`.", dep.package_name());
msg.push_str("\n ... required by ");
msg.push_str(&describe_path_in_context(
resolver_gctx,
resolver_ctx,
&parent.package_id(),
));

Expand Down Expand Up @@ -141,7 +141,7 @@ pub(super) fn activation_error(
msg.push_str("`, but it conflicts with a previous package which links to `");
msg.push_str(link);
msg.push_str("` as well:\n");
msg.push_str(&describe_path_in_context(resolver_gctx, p));
msg.push_str(&describe_path_in_context(resolver_ctx, p));
msg.push_str("\nOnly one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. ");
msg.push_str("Try to adjust your dependencies so that only one package uses the `links = \"");
msg.push_str(link);
Expand Down Expand Up @@ -210,7 +210,7 @@ pub(super) fn activation_error(
for (p, r) in &conflicting_activations {
if let ConflictReason::Semver = r {
msg.push_str("\n\n previously selected ");
msg.push_str(&describe_path_in_context(resolver_gctx, p));
msg.push_str(&describe_path_in_context(resolver_ctx, p));
}
}
}
Expand Down Expand Up @@ -279,7 +279,7 @@ pub(super) fn activation_error(
);
msg.push_str("required by ");
msg.push_str(&describe_path_in_context(
resolver_gctx,
resolver_ctx,
&parent.package_id(),
));

Expand Down Expand Up @@ -364,7 +364,7 @@ pub(super) fn activation_error(
msg.push_str(&format!("location searched: {}\n", dep.source_id()));
msg.push_str("required by ");
msg.push_str(&describe_path_in_context(
resolver_gctx,
resolver_ctx,
&parent.package_id(),
));

Expand Down
12 changes: 6 additions & 6 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ pub struct CleanContext<'gctx> {
pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
let mut target_dir = ws.target_dir();
let gctx = opts.gctx;
let mut clean_gctx = CleanContext::new(gctx);
clean_gctx.dry_run = opts.dry_run;
let mut clean_ctx = CleanContext::new(gctx);
clean_ctx.dry_run = opts.dry_run;

if opts.doc {
if !opts.spec.is_empty() {
Expand All @@ -55,7 +55,7 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
}
// If the doc option is set, we just want to delete the doc directory.
target_dir = target_dir.join("doc");
clean_gctx.remove_paths(&[target_dir.into_path_unlocked()])?;
clean_ctx.remove_paths(&[target_dir.into_path_unlocked()])?;
} else {
let profiles = Profiles::new(&ws, opts.requested_profile)?;

Expand All @@ -73,13 +73,13 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
// Note that we don't bother grabbing a lock here as we're just going to
// blow it all away anyway.
if opts.spec.is_empty() {
clean_gctx.remove_paths(&[target_dir.into_path_unlocked()])?;
clean_ctx.remove_paths(&[target_dir.into_path_unlocked()])?;
} else {
clean_specs(&mut clean_gctx, &ws, &profiles, &opts.targets, &opts.spec)?;
clean_specs(&mut clean_ctx, &ws, &profiles, &opts.targets, &opts.spec)?;
}
}

clean_gctx.display_summary()?;
clean_ctx.display_summary()?;
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<'gctx> SourceConfigMap<'gctx> {
Ok(base)
}

/// Returns the `Config` this source config map is associated with.
/// Returns the [`GlobalContext`] this source config map is associated with.
pub fn gctx(&self) -> &'gctx GlobalContext {
self.gctx
}
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/util/cache_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//!
//! There is a global [`CacheLocker`] held inside cargo's venerable
//! [`GlobalContext`]. The `CacheLocker` manages creating and tracking the locks
//! being held. There are methods on `Config` for managing the locks:
//! being held. There are methods on [`GlobalContext`] for managing the locks:
//!
//! - [`GlobalContext::acquire_package_cache_lock`] --- Acquires a lock. May block if
//! another process holds a lock.
Expand Down Expand Up @@ -468,7 +468,7 @@ pub struct CacheLocker {
/// The state of the locker.
///
/// [`CacheLocker`] uses interior mutability because it is stuffed inside
/// the global `Config`, which does not allow mutation.
/// [`GlobalContext`], which does not allow mutation.
state: RefCell<CacheState>,
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/config/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::collections::HashSet;
use std::vec;

/// Serde deserializer used to convert config values to a target type using
/// `Config::get`.
/// [`GlobalContext::get`].
#[derive(Clone)]
pub(super) struct Deserializer<'gctx> {
pub(super) gctx: &'gctx GlobalContext,
Expand Down
25 changes: 14 additions & 11 deletions src/cargo/util/config/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,20 @@ fn make_case_insensitive_and_normalized_env(
(case_insensitive_env, normalized_env)
}

/// A snapshot of the environment variables available to [`super::GlobalContext`].
/// A snapshot of the environment variables available to [`GlobalContext`].
///
/// Currently, the [`Context`](super::GlobalContext) supports lookup of environment variables
/// Currently, the [`GlobalContext`] supports lookup of environment variables
/// through two different means:
///
/// - [`Context::get_env`](super::GlobalContext::get_env)
/// and [`Context::get_env_os`](super::GlobalContext::get_env_os)
/// - [`GlobalContext::get_env`](super::GlobalContext::get_env)
/// and [`GlobalContext::get_env_os`](super::GlobalContext::get_env_os)
/// for process environment variables (similar to [`std::env::var`] and [`std::env::var_os`]),
/// - Typed Config Value API via [`Context::get`](super::GlobalContext::get).
/// - Typed Config Value API via [`GlobalContext::get`](super::GlobalContext::get).
/// This is only available for `CARGO_` prefixed environment keys.
///
/// This type contains the env var snapshot and helper methods for both APIs.
///
/// [`GlobalContext`]: super::GlobalContext
#[derive(Debug)]
pub struct Env {
/// A snapshot of the process's environment variables.
Expand Down Expand Up @@ -68,8 +70,8 @@ impl Env {
pub fn new() -> Self {
// ALLOWED: This is the only permissible usage of `std::env::vars{_os}`
// within cargo. If you do need access to individual variables without
// interacting with `Config` system, please use `std::env::var{_os}`
// and justify the validity of the usage.
// interacting with the config system in [`GlobalContext`], please use
// `std::env::var{_os}` and justify the validity of the usage.
#[allow(clippy::disallowed_methods)]
let env: HashMap<_, _> = std::env::vars_os().collect();
let (case_insensitive_env, normalized_env) = make_case_insensitive_and_normalized_env(&env);
Expand Down Expand Up @@ -105,9 +107,10 @@ impl Env {
self.env.keys().filter_map(|k| k.to_str())
}

/// Get the value of environment variable `key` through the `Config` snapshot.
/// Get the value of environment variable `key` through the snapshot in
/// [`GlobalContext`](super::GlobalContext).
///
/// This can be used similarly to `std::env::var_os`.
/// This can be used similarly to [`std::env::var_os`].
/// On Windows, we check for case mismatch since environment keys are case-insensitive.
pub fn get_env_os(&self, key: impl AsRef<OsStr>) -> Option<OsString> {
match self.env.get(key.as_ref()) {
Expand Down Expand Up @@ -152,7 +155,7 @@ impl Env {
/// Get the value of environment variable `key` as a `&str`.
/// Returns `None` if `key` is not in `self.env` or if the value is not valid UTF-8.
///
/// This is intended for use in private methods of `Config`,
/// This is intended for use in private methods of [`GlobalContext`](super::GlobalContext),
/// and does not check for env key case mismatch.
///
/// This is case-sensitive on Windows (even though environment keys on Windows are usually
Expand All @@ -172,7 +175,7 @@ impl Env {

/// Check if the environment contains `key`.
///
/// This is intended for use in private methods of `Config`,
/// This is intended for use in private methods of [`GlobalContext`](super::GlobalContext),
/// and does not check for env key case mismatch.
/// See the docstring of [`Env::get_str`] for more context.
pub(super) fn contains_key(&self, key: impl AsRef<OsStr>) -> bool {
Expand Down
Loading

0 comments on commit e975158

Please sign in to comment.