put: run the section pre-flight against the uploaded tree
The Section check always read debian/control from the current working directory, so 'pkh put --changes ../other/pkg_changes' validated the wrong tree. With an explicit --changes the check now runs against that file's own directory when it holds debian/control, and is skipped with a warning otherwise; tree uploads are unchanged.
This commit is contained in:
+112
-1
@@ -99,7 +99,15 @@ pub async fn put(
|
|||||||
// Pre-flight checks for everything the upload queue only rejects after
|
// Pre-flight checks for everything the upload queue only rejects after
|
||||||
// processing: a valid Section, a known target series, and the target
|
// processing: a valid Section, a known target series, and the target
|
||||||
// PPA actually existing (the SFTP queue itself is a blind write)
|
// PPA actually existing (the SFTP queue itself is a blind write)
|
||||||
check_control_section(&opts.cwd, "ubuntu")?;
|
match section_check_target(opts.changes.as_deref(), &opts.cwd) {
|
||||||
|
SectionCheckTarget::Dir(dir) => check_control_section(&dir, "ubuntu")?,
|
||||||
|
SectionCheckTarget::Skip => log::warn!(
|
||||||
|
"cannot check the Section: '{}' has no debian/control next to \
|
||||||
|
it, skipping the pre-flight (the archive still rejects uploads \
|
||||||
|
with unknown sections)",
|
||||||
|
changes_path.display()
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
let checking = multi.add(ProgressBar::new(0));
|
let checking = multi.add(ProgressBar::new(0));
|
||||||
checking.set_style(ui::spinner_style());
|
checking.set_style(ui::spinner_style());
|
||||||
@@ -323,6 +331,40 @@ fn check_control_section(cwd: &Path, dist: &str) -> Result<(), Box<dyn std::erro
|
|||||||
.into())
|
.into())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// What the Section pre-flight decided to validate: a directory holding a
|
||||||
|
/// `debian/control` ([`check_control_section`]'s expected layout), or that
|
||||||
|
/// it cannot run where the upload lives and must be skipped.
|
||||||
|
#[derive(Debug, PartialEq, Eq)]
|
||||||
|
enum SectionCheckTarget {
|
||||||
|
/// Validate `debian/control` in this directory
|
||||||
|
Dir(PathBuf),
|
||||||
|
/// No `debian/control` next to the `.changes`: skip the check
|
||||||
|
Skip,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Decide which tree the Section pre-flight validates. An explicit
|
||||||
|
/// `--changes` file must be followed by its own directory — it can name a
|
||||||
|
/// different package than the current tree — so the check runs against the
|
||||||
|
/// `.changes`' directory when it directly holds a `debian/control` (the
|
||||||
|
/// `.changes` inside the package root), and is skipped otherwise: pkh
|
||||||
|
/// writes the `.changes` next to the source tree (an artifacts-only
|
||||||
|
/// directory as far as `debian/control` goes), and checking `cwd` there
|
||||||
|
/// would validate whatever package the user happens to be in. Without
|
||||||
|
/// `--changes` the upload comes from the current tree, which keeps getting
|
||||||
|
/// checked exactly as before. Pure decision, factored out so the
|
||||||
|
/// wrong-tree rule is testable with plain tempdirs.
|
||||||
|
fn section_check_target(changes_path: Option<&Path>, cwd: &Path) -> SectionCheckTarget {
|
||||||
|
let Some(changes) = changes_path else {
|
||||||
|
return SectionCheckTarget::Dir(cwd.to_path_buf());
|
||||||
|
};
|
||||||
|
let dir = changes.parent().unwrap_or_else(|| Path::new("."));
|
||||||
|
if dir.join("debian/control").is_file() {
|
||||||
|
SectionCheckTarget::Dir(dir.to_path_buf())
|
||||||
|
} else {
|
||||||
|
SectionCheckTarget::Skip
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// The highest version among `published` that parses as a Debian version,
|
/// The highest version among `published` that parses as a Debian version,
|
||||||
/// `None` when the list is empty or nothing in it parses (entries that
|
/// `None` when the list is empty or nothing in it parses (entries that
|
||||||
/// cannot be parsed are skipped rather than failing the check: the API
|
/// cannot be parsed are skipped rather than failing the check: the API
|
||||||
@@ -699,6 +741,75 @@ mod tests {
|
|||||||
assert!(err.contains("has no Section"), "unexpected: {err}");
|
assert!(err.contains("has no Section"), "unexpected: {err}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// An explicit `--changes` file of a different package must be checked
|
||||||
|
/// in its own directory (A), not in the current tree (B): B's control
|
||||||
|
/// file decided the pre-flight before the fix
|
||||||
|
#[test]
|
||||||
|
fn section_check_follows_explicit_changes_directory() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let artifacts = dir.path().join("artifacts");
|
||||||
|
let elsewhere = dir.path().join("elsewhere");
|
||||||
|
std::fs::create_dir_all(artifacts.join("debian")).unwrap();
|
||||||
|
std::fs::create_dir_all(elsewhere.join("debian")).unwrap();
|
||||||
|
let changes = artifacts.join("hello_1.0-1_source.changes");
|
||||||
|
std::fs::write(&changes, b"changes").unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
artifacts.join("debian/control"),
|
||||||
|
"Source: hello\nSection: utils\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
elsewhere.join("debian/control"),
|
||||||
|
"Source: other\nSection: unknown\n",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
section_check_target(Some(&changes), &elsewhere),
|
||||||
|
SectionCheckTarget::Dir(artifacts.clone())
|
||||||
|
);
|
||||||
|
|
||||||
|
// The decided directory carries the valid section, while the tree
|
||||||
|
// the old code checked would have failed the upload
|
||||||
|
match section_check_target(Some(&changes), &elsewhere) {
|
||||||
|
SectionCheckTarget::Dir(d) => check_control_section(&d, "ubuntu").unwrap(),
|
||||||
|
SectionCheckTarget::Skip => panic!("the .changes directory has debian/control"),
|
||||||
|
}
|
||||||
|
assert!(check_control_section(&elsewhere, "ubuntu").is_err());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A `.changes` in a directory without `debian/control` — pkh's own
|
||||||
|
/// layout puts it next to the source tree, and artifacts can be
|
||||||
|
/// collected away from any source — skips the check instead of
|
||||||
|
/// validating whatever tree `cwd` points at
|
||||||
|
#[test]
|
||||||
|
fn section_check_skips_when_changes_dir_has_no_control() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let artifacts = dir.path().join("artifacts");
|
||||||
|
std::fs::create_dir_all(&artifacts).unwrap();
|
||||||
|
let changes = artifacts.join("hello_1.0-1_source.changes");
|
||||||
|
std::fs::write(&changes, b"changes").unwrap();
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
section_check_target(Some(&changes), dir.path()),
|
||||||
|
SectionCheckTarget::Skip
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Without `--changes` the upload comes from the current tree: the
|
||||||
|
/// decision stays the cwd, byte-identical to the pre-fix behavior
|
||||||
|
#[test]
|
||||||
|
fn section_check_without_changes_validates_cwd() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let pkg = dir.path().join("hello");
|
||||||
|
std::fs::create_dir_all(pkg.join("debian")).unwrap();
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
section_check_target(None, &pkg),
|
||||||
|
SectionCheckTarget::Dir(pkg)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn discover_errors_when_several_candidates() {
|
fn discover_errors_when_several_candidates() {
|
||||||
let dir = tempfile::tempdir().unwrap();
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
|||||||
Reference in New Issue
Block a user