Skip to content

Commit

Permalink
feat: allow opting out of required package manager (#8738)
Browse files Browse the repository at this point in the history
### Description

Give users the ability to opt out of the required package manager if
they feel strongly.

This is still highly discouraged as this prevents proper usage of daemon
package watching and leaves us dependent on system configuration to
infer the correct package manager.

Reviewer notes: Each commit can be reviewed on it's own. The second
commit just adds back code deleted in
#8017

### Testing Instructions

Added unit tests where applicable for configuration.

Quick manual verification of the config options:

```
[0 olszewski@chriss-mbp] /tmp/no-pm $ turbo_dev build                                                                
 WARNING  No locally installed `turbo` found. Using version: 2.0.7-canary.0.
  × missing packageManager field in package.json

[1 olszewski@chriss-mbp] /tmp/no-pm $ turbo_dev build --dangerously-disable-package-manager-check --output-logs=none
 WARNING  No locally installed `turbo` found. Using version: 2.0.7-canary.0.
• Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web
• Running build in 5 packages
• Remote caching disabled

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    81ms >>> FULL TURBO

[0 olszewski@chriss-mbp] /tmp/no-pm $ TURBO_DANGEROUSLY_DISABLE_PACKAGE_MANAGER_CHECK=true turbo_dev build --output-logs=none                                           
 WARNING  No locally installed `turbo` found. Using version: 2.0.7-canary.0.
• Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web
• Running build in 5 packages
• Remote caching disabled

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    210ms >>> FULL TURBO

[0 olszewski@chriss-mbp] /tmp/no-pm $ vim turbo.json                                                                         
[0 olszewski@chriss-mbp] /tmp/no-pm $ tail -3 turbo.json 
  },
  "dangerouslyDisablePackageManagerCheck": true
}
[0 olszewski@chriss-mbp] /tmp/no-pm $ turbo_dev build --output-logs=none                                                    
 WARNING  No locally installed `turbo` found. Using version: 2.0.7-canary.0.
• Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web
• Running build in 5 packages
• Remote caching disabled

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    53ms >>> FULL TURBO
```
  • Loading branch information
chris-olszewski authored Jul 12, 2024
1 parent be67cfd commit 6f76520
Show file tree
Hide file tree
Showing 16 changed files with 660 additions and 137 deletions.
24 changes: 24 additions & 0 deletions crates/turborepo-lib/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ pub struct Args {
pub check_for_update: bool,
#[clap(long = "__test-run", global = true, hide = true)]
pub test_run: bool,
/// Allow for missing `packageManager` in `package.json`.
///
/// `turbo` will use hints from codebase to guess which package manager
/// should be used.
#[clap(long, global = true)]
pub dangerously_disable_package_manager_check: bool,
#[clap(flatten, next_help_heading = "Run Arguments")]
pub run_args: Option<RunArgs>,
// This should be inside `RunArgs` but clap currently has a bug
Expand Down Expand Up @@ -2531,4 +2537,22 @@ mod test {
assert!(!LogOrder::Stream.compatible_with_tui());
assert!(!LogOrder::Grouped.compatible_with_tui());
}

#[test]
fn test_dangerously_allow_no_package_manager() {
assert!(
!Args::try_parse_from(["turbo", "build",])
.unwrap()
.dangerously_disable_package_manager_check
);
assert!(
Args::try_parse_from([
"turbo",
"build",
"--dangerously-disable-package-manager-check"
])
.unwrap()
.dangerously_disable_package_manager_check
);
}
}
5 changes: 5 additions & 0 deletions crates/turborepo-lib/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ impl CommandBase {
}
})
}))
.with_allow_no_package_manager(
self.args
.dangerously_disable_package_manager_check
.then_some(true),
)
.build()
}

