Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/handle transparent workspaces #1

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 3 additions & 35 deletions crates/turborepo-repository/src/package_graph/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ use turborepo_graph_utils as graph;
use turborepo_lockfiles::Lockfile;

use super::{
dep_splitter::DependencySplitter, npmrc::NpmRc, yarnrc::YarnRc, PackageGraph, PackageInfo,
PackageName, PackageNode,
dep_splitter::DependencySplitter, PackageGraph, PackageInfo, PackageName, PackageNode,
};
use crate::{
discovery::{
Expand Down Expand Up @@ -352,27 +351,6 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedWorkspaces, T> {
&mut self,
package_manager: &PackageManager,
) -> Result<(), Error> {
let npmrc = match package_manager {
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
let npmrc_path = self.repo_root.join_component(".npmrc");
match npmrc_path.read_existing_to_string().ok().flatten() {
Some(contents) => NpmRc::from_reader(contents.as_bytes()).ok(),
None => None,
}
}
_ => None,
};
let yarnrc_path = self.repo_root.join_component(".yarnrc.yml");
let yarnrc = match package_manager {
PackageManager::Berry => {
// HOME?
match yarnrc_path.read_existing_to_string().ok().flatten() {
Some(contents) => YarnRc::from_reader(contents.as_bytes()).ok(),
None => None,
}
}
_ => None,
};
let split_deps = self
.workspaces
.iter()
Expand All @@ -385,8 +363,6 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedWorkspaces, T> {
&entry.package_json_path,
&self.workspaces,
package_manager,
npmrc.as_ref(),
yarnrc.as_ref(),
entry.package_json.all_dependencies(),
),
)
Expand Down Expand Up @@ -578,8 +554,6 @@ impl Dependencies {
workspace_json_path: &AnchoredSystemPathBuf,
workspaces: &HashMap<PackageName, PackageInfo>,
package_manager: &PackageManager,
npmrc: Option<&NpmRc>,
yarnrc: Option<&YarnRc>,
dependencies: I,
) -> Self {
let resolved_workspace_json_path = repo_root.resolve(workspace_json_path);
Expand All @@ -588,14 +562,8 @@ impl Dependencies {
.expect("package.json path should have parent");
let mut internal = HashSet::new();
let mut external = BTreeMap::new();
let splitter = DependencySplitter::new(
repo_root,
workspace_dir,
workspaces,
package_manager,
npmrc,
yarnrc,
);
let splitter =
DependencySplitter::new(repo_root, workspace_dir, workspaces, package_manager);
for (name, version) in dependencies.into_iter() {
if let Some(workspace) = splitter.is_internal(name, version) {
internal.insert(workspace);
Expand Down
10 changes: 3 additions & 7 deletions crates/turborepo-repository/src/package_graph/dep_splitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use turbopath::{
RelativeUnixPathBuf,
};

use super::{npmrc::NpmRc, yarnrc::YarnRc, PackageInfo, PackageName};
use super::{PackageInfo, PackageName};
use crate::package_manager::PackageManager;

pub struct DependencySplitter<'a> {
Expand All @@ -21,17 +21,13 @@ impl<'a> DependencySplitter<'a> {
workspace_dir: &'a AbsoluteSystemPath,
workspaces: &'a HashMap<PackageName, PackageInfo>,
package_manager: &PackageManager,
npmrc: Option<&'a NpmRc>,
yarnrc: Option<&'a YarnRc>,
) -> Self {
let link_workspace_packages = package_manager.link_workspace_packages(repo_root);
Self {
repo_root,
workspace_dir,
workspaces,
link_workspace_packages: npmrc
.and_then(|npmrc| npmrc.link_workspace_packages)
.or_else(|| yarnrc.map(|yarnrc| yarnrc.enableTransparentWorkspaces))
.unwrap_or(!matches!(package_manager, PackageManager::Pnpm9)),
link_workspace_packages,
}
}

Expand Down
2 changes: 0 additions & 2 deletions crates/turborepo-repository/src/package_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ use crate::{

pub mod builder;
mod dep_splitter;
mod npmrc;
mod yarnrc;

pub use builder::{Error, PackageGraphBuilder};

Expand Down
66 changes: 66 additions & 0 deletions crates/turborepo-repository/src/package_manager/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
mod bun;
mod npm;
mod npmrc;
mod pnpm;
mod yarn;
mod yarnrc;

use std::{
backtrace,
Expand All @@ -16,13 +18,16 @@ use lazy_regex::{lazy_regex, Lazy};
use miette::{Diagnostic, NamedSource, SourceSpan};
use node_semver::SemverError;
use npm::NpmDetector;
use npmrc::NpmRc;
use regex::Regex;
use serde::Deserialize;
use thiserror::Error;
use tracing::debug;
use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, RelativeUnixPath};
use turborepo_errors::Spanned;
use turborepo_lockfiles::Lockfile;
use which::which;
use yarnrc::YarnRc;

use crate::{
discovery,
Expand Down Expand Up @@ -190,6 +195,8 @@ pub enum Error {
WorkspaceDiscovery(#[from] discovery::Error),
#[error("missing packageManager field in package.json")]
MissingPackageManager,
#[error(transparent)]
Yarnrc(#[from] yarnrc::Error),
}

impl From<std::convert::Infallible> for Error {
Expand Down Expand Up @@ -519,13 +526,42 @@ impl PackageManager {
PackageManager::Pnpm | PackageManager::Pnpm9 | PackageManager::Berry => None,
}
}

/// Returns whether or not the package manager will select a package in the
/// workspace as a dependency if the `workspace:` protocol isn't used.
/// For example if a package in the workspace has `"lib": "1.2.3"` and
/// there's a package in the workspace with the name of `lib` and
/// version `1.2.3` if this is true, then the local `lib` package will
/// be used where `false` would use a `lib` package from the registry.
pub fn link_workspace_packages(&self, repo_root: &AbsoluteSystemPath) -> bool {
match self {
PackageManager::Berry => {
let yarnrc = YarnRc::from_file(repo_root)
.inspect_err(|e| debug!("unable to read yarnrc: {e}"))
.unwrap_or_default();
yarnrc.enable_transparent_workspaces
}
PackageManager::Pnpm9 | PackageManager::Pnpm | PackageManager::Pnpm6 => {
let npmrc = NpmRc::from_file(repo_root)
.inspect_err(|e| debug!("unable to read npmrc: {e}"))
.unwrap_or_default();
npmrc
.link_workspace_packages
// The default for pnpm 9 is false if not explicitly set
// All previous versions had a default of true
.unwrap_or(!matches!(self, PackageManager::Pnpm9))
}
PackageManager::Yarn | PackageManager::Bun | PackageManager::Npm => true,
}
}
}

#[cfg(test)]
mod tests {
use std::collections::HashSet;

use pretty_assertions::assert_eq;
use test_case::test_case;

use super::*;

Expand Down Expand Up @@ -787,4 +823,34 @@ mod tests {
assert_eq!(nested.workspaces.as_ref(), vec!["packages/**"]);
Ok(())
}

#[test_case(PackageManager::Npm, None, true)]
#[test_case(PackageManager::Yarn, None, true)]
#[test_case(PackageManager::Bun, None, true)]
#[test_case(PackageManager::Pnpm6, None, true)]
#[test_case(PackageManager::Pnpm, None, true)]
#[test_case(PackageManager::Pnpm, Some(false), false)]
#[test_case(PackageManager::Pnpm, Some(true), true)]
#[test_case(PackageManager::Pnpm9, None, false)]
#[test_case(PackageManager::Pnpm9, Some(true), true)]
#[test_case(PackageManager::Pnpm9, Some(false), false)]
#[test_case(PackageManager::Berry, None, true)]
#[test_case(PackageManager::Berry, Some(false), false)]
#[test_case(PackageManager::Berry, Some(true), true)]
fn test_link_workspace_packages(pm: PackageManager, enabled: Option<bool>, expected: bool) {
let tmpdir = tempfile::tempdir().unwrap();
let repo_root = AbsoluteSystemPath::from_std_path(tmpdir.path()).unwrap();
if let Some(enabled) = enabled {
repo_root
.join_component(npmrc::NPMRC_FILENAME)
.create_with_contents(format!("link-workspace-packages={enabled}"))
.unwrap();
repo_root
.join_component(yarnrc::YARNRC_FILENAME)
.create_with_contents(format!("enableTransparentWorkspaces: {enabled}"))
.unwrap();
}
let actual = pm.link_workspace_packages(repo_root);
assert_eq!(actual, expected);
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
use std::io;

use ini::Ini;
use turbopath::AbsoluteSystemPath;

pub const NPMRC_FILENAME: &str = ".npmrc";

#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("encountered error reading .npmrc: {0}")]
Io(#[from] std::io::Error),
#[error("encountered error parsing .npmrc: {0}")]
Ini(#[from] ini::Error),
}

/// Representation of .npmrc used by both npm and pnpm to configure behavior
/// The representation is intentionally incomplete and is only intended to
/// contain settings that affect the package graph.
#[derive(Debug, PartialEq, Eq, Clone)]
#[derive(Debug, PartialEq, Eq, Clone, Default)]
pub struct NpmRc {
/// Used by pnpm to determine whether dependencies
/// declared without an explicit workspace protocol
Expand All @@ -20,6 +25,16 @@ pub struct NpmRc {
}

impl NpmRc {
pub fn from_file(repo_root: &AbsoluteSystemPath) -> Result<Self, Error> {
let npmrc_path = repo_root.join_component(NPMRC_FILENAME);
let npmrc = npmrc_path
.read_existing_to_string()?
.map(|contents| Self::from_reader(contents.as_bytes()))
.transpose()?
.unwrap_or_default();
Ok(npmrc)
}

pub fn from_reader(mut reader: impl io::Read) -> Result<Self, Error> {
let ini = Ini::read_from(&mut reader)?;
Ok(Self::from_ini(ini))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,38 @@ use std::io;

use serde::Deserialize;
use serde_yaml;
use turbopath::AbsoluteSystemPath;

pub const YARNRC_FILENAME: &str = ".yarnrc.yml";

#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("encountered error opening yarnrc.yml: {0}")]
Io(#[from] std::io::Error),
#[error("encountered error parsing yarnrc.yml: {0}")]
SerdeYaml(#[from] serde_yaml::Error),
}

/// A yarnrc.yaml file representing settings affecting the package graph.
#[allow(non_snake_case)]
#[derive(Debug, PartialEq, Eq, Clone, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct YarnRc {
/// Used by Yarn(Berry) as `enableTransparentWorkspaces`.
/// When true, treats local workspaces that match a package name
/// and semver range as correct match resulting in turbo including
/// the package in the dependency graph
pub enableTransparentWorkspaces: bool,
#[serde(default = "default_enable_transparent_workspaces")]
pub enable_transparent_workspaces: bool,
}

fn default_enable_transparent_workspaces() -> bool {
true
}

impl Default for YarnRc {
fn default() -> YarnRc {
YarnRc {
enableTransparentWorkspaces: true,
enable_transparent_workspaces: default_enable_transparent_workspaces(),
}
}
}
Expand All @@ -33,6 +43,16 @@ impl YarnRc {
let config: YarnRc = serde_yaml::from_reader(&mut reader)?;
Ok(config)
}

pub fn from_file(repo_root: &AbsoluteSystemPath) -> Result<Self, Error> {
let yarnrc_path = repo_root.join_component(YARNRC_FILENAME);
let yarnrc = yarnrc_path
.read_existing_to_string()?
.map(|contents| Self::from_reader(contents.as_bytes()))
.transpose()?
.unwrap_or_default();
Ok(yarnrc)
}
}

#[cfg(test)]
Expand All @@ -45,7 +65,7 @@ mod test {
assert_eq!(
empty,
YarnRc {
enableTransparentWorkspaces: true
enable_transparent_workspaces: true
}
);
}
Expand All @@ -56,7 +76,18 @@ mod test {
assert_eq!(
empty,
YarnRc {
enableTransparentWorkspaces: false
enable_transparent_workspaces: false
}
);
}

#[test]
fn test_parses_additional_settings() {
let empty = YarnRc::from_reader(b"httpProxy: \"http://my-proxy.com\"".as_slice()).unwrap();
assert_eq!(
empty,
YarnRc {
enable_transparent_workspaces: true
}
);
}
Expand Down
Loading