Skip to content

Commit 0759727

Browse files
committed
cargo-rail: fixing PR branch creation on split repo -> monorepo sync. TOML fmt is deeply broken across the codebase at this point
1 parent 47328ad commit 0759727

File tree

2 files changed

+107
-4
lines changed

2 files changed

+107
-4
lines changed

src/sync/engine.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,26 @@ impl<'a> SyncEngine<'a> {
258258
remote_git.get_commits_touching_path(Path::new("."), None, &branch_ref)?
259259
};
260260

261+
// Always create a PR branch for safety when syncing from remote
262+
// This prevents direct commits to protected branches (main, master, develop)
263+
let pr_branch = if !new_commits.is_empty() {
264+
// Create timestamped branch name
265+
let timestamp = std::time::SystemTime::now()
266+
.duration_since(std::time::UNIX_EPOCH)
267+
.unwrap_or_else(|_| std::time::Duration::from_secs(0))
268+
.as_secs();
269+
270+
// Format: cargo-rail-sync-CRATE-TIMESTAMP
271+
let branch_name = format!("cargo-rail-sync-{}-{}", self.config.crate_name, timestamp);
272+
273+
println!(" Creating PR branch: {}", branch_name);
274+
self.ctx.git.git().create_and_checkout_branch(&branch_name)?;
275+
276+
Some(branch_name)
277+
} else {
278+
None
279+
};
280+
261281
let mut conflicts = Vec::new();
262282

263283
let synced_count = if new_commits.is_empty() {
@@ -308,7 +328,36 @@ impl<'a> SyncEngine<'a> {
308328
// Save mappings
309329
self.mapping_store.save(self.ctx.workspace_root())?;
310330

311-
// If we created a PR branch, push it to remote and remind user to create PR
331+
// Print PR creation instructions if we created a branch with synced commits
332+
if let Some(branch_name) = pr_branch
333+
&& synced_count > 0
334+
{
335+
println!(
336+
"\n✅ Synced {} commit{} to branch: {}",
337+
synced_count,
338+
if synced_count == 1 { "" } else { "s" },
339+
branch_name
340+
);
341+
println!("\n📋 To create a pull request:");
342+
println!(" git push origin {}", branch_name);
343+
344+
// Try to detect GitHub URL and suggest gh CLI command
345+
if let Ok(output) = std::process::Command::new("git")
346+
.args(["config", "--get", "remote.origin.url"])
347+
.current_dir(self.ctx.workspace_root())
348+
.output()
349+
&& let Ok(url) = String::from_utf8(output.stdout)
350+
{
351+
let url = url.trim();
352+
if url.contains("github.com") {
353+
println!(
354+
" gh pr create --title \"Sync {} from remote\"",
355+
self.config.crate_name
356+
);
357+
}
358+
}
359+
println!();
360+
}
312361

313362
Ok(SyncResult {
314363
commits_synced: synced_count,

tests/integration/test_sync.rs

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ fn test_sync_from_remote_basic() -> Result<()> {
7272
// Sync from remote
7373
run_cargo_rail(&ws.path, &["rail", "sync", "mylib", "--from-remote"])?;
7474

75-
// Verify change in monorepo
75+
// Verify change in monorepo (on PR branch, not original branch)
7676
let mono_content = std::fs::read_to_string(ws.path.join("crates/mylib/src/lib.rs"))?;
7777
assert!(
7878
mono_content.contains("// Changed in split"),
@@ -87,6 +87,60 @@ fn test_sync_from_remote_basic() -> Result<()> {
8787
Ok(())
8888
}
8989

90+
#[test]
91+
fn test_sync_from_remote_creates_pr_branch() -> Result<()> {
92+
let (ws, split_dir) = setup_split_scenario("mylib")?;
93+
94+
// Get original branch name
95+
let original_branch_output = git(&ws.path, &["rev-parse", "--abbrev-ref", "HEAD"])?;
96+
let original_branch = String::from_utf8_lossy(&original_branch_output.stdout)
97+
.trim()
98+
.to_string();
99+
100+
// Make change in split repo
101+
std::fs::write(split_dir.path().join("src/lib.rs"), "// Changed in split")?;
102+
git(split_dir.path(), &["add", "."])?;
103+
git(split_dir.path(), &["commit", "-m", "Test change in split"])?;
104+
105+
// Sync from remote - should create PR branch
106+
run_cargo_rail(&ws.path, &["rail", "sync", "mylib", "--from-remote"])?;
107+
108+
// Verify we're on a PR branch (not the original branch)
109+
let current_branch_output = git(&ws.path, &["rev-parse", "--abbrev-ref", "HEAD"])?;
110+
let current_branch = String::from_utf8_lossy(&current_branch_output.stdout)
111+
.trim()
112+
.to_string();
113+
114+
assert!(
115+
current_branch.starts_with("cargo-rail-sync-mylib-"),
116+
"Should be on a PR branch starting with cargo-rail-sync-mylib-, but got: {}",
117+
current_branch
118+
);
119+
assert_ne!(current_branch, original_branch, "Should not be on the original branch");
120+
121+
// Verify the change is on the PR branch
122+
let mono_content = std::fs::read_to_string(ws.path.join("crates/mylib/src/lib.rs"))?;
123+
assert!(
124+
mono_content.contains("// Changed in split"),
125+
"Change should be on PR branch"
126+
);
127+
128+
// Verify commit has Rail-Origin trailer
129+
let log_output = git(&ws.path, &["log", "-1", "--format=%B"])?;
130+
let log = String::from_utf8_lossy(&log_output.stdout);
131+
assert!(log.contains("Rail-Origin: remote@"), "Should have Rail-Origin trailer");
132+
133+
// Switch back to original branch and verify it's unchanged
134+
git(&ws.path, &["checkout", &original_branch])?;
135+
let original_content = std::fs::read_to_string(ws.path.join("crates/mylib/src/lib.rs"))?;
136+
assert!(
137+
!original_content.contains("// Changed in split"),
138+
"Original branch should not have the change"
139+
);
140+
141+
Ok(())
142+
}
143+
90144
#[test]
91145
fn test_sync_roundtrip_preserves_content() -> Result<()> {
92146
let (ws, split_dir) = setup_split_scenario("mylib")?;
@@ -104,10 +158,10 @@ fn test_sync_roundtrip_preserves_content() -> Result<()> {
104158
let split_content = std::fs::read_to_string(split_dir.path().join("src/lib.rs"))?;
105159
assert_eq!(split_content, original, "Split should have original content");
106160

107-
// Sync back from split (should be no-op)
161+
// Sync back from split (should be no-op, but creates PR branch)
108162
run_cargo_rail(&ws.path, &["rail", "sync", "mylib", "--from-remote"])?;
109163

110-
// Verify still matches
164+
// Verify still matches (on PR branch)
111165
let final_content = std::fs::read_to_string(ws.path.join("crates/mylib/src/lib.rs"))?;
112166
assert_eq!(final_content, original, "Content should be preserved after roundtrip");
113167

0 commit comments

Comments
 (0)