Skip to content

Commit 47d3cf1

Browse files
authored
fix: pixi-build-rattler-build globs (#159)
* fix: pixi-build-rattler-build globs recipe files are detected under different names and directories. This PR accounts for that. * Clippy lint
1 parent 8cdb878 commit 47d3cf1

File tree

2 files changed

+95
-39
lines changed

2 files changed

+95
-39
lines changed

crates/pixi-build-rattler-build/src/protocol.rs

Lines changed: 87 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,10 @@ impl Protocol for RattlerBuildBackend {
195195
solved_packages.push(conda);
196196
}
197197

198-
let input_globs = Some(Vec::from(["recipe.yaml".to_string()]));
198+
let input_globs = Some(get_metadata_input_globs(
199+
&self.manifest_root,
200+
&self.recipe_source.path,
201+
)?);
199202

200203
Ok(CondaMetadataResult {
201204
packages: solved_packages,
@@ -334,7 +337,11 @@ impl Protocol for RattlerBuildBackend {
334337

335338
built.push(CondaBuiltPackage {
336339
output_file: build_path,
337-
input_globs: build_input_globs(&self.recipe_source.path, package_sources)?,
340+
input_globs: build_input_globs(
341+
&self.manifest_root,
342+
&self.recipe_source.path,
343+
package_sources,
344+
)?,
338345
name: output.name().as_normalized().to_string(),
339346
version: output.version().to_string(),
340347
build: build_string.to_string(),
@@ -345,9 +352,8 @@ impl Protocol for RattlerBuildBackend {
345352
}
346353
}
347354

348-
#[allow(dead_code)]
349355
/// Returns the relative path from `base` to `input`, joined by "/".
350-
fn relative_path_joined(base: &std::path::Path, input: &std::path::Path) -> miette::Result<String> {
356+
fn build_relative_glob(base: &std::path::Path, input: &std::path::Path) -> miette::Result<String> {
351357
let rel = pathdiff::diff_paths(input, base).ok_or_else(|| {
352358
miette::miette!(
353359
"could not compute relative path from '{:?}' to '{:?}'",
@@ -360,16 +366,24 @@ fn relative_path_joined(base: &std::path::Path, input: &std::path::Path) -> miet
360366
.map(|c| c.as_os_str().to_string_lossy())
361367
.collect::<Vec<_>>()
362368
.join("/");
363-
Ok(joined)
369+
370+
if input.is_dir() {
371+
let dir_glob = if joined.is_empty() {
372+
"*".to_string()
373+
} else {
374+
joined
375+
};
376+
Ok(format!("{}/**", dir_glob))
377+
} else {
378+
Ok(joined)
379+
}
364380
}
365381

366382
fn build_input_globs(
383+
manifest_root: &Path,
367384
source: &Path,
368385
package_sources: Option<Vec<PathBuf>>,
369386
) -> miette::Result<Vec<String>> {
370-
// Always add the current directory of the package to the globs
371-
let mut input_globs = vec!["*/**".to_string()];
372-
373387
// Get parent directory path
374388
let parent = if source.is_file() {
375389
// use the parent path as glob
@@ -378,6 +392,10 @@ fn build_input_globs(
378392
// use the source path as glob
379393
source.to_path_buf()
380394
};
395+
396+
// Always add the current directory of the package to the globs
397+
let mut input_globs = Vec::from([build_relative_glob(manifest_root, &parent)?]);
398+
381399
// If there are sources add them to the globs as well
382400
if let Some(package_sources) = package_sources {
383401
for source in package_sources {
@@ -386,18 +404,25 @@ fn build_input_globs(
386404
} else {
387405
parent.join(source)
388406
};
389-
let source_glob = relative_path_joined(&parent, &source)?;
390-
if source.is_dir() {
391-
input_globs.push(format!("{}/**", source_glob));
392-
} else {
393-
input_globs.push(source_glob);
394-
}
407+
input_globs.push(build_relative_glob(manifest_root, &source)?);
395408
}
396409
}
397410

398411
Ok(input_globs)
399412
}
400413

414+
/// Returns the input globs for conda_get_metadata, as used in the CondaMetadataResult.
415+
fn get_metadata_input_globs(
416+
manifest_root: &Path,
417+
recipe_source_path: &Path,
418+
) -> miette::Result<Vec<String>> {
419+
match build_relative_glob(manifest_root, recipe_source_path) {
420+
Ok(rel) if !rel.is_empty() => Ok(vec![rel]),
421+
Ok(_) => Ok(Vec::new()),
422+
Err(e) => Err(e),
423+
}
424+
}
425+
401426
#[async_trait::async_trait]
402427
impl ProtocolInstantiator for RattlerBuildBackendInstantiator {
403428
fn debug_dir(configuration: Option<serde_json::Value>) -> Option<PathBuf> {
@@ -629,24 +654,21 @@ mod tests {
629654
let base = Path::new("/foo/bar");
630655
let input = Path::new("/foo/bar/baz/qux.txt");
631656
assert_eq!(
632-
super::relative_path_joined(base, input).unwrap(),
657+
super::build_relative_glob(base, input).unwrap(),
633658
"baz/qux.txt"
634659
);
635660
// Same path
636661
let base = Path::new("/foo/bar");
637662
let input = Path::new("/foo/bar");
638-
assert_eq!(super::relative_path_joined(base, input).unwrap(), "");
663+
assert_eq!(super::build_relative_glob(base, input).unwrap(), "");
639664
// Input not under base
640665
let base = Path::new("/foo/bar");
641666
let input = Path::new("/foo/other");
642-
assert_eq!(
643-
super::relative_path_joined(base, input).unwrap(),
644-
"../other"
645-
);
667+
assert_eq!(super::build_relative_glob(base, input).unwrap(), "../other");
646668
// Relative paths
647669
let base = Path::new("foo/bar");
648670
let input = Path::new("foo/bar/baz");
649-
assert_eq!(super::relative_path_joined(base, input).unwrap(), "baz");
671+
assert_eq!(super::build_relative_glob(base, input).unwrap(), "baz");
650672
}
651673

652674
#[test]
@@ -656,18 +678,15 @@ mod tests {
656678
let base = Path::new(r"C:\foo\bar");
657679
let input = Path::new(r"C:\foo\bar\baz\qux.txt");
658680
assert_eq!(
659-
super::relative_path_joined(base, input).unwrap(),
681+
super::build_relative_glob(base, input).unwrap(),
660682
"baz/qux.txt"
661683
);
662684
let base = Path::new(r"C:\foo\bar");
663685
let input = Path::new(r"C:\foo\bar");
664-
assert_eq!(super::relative_path_joined(base, input).unwrap(), "");
686+
assert_eq!(super::build_relative_glob(base, input).unwrap(), "");
665687
let base = Path::new(r"C:\foo\bar");
666688
let input = Path::new(r"C:\foo\other");
667-
assert_eq!(
668-
super::relative_path_joined(base, input).unwrap(),
669-
"../other"
670-
);
689+
assert_eq!(super::build_relative_glob(base, input).unwrap(), "../other");
671690
}
672691

673692
#[test]
@@ -682,7 +701,7 @@ mod tests {
682701
// Case 1: source is a file in the base dir
683702
let recipe_path = base_path.join("recipe.yaml");
684703
fs::write(&recipe_path, "fake").unwrap();
685-
let globs = super::build_input_globs(&recipe_path, None).unwrap();
704+
let globs = super::build_input_globs(base_path, &recipe_path, None).unwrap();
686705
assert_eq!(globs, vec!["*/**"]);
687706

688707
// Case 2: source is a directory, with a file and a dir as package sources
@@ -691,9 +710,12 @@ mod tests {
691710
let pkg_subdir = pkg_dir.join("dir");
692711
fs::create_dir_all(&pkg_subdir).unwrap();
693712
fs::write(&pkg_file, "fake").unwrap();
694-
let globs =
695-
super::build_input_globs(base_path, Some(vec![pkg_file.clone(), pkg_subdir.clone()]))
696-
.unwrap();
713+
let globs = super::build_input_globs(
714+
base_path,
715+
base_path,
716+
Some(vec![pkg_file.clone(), pkg_subdir.clone()]),
717+
)
718+
.unwrap();
697719
assert_eq!(globs, vec!["*/**", "pkg/file.txt", "pkg/dir/**"]);
698720
}
699721

@@ -713,10 +735,13 @@ mod tests {
713735
fs::create_dir_all(&package_source_dir).unwrap();
714736

715737
// Call build_input_globs with source_dir as source, and package_source_dir as package source
716-
let globs =
717-
super::build_input_globs(&source_dir, Some(vec![package_source_dir.clone()])).unwrap();
718-
// The relative path from source_dir to package_source_dir should be "../pkgsrc/**"
719-
assert_eq!(globs, vec!["*/**", "../pkgsrc/**"]);
738+
let globs = super::build_input_globs(
739+
temp_path,
740+
&source_dir,
741+
Some(vec![package_source_dir.clone()]),
742+
)
743+
.unwrap();
744+
assert_eq!(globs, vec!["source/**", "pkgsrc/**"]);
720745
}
721746

722747
#[test]
@@ -735,8 +760,34 @@ mod tests {
735760
fs::create_dir_all(&abs_rel_dir).unwrap();
736761

737762
// Call build_input_globs with base_path as source, and rel_dir as package source (relative)
738-
let globs = super::build_input_globs(base_path, Some(vec![rel_dir.clone()])).unwrap();
763+
let globs =
764+
super::build_input_globs(base_path, base_path, Some(vec![rel_dir.clone()])).unwrap();
739765
// The relative path from base_path to rel_dir should be "rel_folder/**"
740766
assert_eq!(globs, vec!["*/**", "rel_folder/**"]);
741767
}
768+
769+
#[test]
770+
fn test_get_metadata_input_globs() {
771+
use std::path::PathBuf;
772+
// Case: file with name
773+
let manifest_root = PathBuf::from("/foo/bar");
774+
let path = PathBuf::from("/foo/bar/recipe.yaml");
775+
let globs = super::get_metadata_input_globs(&manifest_root, &path).unwrap();
776+
assert_eq!(globs, vec!["recipe.yaml"]);
777+
// Case: file with no name (root)
778+
let manifest_root = PathBuf::from("/");
779+
let path = PathBuf::from("/");
780+
let globs = super::get_metadata_input_globs(&manifest_root, &path).unwrap();
781+
assert_eq!(globs, vec!["*/**".to_string()]);
782+
// Case: file with .yml extension
783+
let manifest_root = PathBuf::from("/foo/bar");
784+
let path = PathBuf::from("/foo/bar/recipe.yml");
785+
let globs = super::get_metadata_input_globs(&manifest_root, &path).unwrap();
786+
assert_eq!(globs, vec!["recipe.yml"]);
787+
// Case: file in subdir
788+
let manifest_root = PathBuf::from("/foo");
789+
let path = PathBuf::from("/foo/bar/recipe.yaml");
790+
let globs = super::get_metadata_input_globs(&manifest_root, &path).unwrap();
791+
assert_eq!(globs, vec!["bar/recipe.yaml"]);
792+
}
742793
}

crates/pixi-build-rattler-build/src/rattler_build.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub struct RattlerBuildBackend {
1414
/// In case of rattler-build, manifest is the raw recipe
1515
/// We need to apply later the selectors to get the final recipe
1616
pub(crate) recipe_source: Source,
17+
pub(crate) manifest_root: PathBuf,
1718
pub(crate) cache_dir: Option<PathBuf>,
1819
pub(crate) config: RattlerBuildBackendConfig,
1920
}
@@ -52,13 +53,17 @@ impl RattlerBuildBackend {
5253
};
5354

5455
// Load the manifest from the source directory
55-
let manifest_root = manifest_path.parent().expect("manifest must have a root");
56+
let manifest_root = manifest_path
57+
.parent()
58+
.expect("manifest must have a root")
59+
.to_path_buf();
5660
let recipe_source =
57-
Source::from_rooted_path(manifest_root, recipe_path).into_diagnostic()?;
61+
Source::from_rooted_path(&manifest_root, recipe_path).into_diagnostic()?;
5862

5963
Ok(Self {
60-
recipe_source,
6164
logging_output_handler,
65+
recipe_source,
66+
manifest_root,
6267
cache_dir,
6368
config,
6469
})

0 commit comments

Comments
 (0)