Expand Down
12 changes: 11 additions & 1 deletion crates/turborepo-lib/src/commands/prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ pub enum Error {
MissingLockfile,
#[error("Prune is not supported for Bun")]
BunUnsupported,
#[error("Unable to read config: {0}")]
Config(#[from] crate::config::Error),
}

// Files that should be copied from root and if they're required for install
Expand Down Expand Up @@ -96,7 +98,7 @@ pub async fn prune(
telemetry.track_arg_usage("docker", docker);
telemetry.track_arg_usage("out-dir", output_dir != DEFAULT_OUTPUT_DIR);

let prune = Prune::new(base, scope, docker, output_dir).await?;
let prune = Prune::new(base, scope, docker, output_dir, telemetry).await?;

if matches!(
prune.package_graph.package_manager(),
Expand Down Expand Up @@ -259,7 +261,14 @@ impl<'a> Prune<'a> {
scope: &'a [String],
docker: bool,
output_dir: &str,
telemetry: CommandEventBuilder,
) -> Result<Self, Error> {
let allow_missing_package_manager = base.config()?.allow_no_package_manager();
telemetry.track_arg_usage(
"dangerously-allow-missing-package-manager",
allow_missing_package_manager,
);

if scope.is_empty() {
return Err(Error::NoWorkspaceSpecified);
}
Expand All @@ -268,6 +277,7 @@ impl<'a> Prune<'a> {
let root_package_json = PackageJson::load(&root_package_json_path)?;

let package_graph = PackageGraph::builder(&base.repo_root, root_package_json)
.with_allow_no_package_manager(allow_missing_package_manager)
.build()
.await?;

Expand Down
51 changes: 46 additions & 5 deletions crates/turborepo-lib/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ pub struct ConfigurationOptions {
pub(crate) spaces_id: Option<String>,
#[serde(rename = "ui")]
pub(crate) ui: Option<bool>,
#[serde(rename = "dangerouslyDisablePackageManagerCheck")]
pub(crate) allow_no_package_manager: Option<bool>,
}

#[derive(Default)]
Expand Down Expand Up @@ -274,13 +276,25 @@ impl ConfigurationOptions {
pub fn ui(&self) -> bool {
self.ui.unwrap_or(false) && atty::is(atty::Stream::Stdout)
}

pub fn allow_no_package_manager(&self) -> bool {
self.allow_no_package_manager.unwrap_or_default()
}
}

// Maps Some("") to None to emulate how Go handles empty strings
fn non_empty_str(s: Option<&str>) -> Option<&str> {
s.filter(|s| !s.is_empty())
}

fn truth_env_var(s: &str) -> Option<bool> {
match s {
"true" | "1" => Some(true),
"false" | "0" => Some(false),
_ => None,
}
}

trait ResolvedConfigurationOptions {
fn get_configuration_options(self) -> Result<ConfigurationOptions, Error>;
}
Expand All @@ -299,6 +313,7 @@ impl ResolvedConfigurationOptions for RawTurboJson {
.and_then(|spaces| spaces.id)
.map(|spaces_id| spaces_id.into());
opts.ui = self.ui.map(|ui| ui.use_tui());
opts.allow_no_package_manager = self.allow_no_package_manager;
Ok(opts)
}
}
Expand Down Expand Up @@ -331,6 +346,10 @@ fn get_env_var_config(
"upload_timeout",
);
turbo_mapping.insert(OsString::from("turbo_ui"), "ui");
turbo_mapping.insert(
OsString::from("turbo_dangerously_disable_package_manager_check"),
"allow_no_package_manager",
);
turbo_mapping.insert(OsString::from("turbo_preflight"), "preflight");

// We do not enable new config sources:
Expand Down Expand Up @@ -412,11 +431,15 @@ fn get_env_var_config(
};

// Process experimentalUI
let ui = output_map.get("ui").and_then(|val| match val.as_str() {
"true" | "1" => Some(true),
"false" | "0" => Some(false),
_ => None,
});
let ui = output_map
.get("ui")
.map(|s| s.as_str())
.and_then(truth_env_var);

let allow_no_package_manager = output_map
.get("allow_no_package_manager")
.map(|s| s.as_str())
.and_then(truth_env_var);

// We currently don't pick up a Spaces ID via env var, we likely won't
// continue using the Spaces name, we can add an env var when we have the
Expand All @@ -435,6 +458,7 @@ fn get_env_var_config(
preflight,
enabled,
ui,
allow_no_package_manager,

// Processed numbers
timeout,
Expand Down Expand Up @@ -498,6 +522,7 @@ fn get_override_env_var_config(
timeout: None,
upload_timeout: None,
spaces_id: None,
allow_no_package_manager: None,
};

Ok(output)
Expand Down Expand Up @@ -633,6 +658,11 @@ impl TurborepoConfigBuilder {
create_builder!(with_preflight, preflight, Option<bool>);
create_builder!(with_timeout, timeout, Option<u64>);
create_builder!(with_ui, ui, Option<bool>);
create_builder!(
with_allow_no_package_manager,
allow_no_package_manager,
Option<bool>
);

pub fn build(&self) -> Result<ConfigurationOptions, Error> {
// Priority, from least significant to most significant:
Expand Down Expand Up @@ -710,6 +740,11 @@ impl TurborepoConfigBuilder {
if let Some(ui) = current_source_config.ui {
acc.ui = Some(ui);
}
if let Some(allow_no_package_manager) =
current_source_config.allow_no_package_manager
{
acc.allow_no_package_manager = Some(allow_no_package_manager);
}

acc
})
Expand Down Expand Up @@ -743,6 +778,7 @@ mod test {
assert!(!defaults.preflight());
assert_eq!(defaults.timeout(), DEFAULT_TIMEOUT);
assert_eq!(defaults.spaces_id(), None);
assert!(!defaults.allow_no_package_manager());
}

#[test]
Expand All @@ -766,6 +802,10 @@ mod test {
turbo_remote_cache_timeout.to_string().into(),
);
env.insert("turbo_ui".into(), "true".into());
env.insert(
"turbo_dangerously_disable_package_manager_check".into(),
"true".into(),
);
env.insert("turbo_preflight".into(), "true".into());

let config = get_env_var_config(&env).unwrap();
Expand All @@ -777,6 +817,7 @@ mod test {
assert_eq!(turbo_token, config.token.unwrap());
assert_eq!(turbo_remote_cache_timeout, config.timeout.unwrap());
assert_eq!(Some(true), config.ui);
assert_eq!(Some(true), config.allow_no_package_manager);
}

#[test]
Expand Down
24 changes: 17 additions & 7 deletions crates/turborepo-lib/src/run/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub struct RunBuilder {
// this package.
entrypoint_packages: Option<HashSet<PackageName>>,
should_print_prelude_override: Option<bool>,
allow_missing_package_manager: bool,
}

impl RunBuilder {
Expand All @@ -73,6 +74,8 @@ impl RunBuilder {

let mut opts: Opts = base.args().try_into()?;
let config = base.config()?;
let allow_missing_package_manager = config.allow_no_package_manager();

let is_linked = turborepo_api_client::is_linked(&api_auth);
if !is_linked {
opts.cache_opts.skip_remote = true;
Expand Down Expand Up @@ -116,6 +119,7 @@ impl RunBuilder {
analytics_sender: None,
entrypoint_packages: None,
should_print_prelude_override: None,
allow_missing_package_manager,
})
}

Expand Down Expand Up @@ -187,6 +191,10 @@ impl RunBuilder {
// Pulled from initAnalyticsClient in run.go
let is_linked = turborepo_api_client::is_linked(&self.api_auth);
run_telemetry.track_is_linked(is_linked);
run_telemetry.track_arg_usage(
"dangerously_allow_missing_package_manager",
self.allow_missing_package_manager,
);
// we only track the remote cache if we're linked because this defaults to
// Vercel
if is_linked {
Expand All @@ -206,8 +214,7 @@ impl RunBuilder {
run_telemetry.track_ci(turborepo_ci::Vendor::get_name());

// Remove allow when daemon is flagged back on
#[allow(unused_mut)]
let mut daemon = match (is_ci_or_not_tty, self.opts.run_opts.daemon) {
let daemon = match (is_ci_or_not_tty, self.opts.run_opts.daemon) {
(true, None) => {
run_telemetry.track_daemon_init(DaemonInitStatus::Skipped);
debug!("skipping turbod since we appear to be in a non-interactive context");
Expand Down Expand Up @@ -246,10 +253,13 @@ impl RunBuilder {

let mut pkg_dep_graph = {
let builder = PackageGraph::builder(&self.repo_root, root_package_json.clone())
.with_single_package_mode(self.opts.run_opts.single_package);
.with_single_package_mode(self.opts.run_opts.single_package)
.with_allow_no_package_manager(self.allow_missing_package_manager);

#[cfg(feature = "daemon-package-discovery")]
let graph = {
// Daemon package discovery depends on packageManager existing in package.json
let graph = if cfg!(feature = "daemon-package-discovery")
&& !self.allow_missing_package_manager
{
match (&daemon, self.opts.run_opts.daemon) {
(None, Some(true)) => {
// We've asked for the daemon, but it's not available. This is an error
Expand Down Expand Up @@ -291,9 +301,9 @@ impl RunBuilder {
.await
}
}
} else {
builder.build().await
};
#[cfg(not(feature = "daemon-package-discovery"))]
let graph = builder.build().await;

match graph {
Ok(graph) => graph,
Expand Down
15 changes: 15 additions & 0 deletions crates/turborepo-lib/src/turbo_json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ pub struct RawTurboJson {
pub(crate) remote_cache: Option<RawRemoteCacheOptions>,
#[serde(skip_serializing_if = "Option::is_none", rename = "ui")]
pub ui: Option<UI>,
#[serde(
skip_serializing_if = "Option::is_none",
rename = "dangerouslyDisablePackageManagerCheck"
)]
pub allow_no_package_manager: Option<bool>,

#[deserializable(rename = "//")]
#[serde(skip)]
Expand Down Expand Up @@ -1085,4 +1090,14 @@ mod tests {
let actual = serde_json::to_string(&parsed).unwrap();
assert_eq!(actual, expected);
}

#[test_case(r#"{"dangerouslyDisablePackageManagerCheck":true}"#, Some(true) ; "t")]
#[test_case(r#"{"dangerouslyDisablePackageManagerCheck":false}"#, Some(false) ; "f")]
#[test_case(r#"{}"#, None ; "missing")]
fn test_allow_no_package_manager_serde(json_str: &str, expected: Option<bool>) {
let json = RawTurboJson::parse(json_str, AnchoredSystemPath::new("").unwrap()).unwrap();
assert_eq!(json.allow_no_package_manager, expected);
let serialized = serde_json::to_string(&json).unwrap();
assert_eq!(serialized, json_str);
}
}
12 changes: 11 additions & 1 deletion crates/turborepo-repository/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ pub struct LocalPackageDiscoveryBuilder {
repo_root: AbsoluteSystemPathBuf,
package_manager: Option<PackageManager>,
package_json: Option<PackageJson>,
allow_missing_package_manager: bool,
}

impl LocalPackageDiscoveryBuilder {
Expand All @@ -96,8 +97,13 @@ impl LocalPackageDiscoveryBuilder {
repo_root,
package_manager,
package_json,
allow_missing_package_manager: false,
}
}

pub fn with_allow_no_package_manager(&mut self, allow_missing_package_manager: bool) {
self.allow_missing_package_manager = allow_missing_package_manager;
}
}

impl PackageDiscoveryBuilder for LocalPackageDiscoveryBuilder {
Expand All @@ -111,7 +117,11 @@ impl PackageDiscoveryBuilder for LocalPackageDiscoveryBuilder {
let package_json = self.package_json.map(Ok).unwrap_or_else(|| {
PackageJson::load(&self.repo_root.join_component("package.json"))
})?;
PackageManager::get_package_manager(&package_json)?
if self.allow_missing_package_manager {
PackageManager::read_or_detect_package_manager(&package_json, &self.repo_root)?
} else {
PackageManager::get_package_manager(&package_json)?
}
}
};

Expand Down
Loading

0 comments on commit 6f76520

Please sign in to comment.