Skip to content

Commit 100d659

Browse files
authored
fix: build release packages once (#8192)
1 parent c6dd3d7 commit 100d659

5 files changed

Lines changed: 196 additions & 472 deletions

File tree

src/core/release/checkout_guard.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ mod tests {
198198
}
199199

200200
#[test]
201-
fn restore_after_failure_returns_to_original_branch_and_removes_new_untracked() {
201+
fn package_failure_restores_release_commit_and_generated_files() {
202202
let temp = init_repo();
203203
let dir = temp.path();
204204
run_git(dir, &["checkout", "-q", "-b", "release-local"]);
@@ -207,9 +207,10 @@ mod tests {
207207
.expect("capture")
208208
.expect("git repo");
209209

210-
run_git(dir, &["checkout", "-q", "main"]);
211210
std::fs::write(dir.join("generated.txt"), "release output\n").expect("write generated");
212211
std::fs::write(dir.join("file.txt"), "mutated\n").expect("mutate tracked");
212+
run_git(dir, &["add", "file.txt"]);
213+
run_git(dir, &["commit", "-q", "-m", "release: v1.0.0"]);
213214

214215
guard.restore_after_failure().expect("restore");
215216

@@ -223,6 +224,10 @@ mod tests {
223224
);
224225
assert_eq!(git_stdout_for_test(dir, &["status", "--porcelain=v1"]), "");
225226
assert!(!dir.join("generated.txt").exists());
227+
assert_eq!(
228+
std::fs::read_to_string(dir.join("file.txt")).unwrap(),
229+
"main\n"
230+
);
226231
}
227232

228233
#[test]

src/core/release/execution_dispatch.rs

Lines changed: 151 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,19 @@ pub(super) fn execute_release_plan_step(
4040
Ok(Some(run_changelog_bootstrap_preflight(step, context)))
4141
}
4242
"preflight.package" => Ok(Some(
43-
executor::package_preflight::run_package_preflight(
44-
context.extensions,
45-
context.component,
46-
context.component_id,
47-
&context.component.local_path,
48-
context.options.skip_build_validation,
49-
)
50-
.unwrap_or_else(|err| failed_result("preflight.package", "preflight.package", err)),
43+
executor::package_preflight::run_package_preflight(&context.component.local_path)
44+
.map(|_| {
45+
executor::step_success(
46+
"preflight.package",
47+
"preflight.package",
48+
Some(serde_json::json!({
49+
"component_path": context.component.local_path,
50+
"validated_action": "release.package.guards",
51+
})),
52+
Vec::new(),
53+
)
54+
})
55+
.unwrap_or_else(|err| failed_result("preflight.package", "preflight.package", err)),
5156
)),
5257
"preflight.tag_availability" => {
5358
let tag_name = step
@@ -109,6 +114,14 @@ pub(super) fn execute_release_plan_step(
109114
None,
110115
context.options.skip_build_validation,
111116
)
117+
.and_then(|result| {
118+
executor::package_preflight::validate_package_completeness(
119+
context.component,
120+
std::path::Path::new(&context.component.local_path),
121+
&context.state.artifacts,
122+
)?;
123+
Ok(result)
124+
})
112125
.unwrap_or_else(|err| failed_result("package", "package", err)),
113126
)),
114127
"artifacts.inventory" => {
@@ -729,6 +742,7 @@ mod tests {
729742
release_step_is_show_stopper, release_step_unexpected_dirty_files, ReleaseExecutionContext,
730743
};
731744
use crate::core::component::{Component, ComponentScriptsConfig, VersionTarget};
745+
use crate::core::extension::ExtensionManifest;
732746
use crate::core::plan::PlanStep;
733747
use crate::core::release::types::{
734748
ReleaseOptions, ReleaseState, ReleaseStepResult, ReleaseStepStatus,
@@ -774,6 +788,135 @@ mod tests {
774788
assert!(!release_step_is_plan_only(&plan_step("deploy")));
775789
}
776790

791+
#[test]
792+
fn release_package_builds_once_and_validates_the_uploaded_durable_bytes() {
793+
crate::test_support::with_isolated_home(|_| {
794+
let repo = tempfile::tempdir().expect("repo");
795+
std::fs::write(repo.path().join("plugin.php"), "<?php\n").expect("plugin");
796+
run_in(repo.path(), &["git", "init", "-q"]);
797+
configure_git_user(repo.path());
798+
run_in(repo.path(), &["git", "add", "plugin.php"]);
799+
run_in(repo.path(), &["git", "commit", "-qm", "Initial commit"]);
800+
let counter = repo.path().join("package-count");
801+
let package = package_extension(&format!(
802+
"n=$(cat '{counter}' 2>/dev/null || echo 0); echo $((n + 1)) > '{counter}'; \
803+
mkdir -p build/stage; cp plugin.php build/stage/plugin.php; \
804+
(cd build && zip -q fixture.zip stage/plugin.php); \
805+
printf '[{{\"path\":\"build/fixture.zip\",\"type\":\"archive\"}}]'",
806+
counter = counter.display(),
807+
));
808+
crate::core::extension::save_manifest(&package).expect("save package extension");
809+
810+
let component = Component {
811+
id: "fixture".to_string(),
812+
local_path: repo.path().to_string_lossy().to_string(),
813+
..Component::default()
814+
};
815+
let options = ReleaseOptions::default();
816+
let extensions = vec![package];
817+
let mut context = ReleaseExecutionContext {
818+
component: &component,
819+
extensions: &extensions,
820+
component_id: "fixture",
821+
options: &options,
822+
state: ReleaseState {
823+
version: Some("1.2.3".to_string()),
824+
..ReleaseState::default()
825+
},
826+
publish_failed: false,
827+
};
828+
829+
execute_release_plan_step(&plan_step("preflight.package"), &mut context)
830+
.expect("preflight dispatch")
831+
.expect("preflight result");
832+
assert!(!counter.exists(), "preflight must not run release.package");
833+
834+
let result = execute_release_plan_step(&plan_step("package"), &mut context)
835+
.expect("package dispatch")
836+
.expect("package result");
837+
assert_eq!(result.status, ReleaseStepStatus::Success);
838+
assert_eq!(
839+
std::fs::read_to_string(&counter).expect("count").trim(),
840+
"1"
841+
);
842+
let durable = context.state.artifacts[0]
843+
.durable_path
844+
.as_ref()
845+
.expect("durable artifact");
846+
assert_eq!(
847+
std::fs::read(repo.path().join("build/fixture.zip")).expect("source artifact"),
848+
std::fs::read(durable).expect("uploaded artifact"),
849+
);
850+
});
851+
}
852+
853+
#[test]
854+
fn final_package_completeness_failure_stops_before_publication() {
855+
crate::test_support::with_isolated_home(|_| {
856+
let repo = tempfile::tempdir().expect("repo");
857+
std::fs::create_dir_all(repo.path().join("agents")).expect("agents");
858+
std::fs::write(repo.path().join("plugin.php"), "<?php\n").expect("plugin");
859+
std::fs::write(repo.path().join("agents/runtime.php"), "<?php\n").expect("runtime");
860+
run_in(repo.path(), &["git", "init", "-q"]);
861+
configure_git_user(repo.path());
862+
run_in(
863+
repo.path(),
864+
&["git", "add", "plugin.php", "agents/runtime.php"],
865+
);
866+
run_in(repo.path(), &["git", "commit", "-qm", "Initial commit"]);
867+
let package = package_extension(
868+
"mkdir -p build/stage; cp plugin.php build/stage/plugin.php; (cd build && zip -q fixture.zip stage/plugin.php); printf '[{\"path\":\"build/fixture.zip\",\"type\":\"archive\"}]'",
869+
);
870+
crate::core::extension::save_manifest(&package).expect("save package extension");
871+
872+
let component = Component {
873+
id: "fixture".to_string(),
874+
local_path: repo.path().to_string_lossy().to_string(),
875+
..Component::default()
876+
};
877+
let options = ReleaseOptions::default();
878+
let extensions = vec![package];
879+
let mut context = ReleaseExecutionContext {
880+
component: &component,
881+
extensions: &extensions,
882+
component_id: "fixture",
883+
options: &options,
884+
state: ReleaseState {
885+
version: Some("1.2.3".to_string()),
886+
..ReleaseState::default()
887+
},
888+
publish_failed: false,
889+
};
890+
891+
let result = execute_release_plan_step(&plan_step("package"), &mut context)
892+
.expect("package dispatch")
893+
.expect("package result");
894+
assert_eq!(result.status, ReleaseStepStatus::Failed);
895+
assert!(result
896+
.error
897+
.as_deref()
898+
.unwrap_or_default()
899+
.contains("agents/runtime.php"));
900+
assert!(release_step_is_show_stopper(&result));
901+
});
902+
}
903+
904+
fn package_extension(command: &str) -> ExtensionManifest {
905+
let mut extension: ExtensionManifest = serde_json::from_value(serde_json::json!({
906+
"name": "Fixture Packager",
907+
"version": "1.0.0",
908+
"actions": [{
909+
"id": "release.package",
910+
"label": "Package release",
911+
"type": "command",
912+
"command": command,
913+
}],
914+
}))
915+
.expect("package extension");
916+
extension.id = "fixture-packager".to_string();
917+
extension
918+
}
919+
777920
#[test]
778921
fn dependency_preflight_hydrates_before_lint_self_check() {
779922
let temp = tempfile::tempdir().expect("tempdir");

src/core/release/executor.rs

Lines changed: 18 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,7 @@ mod tests {
907907
}
908908

909909
#[test]
910-
fn package_preflight_payload_distinguishes_staging_path_from_source_path() {
910+
fn package_preflight_runs_guards_without_building_an_artifact() {
911911
crate::test_support::with_isolated_home(|_| {
912912
let component = tempfile::tempdir().expect("component tempdir");
913913
run_in(component.path(), &["git", "init"]);
@@ -931,45 +931,17 @@ mod tests {
931931
);
932932
crate::core::extension::save_manifest(&package).expect("save package extension");
933933

934-
let result = package_preflight::run_package_preflight(
935-
&[package],
936-
&Component {
937-
id: "fixture".to_string(),
938-
local_path: component.path().to_string_lossy().to_string(),
939-
..Component::default()
940-
},
941-
"fixture",
942-
&component.path().to_string_lossy(),
943-
false,
944-
)
945-
.expect("package preflight");
934+
package_preflight::run_package_preflight(&component.path().to_string_lossy())
935+
.expect("package guards");
946936

947-
assert_eq!(result.status, ReleaseStepStatus::Success);
948-
let payload: serde_json::Value = serde_json::from_str(
949-
&std::fs::read_to_string(&payload_out).expect("payload output"),
950-
)
951-
.expect("payload json");
952-
assert_eq!(
953-
payload["release"]["source_path"].as_str(),
954-
Some(component.path().to_string_lossy().as_ref())
955-
);
956-
assert_ne!(
957-
payload["release"]["local_path"].as_str(),
958-
payload["release"]["source_path"].as_str()
959-
);
960-
assert_eq!(
961-
std::fs::read_to_string(&source_env_out).expect("source env"),
962-
component.path().to_string_lossy()
963-
);
964-
assert_eq!(
965-
std::fs::read_to_string(&component_env_out).expect("component env"),
966-
payload["release"]["local_path"].as_str().unwrap()
967-
);
937+
assert!(!payload_out.exists());
938+
assert!(!source_env_out.exists());
939+
assert!(!component_env_out.exists());
968940
});
969941
}
970942

971943
#[test]
972-
fn package_preflight_materializes_repo_root_for_subdirectory_component() {
944+
fn package_preflight_does_not_materialize_a_repository_copy() {
973945
crate::test_support::with_isolated_home(|_| {
974946
let repo = tempfile::tempdir().expect("repo tempdir");
975947
std::fs::write(repo.path().join("build-input.txt"), "repo-root-input")
@@ -998,27 +970,14 @@ mod tests {
998970
);
999971
crate::core::extension::save_manifest(&package).expect("save package extension");
1000972

1001-
let result = package_preflight::run_package_preflight(
1002-
&[package],
1003-
&Component {
1004-
id: "fixture".to_string(),
1005-
local_path: component.to_string_lossy().to_string(),
1006-
..Component::default()
1007-
},
1008-
"fixture",
1009-
&component.to_string_lossy(),
1010-
false,
1011-
)
1012-
.expect("package preflight");
1013-
1014-
assert_eq!(result.status, ReleaseStepStatus::Success);
1015-
let data = result.data.expect("preflight data");
1016-
assert_eq!(data["validated_action"].as_str(), Some("release.package"));
973+
package_preflight::run_package_preflight(&component.to_string_lossy())
974+
.expect("package guards");
975+
assert!(!component.join("build").exists());
1017976
});
1018977
}
1019978

1020979
#[test]
1021-
fn package_preflight_failure_includes_actionable_diagnostics() {
980+
fn package_failure_surfaces_actionable_output() {
1022981
crate::test_support::with_isolated_home(|_| {
1023982
let component = tempfile::tempdir().expect("component tempdir");
1024983
let package = release_package_extension(
@@ -1027,59 +986,19 @@ mod tests {
1027986
);
1028987
crate::core::extension::save_manifest(&package).expect("save package extension");
1029988

1030-
let err = package_preflight::run_package_preflight(
989+
let mut state = ReleaseState::default();
990+
let err = run_package(
1031991
&[package],
1032-
&Component {
1033-
id: "fixture".to_string(),
1034-
local_path: component.path().to_string_lossy().to_string(),
1035-
..Component::default()
1036-
},
992+
&mut state,
1037993
"fixture",
1038994
&component.path().to_string_lossy(),
995+
None,
1039996
true,
1040997
)
1041-
.expect_err("failing package preflight should surface diagnostics");
998+
.expect_err("failing final package should surface output");
1042999

1043-
let diagnostic = &err.details["diagnostic"];
1044-
assert_eq!(diagnostic["component_id"].as_str(), Some("fixture"));
1045-
assert_eq!(
1046-
diagnostic["package_root"].as_str(),
1047-
Some(component.path().to_string_lossy().as_ref())
1048-
);
1049-
assert!(diagnostic["build_cwd"]
1050-
.as_str()
1051-
.unwrap_or_default()
1052-
.contains("homeboy-release-package-preflight"));
1053-
assert!(diagnostic["materialized_temp_root"]
1054-
.as_str()
1055-
.unwrap_or_default()
1056-
.contains("homeboy-release-package-preflight"));
1057-
assert_eq!(diagnostic["exit_code"].as_i64(), Some(9));
1058-
let command = diagnostic["command"].as_str().expect("command");
1059-
assert!(command.contains("building package"));
1060-
assert!(command.contains("missing tsconfig.base.json"));
1061-
assert!(command.contains("exit 9"));
1062-
assert_eq!(
1063-
diagnostic["config_fields"]["component.local_path"].as_str(),
1064-
Some(component.path().to_string_lossy().as_ref())
1065-
);
1066-
assert_eq!(
1067-
diagnostic["config_fields"]["config.skip_build_validation"].as_bool(),
1068-
Some(true)
1069-
);
1070-
assert!(diagnostic["relevant_error_lines"]
1071-
.as_array()
1072-
.expect("relevant error lines")
1073-
.iter()
1074-
.any(|line| line.as_str() == Some("error: missing tsconfig.base.json")));
1075-
1076-
let artifact_path = err.details["artifact_path"]
1077-
.as_str()
1078-
.expect("artifact path");
1079-
assert!(std::path::Path::new(artifact_path).is_file());
1080-
let artifact = std::fs::read_to_string(artifact_path).expect("diagnostic artifact");
1081-
assert!(artifact.contains("missing tsconfig.base.json"));
1082-
assert!(err.message.contains("diagnostic artifact"));
1000+
assert!(err.message.contains("missing tsconfig.base.json"));
1001+
assert_eq!(err.details["source"]["exit_code"].as_i64(), Some(9));
10831002
});
10841003
}
10851004

0 commit comments

Comments
 (0)