From 41990a003ff863334573951711d2bd56b06cdbca Mon Sep 17 00:00:00 2001 From: John Downey Date: Sat, 13 Jun 2026 06:59:15 +0200 Subject: [PATCH] Stage git path dependencies via worktrees Clone the repo once, check the commit out in a temp worktree, and hard-link the dependency's subdirectory into build/packages. --- compiler-cli/src/dependencies.rs | 497 ++++++++++++++---- compiler-cli/src/dependencies/tests.rs | 313 +++++++++-- compiler-cli/src/fs.rs | 1 + compiler-cli/src/fs/tests.rs | 21 +- compiler-core/src/config.rs | 25 +- compiler-core/src/io.rs | 20 +- compiler-core/src/manifest.rs | 47 +- compiler-core/src/paths.rs | 6 +- compiler-core/src/requirement.rs | 20 +- ...ifest__git_package_with_escaping_path.snap | 9 + ...ad_git_requirement_with_escaping_path.snap | 9 + test/project_git_deps_path/test.sh | 55 ++ 12 files changed, 825 insertions(+), 198 deletions(-) create mode 100644 compiler-core/src/snapshots/gleam_core__manifest__git_package_with_escaping_path.snap create mode 100644 compiler-core/src/snapshots/gleam_core__requirement__tests__read_git_requirement_with_escaping_path.snap diff --git a/compiler-cli/src/dependencies.rs b/compiler-cli/src/dependencies.rs index e9111864b..89360ae02 100644 --- a/compiler-cli/src/dependencies.rs +++ b/compiler-cli/src/dependencies.rs @@ -520,7 +520,10 @@ async fn add_missing_packages( let ManifestPackageSource::Git { repo, commit, path } = &package.source else { continue; }; - let _ = download_git_package(&package.name, repo, commit, path.as_deref(), paths)?; + + let checkout = + download_git_package(&package.name, repo, commit, path.as_deref(), paths)?; + checkout.cleanup()?; } telemetry.packages_downloaded(start, num_to_download); } @@ -563,6 +566,36 @@ fn remove_extra_packages( } } } + + remove_unused_git_clones(paths, manifest)?; + Ok(()) +} + +fn remove_unused_git_clones(paths: &ProjectPaths, manifest: &Manifest) -> Result<()> { + let git_directory = paths.build_git_directory(); + if !git_directory.is_dir() { + return Ok(()); + } + + let expected: HashSet = manifest + .packages + .iter() + .filter_map(|package| match &package.source { + ManifestPackageSource::Git { + repo, + path: Some(_), + .. + } => Some(git_repo_dir_name(repo)), + _ => None, + }) + .collect(); + + for entry in fs::read_dir(&git_directory)?.filter_map(Result::ok) { + if !expected.contains(entry.file_name()) { + tracing::debug!(path=%entry.path(), "removing_unused_git_clone"); + fs::delete_directory(entry.path())?; + } + } Ok(()) } @@ -910,6 +943,35 @@ impl PartialEq for ProvidedPackageSource { } } +/// Where a provided package came from. Git sources carry the on-disc repository +/// root so path dependencies can be resolved relative to it. +enum SourceContext<'a> { + Local { + path: Utf8PathBuf, + }, + Git { + repo: EcoString, + commit: EcoString, + path: Option, + repo_root: &'a Utf8Path, + }, +} + +impl SourceContext<'_> { + fn to_provided_source(&self) -> ProvidedPackageSource { + match self { + Self::Local { path } => ProvidedPackageSource::Local { path: path.clone() }, + Self::Git { + repo, commit, path, .. + } => ProvidedPackageSource::Git { + repo: repo.clone(), + commit: commit.clone(), + path: path.clone(), + }, + } + } +} + /// Provide a package from a local project fn provide_local_package( package_name: EcoString, @@ -925,19 +987,50 @@ fn provide_local_package( fs::canonicalise(&parent_path.join(package_path))? }; - let package_source = ProvidedPackageSource::Local { - path: package_path.clone(), - }; provide_package( package_name, - package_path, - package_source, + package_path.clone(), + SourceContext::Local { path: package_path }, project_paths, provided, parents, ) } +/// Resolve a path dependency of a git package to its canonical filesystem +/// location and its repository-relative path. +fn resolve_git_path_package( + package_name: &EcoString, + path: &Utf8Path, + repo: &EcoString, + parent_path: &Utf8Path, + repo_root: &Utf8Path, +) -> Result<(Utf8PathBuf, Utf8PathBuf)> { + let location = parent_path.join(path); + if !location.is_dir() { + return Err(Error::GitDependencyPathNotFound { + package: package_name.to_string(), + path: path.to_string(), + repo: repo.to_string(), + }); + } + + // The path may name a symlink that points outside the repository + // checkout, so resolve it and take the repository-relative path from the + // canonical location. + let package_path = fs::canonicalise(&location)?; + let repo_path = package_path + .strip_prefix(repo_root) + .map_err(|_| Error::GitDependencyPathNotFound { + package: package_name.to_string(), + path: path.to_string(), + repo: repo.to_string(), + })? + .to_path_buf(); + + Ok((package_path, repo_path)) +} + fn execute_command(command: &mut Command) -> Result { let result = command.output(); match result { @@ -966,41 +1059,47 @@ fn execute_command(command: &mut Command) -> Result { } } -fn git_repo_dir_name(repo: &str, ref_: &str) -> String { - let sanitized = repo +fn git_repo_dir_name(repo: &str) -> String { + let name = repo + .trim_end_matches('/') .trim_end_matches(".git") - .replace("://", "-") - .replace(['/', ':', '@'], "-"); - let sanitized = sanitized.trim_matches('-'); - let hash = xxhash_rust::xxh3::xxh3_64(format!("{repo}\0{ref_}").as_bytes()); - format!("{sanitized}-{hash:016x}") + .rsplit(['/', ':']) + .next() + .filter(|name| !name.is_empty()) + .unwrap_or("repo"); + let hash = xxhash_rust::xxh3::xxh3_64(repo.as_bytes()); + format!("{name}-{hash:016x}") } -/// Reject paths that escape the repository root via `..` components or absolute -/// paths so that typos like `path = "../oops"` fail early with a clear error -/// instead of silently linking the wrong directory. -fn validate_git_dependency_path(package_name: &str, path: &Utf8Path, repo: &str) -> Result<()> { - if path.is_absolute() || path.as_str().starts_with('/') { - return Err(Error::GitDependencyPathNotFound { - package: package_name.into(), - path: path.to_string(), - repo: repo.into(), - }); - } - for component in path.components() { - if matches!(component, camino::Utf8Component::ParentDir) { - return Err(Error::GitDependencyPathNotFound { - package: package_name.into(), - path: path.to_string(), - repo: repo.into(), - }); +fn git_staging_path(project_paths: &ProjectPaths, repo: &str) -> Utf8PathBuf { + project_paths.build_git_repo(&format!("{}-staging", git_repo_dir_name(repo))) +} + +enum GitCheckout { + InPlace { + commit: EcoString, + }, + Staged { + commit: EcoString, + staging_path: Utf8PathBuf, + }, +} + +impl GitCheckout { + fn cleanup(self) -> Result<()> { + // The clones dangling worktree registration is left for the next + // download to prune before it re-adds the staging worktree. + if let Self::Staged { staging_path, .. } = self { + fs::delete_directory(&staging_path)?; } + Ok(()) } - Ok(()) } -/// Downloads a git package from a remote repository. The commands that are run -/// looks like this: +/// Downloads a git package from a remote repository. +/// +/// For a package at the root of its repository the commands that are run look +/// like this, cloning directly into `build/packages/`: /// /// ```sh /// git init @@ -1011,6 +1110,25 @@ fn validate_git_dependency_path(package_name: &str, path: &Utf8Path, repo: &str) /// git rev-parse HEAD /// ``` /// +/// For a package in a subdirectory of its repository the repository is +/// instead cloned into `build/git/-` without ever populating +/// its worktree, and the package is checked out into a transient staging +/// worktree which the subdirectory is hard-linked out of: +/// +/// ```sh +/// git init +/// git remote remove origin +/// git remote add origin +/// git fetch origin +/// git rev-parse --verify --quiet ^{commit} +/// git worktree prune +/// git worktree add --force --detach -staging +/// ``` +/// +/// The staging worktree exists only for the duration of one package download. +/// It is deleted (and the clone's worktree registration pruned) by +/// `GitCheckout::cleanup` once the package has been hard-linked and read. +/// /// This is somewhat inefficient as we have to fetch the entire git history before /// switching to the exact commit we want. There a few alternatives to this: /// @@ -1035,31 +1153,110 @@ fn download_git_package( ref_: &str, path: Option<&Utf8Path>, project_paths: &ProjectPaths, -) -> Result { - // When a subdirectory path is specified, clone to a staging area. - // Otherwise clone directly to the package directory. - let (clone_path, subdir) = match path { - None => (project_paths.build_packages_package(package_name), None), +) -> Result { + match path { + None => download_git_package_in_place(package_name, repo, ref_, project_paths), Some(subdir) => { - validate_git_dependency_path(package_name, subdir, repo)?; - - let staging_name = git_repo_dir_name(repo, ref_); - let staging_path = project_paths.build_git_repo(&staging_name); - (staging_path, Some(subdir)) + download_git_package_to_staged_path(package_name, repo, ref_, project_paths, subdir) } + } +} + +fn download_git_package_in_place( + package_name: &str, + repo: &str, + ref_: &str, + project_paths: &ProjectPaths, +) -> Result { + let clone_path = project_paths.build_packages_package(package_name); + prepare_git_clone(&clone_path, repo)?; + let _ = execute_command( + Command::new("git") + .arg("checkout") + .arg(ref_) + .current_dir(&clone_path), + )?; + let output = execute_command( + Command::new("git") + .arg("rev-parse") + .arg("HEAD") + .current_dir(&clone_path), + )?; + let commit = String::from_utf8(output.stdout) + .expect("Output should be UTF-8") + .trim() + .into(); + + Ok(GitCheckout::InPlace { commit }) +} + +fn download_git_package_to_staged_path( + package_name: &str, + repo: &str, + ref_: &str, + project_paths: &ProjectPaths, + subdir: &Utf8Path, +) -> Result { + let clone_path = project_paths.build_git_repo(&git_repo_dir_name(repo)); + prepare_git_clone(&clone_path, repo)?; + + let commit = resolve_git_ref(&clone_path, repo, ref_)?; + + // Delete any staging worktree left behind by a previous crash, and prune + // its registration from the clone so `git worktree add` can reuse the + // path. + let staging_path = git_staging_path(project_paths, repo); + fs::delete_directory(&staging_path)?; + let _ = Command::new("git") + .arg("worktree") + .arg("prune") + .current_dir(&clone_path) + .output(); + + let _ = execute_command( + Command::new("git") + .arg("worktree") + .arg("add") + .arg("--force") + .arg("--detach") + .arg(&staging_path) + .arg(commit.as_str()) + .current_dir(&clone_path), + )?; + + let Some(subdir_source) = resolve_git_subdir(&staging_path, subdir) else { + return Err(Error::GitDependencyPathNotFound { + package: package_name.into(), + path: subdir.to_string(), + repo: repo.into(), + }); }; + let package_path = project_paths.build_packages_package(package_name); + fs::delete_directory(&package_path)?; + fs::mkdir(&package_path)?; + fs::hardlink_dir(&subdir_source, &package_path)?; + + Ok(GitCheckout::Staged { + commit, + staging_path, + }) +} + +/// Initialise (or reuse) a git clone at the given path and fetch from the +/// remote repository. +fn prepare_git_clone(clone_path: &Utf8Path, repo: &str) -> Result<()> { // If the clone path exists but is not inside a git work tree, we need to // remove the directory because running `git init` in a non-empty directory // followed by `git checkout ...` is an error. See // https://github.com/gleam-lang/gleam/issues/4488 for details. - if !fs::is_git_work_tree_root(&clone_path) { - fs::delete_directory(&clone_path)?; + if !fs::is_git_work_tree_root(clone_path) { + fs::delete_directory(clone_path)?; } - fs::mkdir(&clone_path)?; + fs::mkdir(clone_path)?; - let _ = execute_command(Command::new("git").arg("init").current_dir(&clone_path))?; + let _ = execute_command(Command::new("git").arg("init").current_dir(clone_path))?; // If this directory already exists, but the remote URL has been edited in // `gleam.toml` without a `gleam clean`, `git remote add` will fail, causing @@ -1071,7 +1268,7 @@ fn download_git_package( .arg("remote") .arg("remove") .arg("origin") - .current_dir(&clone_path) + .current_dir(clone_path) .output(); let _ = execute_command( @@ -1080,53 +1277,90 @@ fn download_git_package( .arg("add") .arg("origin") .arg(repo) - .current_dir(&clone_path), + .current_dir(clone_path), )?; let _ = execute_command( Command::new("git") .arg("fetch") .arg("origin") - .current_dir(&clone_path), + .current_dir(clone_path), )?; - let _ = execute_command( - Command::new("git") - .arg("checkout") - .arg(ref_) - .current_dir(&clone_path), - )?; + Ok(()) +} - let output = execute_command( - Command::new("git") +/// Resolve a ref (a branch name, tag, or full or partial commit hash) to a +/// full commit hash using the objects fetched into the clone, without +/// checking anything out. Branch names only exist as remote-tracking refs in +/// the never-checked-out clone, so when the ref does not resolve directly we +/// fall back to `origin/`. +fn resolve_git_ref(clone_path: &Utf8Path, repo: &str, ref_: &str) -> Result { + let revisions = [ + format!("{ref_}^{{commit}}"), + format!("origin/{ref_}^{{commit}}"), + ]; + + for revision in revisions { + let result = Command::new("git") .arg("rev-parse") - .arg("HEAD") - .current_dir(&clone_path), - )?; + .arg("--verify") + .arg("--quiet") + .arg(&revision) + .current_dir(clone_path) + .output(); + + match result { + // A failed rev-parse is expected when the ref form doesn't match + // this pattern, so try the next one. + Ok(output) if !output.status.success() => (), + Ok(output) => { + let commit = String::from_utf8(output.stdout) + .expect("Output should be UTF-8") + .trim() + .into(); + return Ok(commit); + } + Err(error) => { + return Err(match error.kind() { + ErrorKind::NotFound => Error::ShellProgramNotFound { + program: "git".into(), + os: fs::get_os(), + }, + other => Error::ShellCommand { + program: "git".into(), + reason: ShellCommandFailureReason::IoError(other), + }, + }); + } + } + } - let commit = String::from_utf8(output.stdout) - .expect("Output should be UTF-8") - .trim() - .into(); + Err(Error::ShellCommand { + program: "git".into(), + reason: ShellCommandFailureReason::ShellCommandError(format!( + "Unable to resolve git ref `{ref_}` for repository `{repo}`\n" + )), + }) +} - // If a subdirectory was specified, hard-link it into the package directory - if let Some(subdir) = subdir { - let subdir_source = clone_path.join(subdir); - if !subdir_source.is_dir() { - return Err(Error::GitDependencyPathNotFound { - package: package_name.into(), - path: subdir.to_string(), - repo: repo.into(), - }); - } +/// Resolves a subdirectory within a cloned git repository, ensuring that the +/// resolved directory (after following any symlinks) is still inside the +/// repository checkout. Returns `None` if the directory does not exist or +/// escapes the checkout. +fn resolve_git_subdir(clone_path: &Utf8Path, subdir: &Utf8Path) -> Option { + let subdir_source = clone_path.join(subdir); + if !subdir_source.is_dir() { + return None; + } - let package_path = project_paths.build_packages_package(package_name); - fs::delete_directory(&package_path)?; - fs::mkdir(&package_path)?; - fs::hardlink_dir(&subdir_source, &package_path)?; + let canonical_clone = fs::canonicalise(clone_path).ok()?; + let canonical_subdir = fs::canonicalise(&subdir_source).ok()?; + if !canonical_subdir.starts_with(&canonical_clone) { + return None; } - Ok(commit) + Some(canonical_subdir) } /// Provide a package from a git repository @@ -1140,45 +1374,59 @@ fn provide_git_package( provided: &mut HashMap, parents: &mut Vec, ) -> Result { - let commit = download_git_package(&package_name, repo, ref_, path.as_deref(), project_paths)?; - - let package_source = ProvidedPackageSource::Git { - repo: repo.into(), - commit, - path: path.clone(), + let checkout = download_git_package(&package_name, repo, ref_, path.as_deref(), project_paths)?; + let (commit, staging_path) = match &checkout { + GitCheckout::InPlace { commit } => (commit, None), + GitCheckout::Staged { + commit, + staging_path, + } => (commit, Some(staging_path)), }; - // When a subdirectory path is used, resolve the package from the staging - // clone so that transitive path dependencies (e.g. `../sibling`) resolve - // against the original repository structure rather than build/packages/. - let package_path = match path { - Some(ref subdir) => { - let staging_name = git_repo_dir_name(repo, ref_); - let staging_path = project_paths.build_git_repo(&staging_name); - fs::canonicalise(&staging_path.join(subdir))? + // Use the package's location in the staging worktree, where its gleam.toml + // lives, not build/packages/. A `../sibling` path dep is resolved next to + // that gleam.toml, and the sibling is only present in the worktree. + let (package_path, repo_root) = match (&path, staging_path) { + (Some(subdir), Some(staging_path)) => { + let repo_root = fs::canonicalise(staging_path)?; + (fs::canonicalise(&staging_path.join(subdir))?, repo_root) + } + _ => { + let package_path = + fs::canonicalise(&project_paths.build_packages_package(&package_name))?; + (package_path.clone(), package_path) } - None => fs::canonicalise(&project_paths.build_packages_package(&package_name))?, }; - provide_package( + let version = provide_package( package_name, package_path, - package_source, + SourceContext::Git { + repo: repo.into(), + commit: commit.clone(), + path: path.clone(), + repo_root: &repo_root, + }, project_paths, provided, parents, - ) + )?; + + checkout.cleanup()?; + Ok(version) } /// Adds a gleam project located at a specific path to the list of "provided packages" fn provide_package( package_name: EcoString, package_path: Utf8PathBuf, - package_source: ProvidedPackageSource, + source: SourceContext<'_>, project_paths: &ProjectPaths, provided: &mut HashMap, parents: &mut Vec, ) -> Result { + let package_source = source.to_provided_source(); + // Return early if a package cycle is detected if parents.contains(&package_name) { let mut last_cycle = parents @@ -1225,17 +1473,44 @@ fn provide_package( for (name, requirement) in config.dependencies.into_iter() { let version = match requirement { Requirement::Hex { version } => version, - Requirement::Path { path } => { - // Recursively walk local packages - provide_local_package( - name.clone(), - &path, - &package_path, - project_paths, - provided, - parents, - )? - } + Requirement::Path { path } => match &source { + // A path dependency of a git package points to another + // package within the same repository, so lock it as a git + // source to keep the manifest portable. + SourceContext::Git { + repo, + commit, + repo_root, + .. + } => { + let (child_path, child_repo_path) = + resolve_git_path_package(&name, &path, repo, &package_path, repo_root)?; + provide_package( + name.clone(), + child_path, + SourceContext::Git { + repo: repo.clone(), + commit: commit.clone(), + path: Some(child_repo_path), + repo_root, + }, + project_paths, + provided, + parents, + )? + } + SourceContext::Local { .. } => { + // Recursively walk local packages + provide_local_package( + name.clone(), + &path, + &package_path, + project_paths, + provided, + parents, + )? + } + }, Requirement::Git { git, ref_, path } => provide_git_package( name.clone(), &git, diff --git a/compiler-cli/src/dependencies/tests.rs b/compiler-cli/src/dependencies/tests.rs index c3b840f8d..b7d03abfb 100644 --- a/compiler-cli/src/dependencies/tests.rs +++ b/compiler-cli/src/dependencies/tests.rs @@ -554,7 +554,7 @@ fn provide_conflicting_package() { let result = provide_package( "hello_world".into(), Utf8PathBuf::from("./test/other"), - ProvidedPackageSource::Local { + SourceContext::Local { path: Utf8Path::new("./test/other").to_path_buf(), }, &project_paths, @@ -815,88 +815,303 @@ fn provided_git_to_manifest() { } #[test] -fn validate_git_dependency_path_accepts_subdir() { - assert!( - validate_git_dependency_path( - "package", - Utf8Path::new("subdir"), - "https://github.com/gleam-lang/gleam.git" - ) - .is_ok() +fn provided_git_path_package_resolves_repo_relative_path() { + let repo_root = fs::canonicalise(Utf8Path::new("./test")).unwrap(); + let result = resolve_git_path_package( + &"hello_world".into(), + Utf8Path::new("../hello_world"), + &"https://github.com/gleam-lang/wibble.git".into(), + &repo_root.join("hello_world"), + &repo_root, + ); + assert_eq!( + result, + Ok((repo_root.join("hello_world"), "hello_world".into())) ); } #[test] -fn validate_git_dependency_path_accepts_nested_subdir() { - assert!( - validate_git_dependency_path( - "package", - Utf8Path::new("packages/subdir"), - "https://github.com/gleam-lang/gleam.git" - ) - .is_ok() +fn provided_git_path_package_child_uses_canonical_parent_location() { + let tmp = tempfile::tempdir().unwrap(); + let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let repo = base.join("repo"); + fs::mkdir(&repo.join("packages").join("package_a")).unwrap(); + fs::mkdir(&repo.join("packages").join("package_b")).unwrap(); + fs::write( + &repo.join("packages").join("package_a").join("gleam.toml"), + r#" +name = "package_a" +version = "0.1.0" + +[dependencies] +package_b = { path = "../package_b" } +"#, + ) + .unwrap(); + fs::write( + &repo.join("packages").join("package_b").join("gleam.toml"), + r#" +name = "package_b" +version = "0.2.0" +"#, + ) + .unwrap(); + + let mut provided = HashMap::new(); + let project_paths = crate::project_paths_at_current_directory_without_toml(); + let repo_root = fs::canonicalise(&repo).unwrap(); + let result = provide_package( + "package_a".into(), + fs::canonicalise(&repo.join("packages").join("package_a")).unwrap(), + SourceContext::Git { + repo: "https://github.com/gleam-lang/wibble.git".into(), + commit: "95cd2c2f45907e5571e9b5fcdfb27ff35cdcdd29".into(), + path: Some("packages/package_a".into()), + repo_root: &repo_root, + }, + &project_paths, + &mut provided, + &mut vec!["root".into()], + ); + assert_eq!( + result, + Ok(hexpm::version::Range::new("== 0.1.0".into()).unwrap()) + ); + let package = provided.get("package_b").unwrap(); + assert_eq!( + package.source, + ProvidedPackageSource::Git { + repo: "https://github.com/gleam-lang/wibble.git".into(), + commit: "95cd2c2f45907e5571e9b5fcdfb27ff35cdcdd29".into(), + path: Some("packages/package_b".into()), + } ); } #[test] -fn validate_git_dependency_path_rejects_parent_traversal() { - let result = validate_git_dependency_path( - "package", - Utf8Path::new("../escape"), - "https://github.com/gleam-lang/gleam.git", +fn provided_git_path_package_rejects_missing_directory() { + let tmp = tempfile::tempdir().unwrap(); + let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let repo = base.join("repo"); + fs::mkdir(&repo.join("parent")).unwrap(); + + let repo_root = fs::canonicalise(&repo).unwrap(); + let result = resolve_git_path_package( + &"missing".into(), + Utf8Path::new("../missing"), + &"https://github.com/gleam-lang/wibble.git".into(), + &repo_root.join("parent"), + &repo_root, ); - assert!(matches!( + assert_eq!( result, - Err(Error::GitDependencyPathNotFound { .. }) - )); + Err(Error::GitDependencyPathNotFound { + package: "missing".into(), + path: "../missing".into(), + repo: "https://github.com/gleam-lang/wibble.git".into(), + }) + ); } #[test] -fn validate_git_dependency_path_rejects_nested_parent_traversal() { - let result = validate_git_dependency_path( - "package", - Utf8Path::new("packages/../../escape"), - "https://github.com/gleam-lang/gleam.git", +fn resolve_git_subdir_accepts_directory_inside_repo() { + let tmp = tempfile::tempdir().unwrap(); + let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let clone = base.join("clone"); + fs::mkdir(&clone.join("sub")).unwrap(); + + let expected = fs::canonicalise(&clone).unwrap().join("sub"); + assert_eq!( + resolve_git_subdir(&clone, Utf8Path::new("sub")), + Some(expected) ); - assert!(matches!( +} + +#[test] +fn resolve_git_subdir_rejects_missing_directory() { + let tmp = tempfile::tempdir().unwrap(); + let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let clone = base.join("clone"); + fs::mkdir(&clone).unwrap(); + + assert_eq!(resolve_git_subdir(&clone, Utf8Path::new("missing")), None); +} + +#[cfg(unix)] +#[test] +fn resolve_git_subdir_rejects_symlink_escaping_repo() { + let tmp = tempfile::tempdir().unwrap(); + let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let clone = base.join("clone"); + let outside = base.join("outside"); + fs::mkdir(&clone).unwrap(); + fs::mkdir(&outside).unwrap(); + std::os::unix::fs::symlink(outside.as_std_path(), clone.join("escape").as_std_path()).unwrap(); + + assert_eq!(resolve_git_subdir(&clone, Utf8Path::new("escape")), None); +} + +#[test] +fn provided_git_path_package_rejects_escaping_path() { + let tmp = tempfile::tempdir().unwrap(); + let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let repo = base.join("repo"); + let outside = base.join("outside"); + fs::mkdir(&repo.join("parent")).unwrap(); + fs::mkdir(&outside).unwrap(); + + let repo_root = fs::canonicalise(&repo).unwrap(); + let result = resolve_git_path_package( + &"package_a".into(), + Utf8Path::new("../../outside"), + &"https://github.com/gleam-lang/wibble.git".into(), + &repo_root.join("parent"), + &repo_root, + ); + assert_eq!( result, - Err(Error::GitDependencyPathNotFound { .. }) - )); + Err(Error::GitDependencyPathNotFound { + package: "package_a".into(), + path: "../../outside".into(), + repo: "https://github.com/gleam-lang/wibble.git".into(), + }) + ); } +#[cfg(unix)] #[test] -fn validate_git_dependency_path_rejects_absolute_path() { - let result = validate_git_dependency_path( - "package", - Utf8Path::new("/etc/passwd"), - "https://github.com/gleam-lang/gleam.git", +fn provided_git_path_package_rejects_symlink_escaping_repo() { + let tmp = tempfile::tempdir().unwrap(); + let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let repo = base.join("repo"); + let outside = base.join("outside"); + fs::mkdir(&repo.join("parent")).unwrap(); + fs::mkdir(&outside).unwrap(); + std::os::unix::fs::symlink(outside.as_std_path(), repo.join("escape").as_std_path()).unwrap(); + + let repo_root = fs::canonicalise(&repo).unwrap(); + let result = resolve_git_path_package( + &"escape".into(), + Utf8Path::new("../escape"), + &"https://github.com/gleam-lang/wibble.git".into(), + &repo_root.join("parent"), + &repo_root, ); - assert!(matches!( + assert_eq!( result, - Err(Error::GitDependencyPathNotFound { .. }) - )); + Err(Error::GitDependencyPathNotFound { + package: "escape".into(), + path: "../escape".into(), + repo: "https://github.com/gleam-lang/wibble.git".into(), + }) + ); } #[test] -fn git_repo_dir_name_produces_expected_name() { +fn git_repo_dir_name_uses_repo_basename_and_hash_of_full_url() { assert_eq!( - git_repo_dir_name("https://github.com/gleam-lang/gleam.git", "main"), - "https-github.com-gleam-lang-gleam-edfec3bb6bbeb6c1" + git_repo_dir_name("https://github.com/gleam-lang/gleam"), + "gleam-4d40009bf5eb0110" + ); + assert_eq!( + git_repo_dir_name("https://github.com/gleam-lang/gleam.git"), + "gleam-ea2aeaa3761e8bc6" + ); + assert_eq!( + git_repo_dir_name("git@github.com:gleam-lang/gleam.git"), + "gleam-102c9e1e5cf87965" ); } #[test] -fn git_repo_dir_name_different_refs_produce_different_names() { +fn git_staging_path_is_clone_path_with_staging_suffix() { + let paths = ProjectPaths::new("/app".into()); + let repo = "https://github.com/gleam-lang/gleam.git"; assert_eq!( - git_repo_dir_name("https://github.com/gleam-lang/gleam.git", "main"), - "https-github.com-gleam-lang-gleam-edfec3bb6bbeb6c1" + paths.build_git_repo(&git_repo_dir_name(repo)), + Utf8PathBuf::from("/app/build/git/gleam-ea2aeaa3761e8bc6") ); assert_eq!( - git_repo_dir_name("https://github.com/gleam-lang/gleam.git", "v1.0.0"), - "https-github.com-gleam-lang-gleam-90203c669cc91eee" + git_staging_path(&paths, repo), + Utf8PathBuf::from("/app/build/git/gleam-ea2aeaa3761e8bc6-staging") ); } +#[test] +fn git_checkout_cleanup_deletes_staging_directory() { + let tmp = tempfile::tempdir().unwrap(); + let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let staging_path = base.join("clone-staging"); + fs::mkdir(&staging_path).unwrap(); + fs::write(&staging_path.join("gleam.toml"), "name = \"wibble\"").unwrap(); + + let checkout = GitCheckout::Staged { + commit: "95cd2c2f45907e5571e9b5fcdfb27ff35cdcdd29".into(), + staging_path: staging_path.clone(), + }; + assert_eq!(checkout.cleanup(), Ok(())); + + assert!(!staging_path.exists()); +} + +#[test] +fn git_checkout_cleanup_without_staging_is_noop() { + let checkout = GitCheckout::InPlace { + commit: "95cd2c2f45907e5571e9b5fcdfb27ff35cdcdd29".into(), + }; + assert_eq!(checkout.cleanup(), Ok(())); +} + +#[test] +fn remove_unused_git_clones_sweeps_unexpected_directories() { + let tmp = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let paths = ProjectPaths::new(root); + let repo = "https://github.com/gleam-lang/gleam.git"; + + let expected_clone = paths.build_git_repo(&git_repo_dir_name(repo)); + let stale_clone = paths.build_git_repo("other-0011223344556677"); + let stale_staging = paths.build_git_repo(&format!("{}-staging", git_repo_dir_name(repo))); + fs::mkdir(&expected_clone).unwrap(); + fs::mkdir(&stale_clone).unwrap(); + fs::mkdir(&stale_staging).unwrap(); + + let manifest = Manifest { + requirements: HashMap::new(), + packages: vec![ManifestPackage { + name: "wibble".into(), + version: Version::new(1, 0, 0), + build_tools: ["gleam".into()].into(), + otp_app: None, + requirements: vec![], + source: ManifestPackageSource::Git { + repo: repo.into(), + commit: "95cd2c2f45907e5571e9b5fcdfb27ff35cdcdd29".into(), + path: Some("wibble".into()), + }, + }], + }; + + assert_eq!(remove_unused_git_clones(&paths, &manifest), Ok(())); + + assert!(expected_clone.is_dir()); + assert!(!stale_clone.exists()); + assert!(!stale_staging.exists()); +} + +#[test] +fn remove_unused_git_clones_missing_directory_is_noop() { + let tmp = tempfile::tempdir().unwrap(); + let root = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + let paths = ProjectPaths::new(root); + let manifest = Manifest { + requirements: HashMap::new(), + packages: vec![], + }; + + assert_eq!(remove_unused_git_clones(&paths, &manifest), Ok(())); +} + #[test] fn verified_requirements_equality_with_canonicalized_paths() { let temp_dir = tempfile::tempdir().expect("Failed to create a temp directory"); diff --git a/compiler-cli/src/fs.rs b/compiler-cli/src/fs.rs index 86abc99ef..7224471ec 100644 --- a/compiler-cli/src/fs.rs +++ b/compiler-cli/src/fs.rs @@ -733,6 +733,7 @@ pub fn hardlink(from: impl AsRef, to: impl AsRef) -> Result< .map(|_| ()) } +/// Recursively hardlinks all files from one directory into another. pub fn hardlink_dir( from: impl AsRef + Debug, to: impl AsRef + Debug, diff --git a/compiler-cli/src/fs/tests.rs b/compiler-cli/src/fs/tests.rs index 956999146..a7c84b7e1 100644 --- a/compiler-cli/src/fs/tests.rs +++ b/compiler-cli/src/fs/tests.rs @@ -217,26 +217,7 @@ fn hardlink_dir_copies_files_and_directories() { } #[test] -fn hardlink_dir_skips_git_directory() { - let tmp = tempfile::tempdir().unwrap(); - let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); - let src = base.join("src"); - let dest = base.join("dest"); - - super::mkdir(&src).unwrap(); - super::mkdir(&src.join(".git")).unwrap(); - super::write(&src.join("a.txt"), "content").unwrap(); - super::write(&src.join(".git").join("config"), "gitdata").unwrap(); - - super::mkdir(&dest).unwrap(); - super::hardlink_dir(&src, &dest).unwrap(); - - assert!(dest.join("a.txt").exists()); - assert!(!dest.join(".git").exists()); -} - -#[test] -fn hardlink_dir_empty_source() { +fn hardlink_dir_empty_source_creates_empty_dest() { let tmp = tempfile::tempdir().unwrap(); let base = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); let src = base.join("src"); diff --git a/compiler-core/src/config.rs b/compiler-core/src/config.rs index 4c942c419..6396a9d00 100644 --- a/compiler-core/src/config.rs +++ b/compiler-core/src/config.rs @@ -1018,8 +1018,8 @@ pub struct DocsPage { pub source: Utf8PathBuf, } -mod package_scoped_path { - use camino::{Utf8Component, Utf8PathBuf}; +pub(crate) mod package_scoped_path { + use camino::Utf8PathBuf; use serde::{Deserialize, Deserializer, de::Error as _}; pub fn deserialize<'de, D>(deserializer: D) -> Result @@ -1027,16 +1027,17 @@ mod package_scoped_path { D: Deserializer<'de>, { let path = Utf8PathBuf::deserialize(deserializer)?; - // Absolute paths are not permitted. - // On Windows paths starting with \\ are drive-relative, so absolute as - // far as we are concerned. - if path.is_absolute() || (cfg!(windows) && path.starts_with("\\")) { - return Err(D::Error::custom("paths must be relative")); - } - for component in path.components() { - if component == Utf8Component::ParentDir { - return Err(D::Error::custom("paths must not contain .. segments")); - } + crate::io::validate_safe_relative_path(&path).map_err(D::Error::custom)?; + Ok(path) + } + + pub fn optional_deserialize<'de, D>(deserializer: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + let path = Option::::deserialize(deserializer)?; + if let Some(path) = &path { + crate::io::validate_safe_relative_path(path).map_err(D::Error::custom)?; } Ok(path) } diff --git a/compiler-core/src/io.rs b/compiler-core/src/io.rs index e661afe01..f721eb505 100644 --- a/compiler-core/src/io.rs +++ b/compiler-core/src/io.rs @@ -17,7 +17,25 @@ use std::{ }; use tar::{Archive, Entry}; -use camino::{Utf8Path, Utf8PathBuf}; +use camino::{Utf8Component, Utf8Path, Utf8PathBuf}; + +/// Validates that a path is safe to use as a relative path. +pub fn validate_safe_relative_path(path: &Utf8Path) -> Result<(), &'static str> { + // Absolute paths are not permitted. + // On Windows paths starting with \\ are drive-relative, so absolute as + // far as we are concerned. + if path.is_absolute() || (cfg!(windows) && path.starts_with("\\")) { + return Err("paths must be relative"); + } + + for component in path.components() { + if component == Utf8Component::ParentDir { + return Err("paths must not contain .. segments"); + } + } + + Ok(()) +} /// Takes in a source path and a target path and determines a relative path /// from source -> target. diff --git a/compiler-core/src/manifest.rs b/compiler-core/src/manifest.rs index f69d11e65..02c12f072 100644 --- a/compiler-core/src/manifest.rs +++ b/compiler-core/src/manifest.rs @@ -249,7 +249,11 @@ pub enum ManifestPackageSource { Git { repo: EcoString, commit: EcoString, - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde( + default, + skip_serializing_if = "Option::is_none", + deserialize_with = "super::config::package_scoped_path::optional_deserialize" + )] path: Option, }, #[serde(rename = "local")] @@ -313,6 +317,14 @@ mod tests { "awsome_local2".into(), Requirement::git("https://github.com/gleam-lang/gleam.git", "bd9fe02f"), ), + ( + "awsome_local3".into(), + Requirement::git_with_path( + "https://github.com/gleam-lang/gleam.git", + "bd9fe02f", + "packages/sub", + ), + ), ( "awsome_local1".into(), Requirement::path("../path/to/package"), @@ -364,6 +376,18 @@ mod tests { path: None, }, }, + ManifestPackage { + name: "awsome_local3".into(), + version: Version::new(1, 2, 3), + build_tools: ["gleam".into()].into(), + otp_app: None, + requirements: vec![], + source: ManifestPackageSource::Git { + repo: "https://github.com/gleam-lang/gleam.git".into(), + commit: "bd9fe02f72250e6a136967917bcb1bdccaffa3c8".into(), + path: Some("packages/sub".into()), + }, + }, ManifestPackage { name: "awsome_local1".into(), version: Version::new(1, 2, 3), @@ -402,6 +426,7 @@ packages = [ { name = "aaa", version = "0.4.0", build_tools = ["rebar3", "make"], requirements = ["gleam_stdlib", "zzz"], otp_app = "aaa_app", source = "hex", outer_checksum = "0316" }, { name = "awsome_local1", version = "1.2.3", build_tools = ["gleam"], requirements = [], source = "local", path = "../path/to/package" }, { name = "awsome_local2", version = "1.2.3", build_tools = ["gleam"], requirements = [], source = "git", repo = "https://github.com/gleam-lang/gleam.git", commit = "bd9fe02f72250e6a136967917bcb1bdccaffa3c8" }, + { name = "awsome_local3", version = "1.2.3", build_tools = ["gleam"], requirements = [], source = "git", repo = "https://github.com/gleam-lang/gleam.git", commit = "bd9fe02f72250e6a136967917bcb1bdccaffa3c8", path = "packages/sub" }, { name = "gleam_stdlib", version = "0.17.1", build_tools = ["gleam"], requirements = [], source = "hex", outer_checksum = "0116" }, { name = "gleeunit", version = "0.4.0", build_tools = ["gleam"], requirements = ["gleam_stdlib"], source = "hex", outer_checksum = "032E" }, { name = "zzz", version = "0.4.0", build_tools = ["mix"], requirements = [], source = "hex", outer_checksum = "0316" }, @@ -411,6 +436,7 @@ packages = [ aaa = { version = "> 0.0.0" } awsome_local1 = { path = "../path/to/package" } awsome_local2 = { git = "https://github.com/gleam-lang/gleam.git", ref = "bd9fe02f" } +awsome_local3 = { git = "https://github.com/gleam-lang/gleam.git", ref = "bd9fe02f", path = "packages/sub" } gleam_stdlib = { version = "~> 0.17" } gleeunit = { version = "~> 0.1" } zzz = { version = "> 0.0.0" } @@ -849,6 +875,25 @@ gleam_stdlib = { version = ">= 0.58.0 and < 2.0.0" } insta::assert_snapshot!(insta::internals::AutoName, error.to_string()); } +#[test] +fn git_package_with_escaping_path() { + let toml = r#"# This file was generated by Gleam +# You typically do not need to edit this file + +packages = [ + { name = "wibble", version = "0.1.0", build_tools = ["gleam"], requirements = [], source = "git", repo = "https://github.com/gleam-lang/gleam.git", commit = "bd9fe02f72250e6a136967917bcb1bdccaffa3c8", path = "../escape" }, +] + +[requirements] +wibble = { git = "https://github.com/gleam-lang/gleam.git", ref = "main", path = "wibble" } +"#; + + let manifest: Result = toml::from_str(toml); + let error = + manifest.expect_err("should fail to deserialise because path escapes the repository"); + insta::assert_snapshot!(insta::internals::AutoName, error.to_string()); +} + #[test] fn no_otp_app() { // This is valid toml but wibble/wobble is not a valid package name diff --git a/compiler-core/src/paths.rs b/compiler-core/src/paths.rs index 1d4ec1b38..abeb3636c 100644 --- a/compiler-core/src/paths.rs +++ b/compiler-core/src/paths.rs @@ -66,12 +66,12 @@ impl ProjectPaths { self.build_directory().join("packages") } - pub fn build_git_repos_directory(&self) -> Utf8PathBuf { - self.build_directory().join("git_repos") + pub fn build_git_directory(&self) -> Utf8PathBuf { + self.build_directory().join("git") } pub fn build_git_repo(&self, name: &str) -> Utf8PathBuf { - self.build_git_repos_directory().join(name) + self.build_git_directory().join(name) } pub fn build_packages_toml(&self) -> Utf8PathBuf { diff --git a/compiler-core/src/requirement.rs b/compiler-core/src/requirement.rs index f0f82d289..a17a456e6 100644 --- a/compiler-core/src/requirement.rs +++ b/compiler-core/src/requirement.rs @@ -170,7 +170,14 @@ impl<'de> Deserialize<'de> for Requirement { where D: Deserializer<'de>, { - deserializer.deserialize_any(RequirementVisitor) + let requirement = deserializer.deserialize_any(RequirementVisitor)?; + if let Requirement::Git { + path: Some(path), .. + } = &requirement + { + crate::io::validate_safe_relative_path(path).map_err(de::Error::custom)?; + } + Ok(requirement) } } @@ -208,4 +215,15 @@ mod tests { toml::from_str::>(toml).expect_err("invalid version"); insta::assert_snapshot!(error.to_string()); } + + #[test] + fn read_git_requirement_with_escaping_path() { + let toml = r#" + monorepo = { git = "https://github.com/gleam-lang/gleam.git", ref = "main", path = "../escape" } + "#; + + let error = + toml::from_str::>(toml).expect_err("escaping path"); + insta::assert_snapshot!(error.to_string()); + } } diff --git a/compiler-core/src/snapshots/gleam_core__manifest__git_package_with_escaping_path.snap b/compiler-core/src/snapshots/gleam_core__manifest__git_package_with_escaping_path.snap new file mode 100644 index 000000000..c21704189 --- /dev/null +++ b/compiler-core/src/snapshots/gleam_core__manifest__git_package_with_escaping_path.snap @@ -0,0 +1,9 @@ +--- +source: compiler-core/src/manifest.rs +expression: error.to_string() +--- +TOML parse error at line 5, column 3 + | +5 | { name = "wibble", version = "0.1.0", build_tools = ["gleam"], requirements = [], source = "git", repo = "https://github.com/gleam-lang/gleam.git", commit = "bd9fe02f72250e6a136967917bcb1bdccaffa3c8", path = "../escape" }, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +paths must not contain .. segments diff --git a/compiler-core/src/snapshots/gleam_core__requirement__tests__read_git_requirement_with_escaping_path.snap b/compiler-core/src/snapshots/gleam_core__requirement__tests__read_git_requirement_with_escaping_path.snap new file mode 100644 index 000000000..d003dd5bc --- /dev/null +++ b/compiler-core/src/snapshots/gleam_core__requirement__tests__read_git_requirement_with_escaping_path.snap @@ -0,0 +1,9 @@ +--- +source: compiler-core/src/requirement.rs +expression: error.to_string() +--- +TOML parse error at line 2, column 24 + | +2 | monorepo = { git = "https://github.com/gleam-lang/gleam.git", ref = "main", path = "../escape" } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +paths must not contain .. segments diff --git a/test/project_git_deps_path/test.sh b/test/project_git_deps_path/test.sh index 65da43a34..4338f3fc5 100755 --- a/test/project_git_deps_path/test.sh +++ b/test/project_git_deps_path/test.sh @@ -51,6 +51,7 @@ cd "$REPO_DIR" git init -q git add . git -c user.name="Test" -c user.email="test@example.com" commit -q -m "Initial commit" +git branch -M main REF=$(git rev-parse HEAD) cd "$OLDPWD" @@ -68,6 +69,60 @@ EOF g update g check +echo +echo Testing with a branch ref, two packages from one repo, and a clean rebuild +rm -fr build + +cat > gleam.toml << EOF +name = "git_deps_path" +version = "0.1.0" + +[dependencies] +package_a = { git = "file://${REPO_DIR}", ref = "main", path = "package_a" } +package_b = { git = "file://${REPO_DIR}", ref = "main", path = "package_b" } +EOF + +g update + +echo Checking that the hard-linked packages do not contain a .git entry +if [ -e "build/packages/package_a/.git" ]; then + echo "build/packages/package_a/.git should not exist" + exit 1 +fi +if [ -e "build/packages/package_b/.git" ]; then + echo "build/packages/package_b/.git should not exist" + exit 1 +fi + +EXPECTED_PACKAGE_B=" { name = \"package_b\", version = \"0.1.0\", build_tools = [\"gleam\"], requirements = [], source = \"git\", repo = \"file://${REPO_DIR}\", commit = \"${REF}\", path = \"package_b\" }," +if ! grep -qFx "$EXPECTED_PACKAGE_B" manifest.toml; then + echo "manifest.toml does not lock package_b as a git source. Expected:" + echo "$EXPECTED_PACKAGE_B" + echo "Got:" + cat manifest.toml + exit 1 +fi + +rm -fr build +g check + +echo +echo Testing that an unresolvable ref fails +rm -fr build + +cat > gleam.toml << EOF +name = "git_deps_path" +version = "0.1.0" + +[dependencies] +package_a = { git = "file://${REPO_DIR}", ref = "no-such-ref", path = "package_a" } +EOF + +if g update; then + echo "g update should have failed for an unresolvable ref" + exit 1 +fi + echo echo Success! 💖 echo -- 2.51.2