Skip to content

Commit ec08326

Browse files
committed
fix: skip license-file validation when build.source is git or URL
When a package specifies a build source via `[package.build.source]`, the license-file and readme files should be validated against the appropriate directory: - Git or URL source: skip validation (files are in remote location) - Path source: validate against the resolved path directory - No source: validate against the manifest directory (default behavior) Previously, pixi would always validate these files against the manifest directory, causing false "file does not exist" errors when the files were expected to be in a different location. Fixes: #4985
1 parent 70d7390 commit ec08326

File tree

1 file changed

+242
-3
lines changed

1 file changed

+242
-3
lines changed

crates/pixi_manifest/src/toml/package.rs

Lines changed: 242 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,8 @@ impl TomlPackage {
364364
}
365365
}
366366

367-
// Check file existence for resolved paths with 3-tier hierarchy
367+
// Check file existence for resolved paths with 3-tier hierarchy.
368+
// If root_directory is None, validation is skipped.
368369
fn check_resolved_file(
369370
root_directory: Option<&Path>,
370371
field: Option<WorkspaceInheritableField<Spanned<PathBuf>>>,
@@ -403,14 +404,33 @@ impl TomlPackage {
403404
}
404405
}
405406

407+
// Determine the directory to use for file validation based on build.source:
408+
// - If build.source is a git or URL source, pass None to skip validation (files are remote)
409+
// - If build.source is a path source, resolve the path and validate against that directory
410+
// - If build.source is not set, validate against the manifest directory
411+
let file_validation_dir: Option<PathBuf> =
412+
match (&build_result.value.source, root_directory) {
413+
// Git or URL source: skip validation (files are in remote location)
414+
(Some(pixi_spec::SourceLocationSpec::Git(_)), _)
415+
| (Some(pixi_spec::SourceLocationSpec::Url(_)), _) => None,
416+
// Path source: resolve the path and use that directory for validation
417+
(Some(pixi_spec::SourceLocationSpec::Path(path_spec)), Some(root_dir)) => {
418+
path_spec.resolve(root_dir).ok()
419+
}
420+
// No source: use the manifest directory
421+
(None, Some(root_dir)) => Some(root_dir.to_path_buf()),
422+
// No root directory provided: skip validation
423+
(_, None) => None,
424+
};
425+
406426
let license_file = check_resolved_file(
407-
root_directory,
427+
file_validation_dir.as_deref(),
408428
self.license_file,
409429
workspace.license_file,
410430
package_defaults.license_file,
411431
)?;
412432
let readme = check_resolved_file(
413-
root_directory,
433+
file_validation_dir.as_deref(),
414434
self.readme,
415435
workspace.readme,
416436
package_defaults.readme,
@@ -882,4 +902,223 @@ mod test {
882902
assert!(!parsed_deprecated.warnings.is_empty());
883903
assert_eq!(parsed.value.build, parsed_deprecated.value.build);
884904
}
905+
906+
#[test]
907+
fn test_license_file_validation_skipped_for_git_source() {
908+
// When build.source is a git source, license-file validation should be skipped
909+
// because the file will be in the checked-out source directory, not the manifest directory.
910+
let input = r#"
911+
name = "bla"
912+
version = "1.0"
913+
license-file = "LICENSE.txt"
914+
915+
[build]
916+
backend = { name = "bla", version = "1.0" }
917+
source = { git = "https://github.com/example/repo", rev = "abc123" }
918+
"#;
919+
let path = Path::new("");
920+
// This should NOT fail even though LICENSE.txt doesn't exist,
921+
// because the source is a git repository.
922+
let result = TomlPackage::from_toml_str(input).and_then(|w| {
923+
w.into_manifest(
924+
WorkspacePackageProperties::default(),
925+
PackageDefaults::default(),
926+
&Preview::default(),
927+
Some(path),
928+
)
929+
});
930+
assert!(result.is_ok(), "Expected success but got: {:?}", result);
931+
}
932+
933+
#[test]
934+
fn test_license_file_validation_skipped_for_url_source() {
935+
// When build.source is a URL source, license-file validation should be skipped
936+
// because the file will be in the downloaded/extracted source directory.
937+
let input = r#"
938+
name = "bla"
939+
version = "1.0"
940+
license-file = "LICENSE.txt"
941+
942+
[build]
943+
backend = { name = "bla", version = "1.0" }
944+
source = { url = "https://example.com/archive.tar.gz" }
945+
"#;
946+
let path = Path::new("");
947+
// This should NOT fail even though LICENSE.txt doesn't exist,
948+
// because the source is a URL.
949+
let result = TomlPackage::from_toml_str(input).and_then(|w| {
950+
w.into_manifest(
951+
WorkspacePackageProperties::default(),
952+
PackageDefaults::default(),
953+
&Preview::default(),
954+
Some(path),
955+
)
956+
});
957+
assert!(result.is_ok(), "Expected success but got: {:?}", result);
958+
}
959+
960+
#[test]
961+
fn test_readme_validation_skipped_for_git_source() {
962+
// When build.source is a git source, readme validation should be skipped
963+
let input = r#"
964+
name = "bla"
965+
version = "1.0"
966+
readme = "README.md"
967+
968+
[build]
969+
backend = { name = "bla", version = "1.0" }
970+
source = { git = "https://github.com/example/repo", branch = "main" }
971+
"#;
972+
let path = Path::new("");
973+
// This should NOT fail even though README.md doesn't exist
974+
let result = TomlPackage::from_toml_str(input).and_then(|w| {
975+
w.into_manifest(
976+
WorkspacePackageProperties::default(),
977+
PackageDefaults::default(),
978+
&Preview::default(),
979+
Some(path),
980+
)
981+
});
982+
assert!(result.is_ok(), "Expected success but got: {:?}", result);
983+
}
984+
985+
#[test]
986+
fn test_license_file_validation_fails_for_path_source_missing_file() {
987+
// When build.source is a path source, license-file validation should still run
988+
// and fail if the file doesn't exist in the source directory
989+
let input = r#"
990+
name = "bla"
991+
version = "1.0"
992+
license-file = "LICENSE.txt"
993+
994+
[build]
995+
backend = { name = "bla", version = "1.0" }
996+
source = { path = "../some/path" }
997+
"#;
998+
let path = Path::new("");
999+
// This should fail because LICENSE.txt doesn't exist and source is a path
1000+
let result = TomlPackage::from_toml_str(input).and_then(|w| {
1001+
w.into_manifest(
1002+
WorkspacePackageProperties::default(),
1003+
PackageDefaults::default(),
1004+
&Preview::default(),
1005+
Some(path),
1006+
)
1007+
});
1008+
assert!(result.is_err(), "Expected failure for path source");
1009+
}
1010+
1011+
#[test]
1012+
fn test_license_file_validation_succeeds_without_build_source() {
1013+
// When no build.source is specified, license-file should be validated
1014+
// against the manifest directory
1015+
use std::fs;
1016+
use tempfile::TempDir;
1017+
1018+
let temp_dir = TempDir::new().unwrap();
1019+
let license_path = temp_dir.path().join("LICENSE.txt");
1020+
fs::write(&license_path, "MIT License").unwrap();
1021+
1022+
let input = r#"
1023+
name = "bla"
1024+
version = "1.0"
1025+
license-file = "LICENSE.txt"
1026+
1027+
[build]
1028+
backend = { name = "bla", version = "1.0" }
1029+
"#;
1030+
1031+
let result = TomlPackage::from_toml_str(input).and_then(|w| {
1032+
w.into_manifest(
1033+
WorkspacePackageProperties::default(),
1034+
PackageDefaults::default(),
1035+
&Preview::default(),
1036+
Some(temp_dir.path()),
1037+
)
1038+
});
1039+
assert!(result.is_ok(), "Expected success but got: {:?}", result);
1040+
1041+
// Verify the license_file path is set correctly
1042+
let manifest = result.unwrap().value;
1043+
assert!(manifest.package.license_file.is_some());
1044+
}
1045+
1046+
#[test]
1047+
fn test_license_file_validation_succeeds_with_path_source() {
1048+
// When build.source is a path, license-file should be validated
1049+
// against the resolved path source directory
1050+
use std::fs;
1051+
use tempfile::TempDir;
1052+
1053+
// Create manifest directory
1054+
let manifest_dir = TempDir::new().unwrap();
1055+
1056+
// Create source directory with license file
1057+
let source_dir = TempDir::new().unwrap();
1058+
let license_path = source_dir.path().join("LICENSE.txt");
1059+
fs::write(&license_path, "MIT License").unwrap();
1060+
1061+
// Use the source directory path in the manifest
1062+
let source_path = source_dir.path().to_string_lossy();
1063+
let input = format!(
1064+
r#"
1065+
name = "bla"
1066+
version = "1.0"
1067+
license-file = "LICENSE.txt"
1068+
1069+
[build]
1070+
backend = {{ name = "bla", version = "1.0" }}
1071+
source = {{ path = "{source_path}" }}
1072+
"#
1073+
);
1074+
1075+
let result = TomlPackage::from_toml_str(&input).and_then(|w| {
1076+
w.into_manifest(
1077+
WorkspacePackageProperties::default(),
1078+
PackageDefaults::default(),
1079+
&Preview::default(),
1080+
Some(manifest_dir.path()),
1081+
)
1082+
});
1083+
assert!(result.is_ok(), "Expected success but got: {:?}", result);
1084+
1085+
// Verify the license_file path is set correctly
1086+
let manifest = result.unwrap().value;
1087+
assert!(manifest.package.license_file.is_some());
1088+
}
1089+
1090+
#[test]
1091+
fn test_readme_validation_succeeds_without_build_source() {
1092+
// When no build.source is specified, readme should be validated
1093+
// against the manifest directory
1094+
use std::fs;
1095+
use tempfile::TempDir;
1096+
1097+
let temp_dir = TempDir::new().unwrap();
1098+
let readme_path = temp_dir.path().join("README.md");
1099+
fs::write(&readme_path, "# My Package").unwrap();
1100+
1101+
let input = r#"
1102+
name = "bla"
1103+
version = "1.0"
1104+
readme = "README.md"
1105+
1106+
[build]
1107+
backend = { name = "bla", version = "1.0" }
1108+
"#;
1109+
1110+
let result = TomlPackage::from_toml_str(input).and_then(|w| {
1111+
w.into_manifest(
1112+
WorkspacePackageProperties::default(),
1113+
PackageDefaults::default(),
1114+
&Preview::default(),
1115+
Some(temp_dir.path()),
1116+
)
1117+
});
1118+
assert!(result.is_ok(), "Expected success but got: {:?}", result);
1119+
1120+
// Verify the readme path is set correctly
1121+
let manifest = result.unwrap().value;
1122+
assert!(manifest.package.readme.is_some());
1123+
}
8851124
}

0 commit comments

Comments
 (0)