feat: patchable init mirroring #1070
Conversation
|
As a nomenclature thing, I think I prefer calling this mirroring instead. |
Had the same thought when opening the PR 🙂 Will refactor it. |
| @@ -1,2 +1,2 @@ | |||
| upstream = "https://github.com/apache/druid.git" | |||
| upstream = "https://github.com/stackabletech/druid.git" | |||
There was a problem hiding this comment.
I think we still want to save the original upstream, maybe add this as a new field called mirror?
rust/patchable/src/main.rs
Outdated
| tracing::info!(commit = %base_commit_oid, "fetched base commit OID"); | ||
|
|
||
| // Add fork remote | ||
| let temp_fork_remote = "patchable_fork_push"; |
There was a problem hiding this comment.
You should be able to use remote_anonymous instead of creating and deleting a dummy remote.
| // Check if the reference is a tag or branch by inspecting the git repository | ||
| let refspec = { | ||
| let tag_ref = format!("refs/tags/{}", base); | ||
| let is_tag = product_repo |
There was a problem hiding this comment.
I half think we should just decide on tags with a common format here.
There was a problem hiding this comment.
Hmm but we can't really know if the upstream repository uses tags in a specific format (or if it uses tags at all). Zookeeper uses release-3.9.2, Trino 470, trino-storage uses v470, ...
Or do you mean we should re-tag the commit in our mirror?
rust/patchable/src/main.rs
Outdated
| // Add progress tracking for push operation | ||
| let span_push = tracing::info_span!("pushing"); | ||
| span_push.pb_set_style(&utils::progress_bar_style()); | ||
| let _ = span_push.enter(); | ||
| let mut quant_push = utils::Quantizer::percent(); | ||
| callbacks.push_transfer_progress(move |current, total, _| { | ||
| if total > 0 { | ||
| quant_push.update_span_progress(current, total, &span_push); | ||
| } | ||
| }); |
There was a problem hiding this comment.
We should probably break this stuff out into a utils module.
There was a problem hiding this comment.
Did some refactoring here
rust/patchable/src/main.rs
Outdated
| } else { | ||
| (repo::resolve_and_fetch_commitish(&product_repo, &base, &upstream).context(FetchBaseCommitSnafu)?, upstream) |
There was a problem hiding this comment.
The initial resolve_and_fetch is still the same, no? The new mirroring stuff is "just" an extra step afterwards.
There was a problem hiding this comment.
Correct. Didn't notice this after my last refactoring 🙂 It's simplified now.
| @@ -180,7 +180,9 @@ pub fn resolve_and_fetch_commitish( | |||
| &[commitish], | |||
| Some( | |||
| FetchOptions::new() | |||
| .update_fetchhead(true) | |||
There was a problem hiding this comment.
Curious about the fetchhead issues you ran into, do you have a repro for that? Does that happen with the current main, or only with (the rest of) this PR applied?
There was a problem hiding this comment.
Interestingly it works with the current main branch. As soon as the line .download_tags(git2::AutotagOption::Auto) is added, it starts breaking. Steps to reproduce:
Add the .download_tags(git2::AutotagOption::Auto) line after .update_fetchhead, then run
cargo patchable init zookeeper 3.9.2 --upstream=https://github.com/apache/zookeeper.git --base=release-3.9.2
rm zookeeper/stackable/patches/3.9.2/patchable.toml
cargo patchable init zookeeper 3.9.2 --upstream=https://github.com/apache/zookeeper.git --base=release-3.9.2
For me, the second command fails with:
Error: failed to fetch patch series' base commit
Caused by these errors (recent errors listed first):
1: failed to find commit "release-3.9.2" in "/home/voeti/docker-images/zookeeper/patchable-work/product-repo/"
2: corrupted loose reference file: FETCH_HEAD; class=Reference (4)
I'm not sure why FETCH_HEAD is empty. I'm also not sure why/if download_tags is really required to keep the tag around in patchable-work/product-repo/refs/tags. I tried a few things (disabled pruning) but I couldn't make it work, so I decided to remove the update_fetchhead and just use commitish instead of FETCH_HEAD, which circumvented the problem.
Fixes #1049
This adds the
--mirroredflag topatchable init. It assumes that a mirror repository already exist, pushes the ref specified via--baseto the mirror and configures the mirrorer repo asupstreamin patchable.toml.As discussed with @lfrancke, we want to fork the repos in the
stackabletechorg and use forks instead of clones (the latter decision was not a strong opinion, I decided to go with forks since they would facilitate back-contributions).I created mirror repositories (forks in our case) for all our products and synced the tags for all versions we use. I updated all patchable.toml files to use the mirror repositories (with the exception of Omid
1.1.3-SNAPSHOT, which is a temporary version we don't ship).I had to drop the usage of
FETCH_HEAD, since it caused problems when runningpatchable initfor a tag/branch that for some reason was already present in the local Git repo. The problem is thatFETCH_HEADis empty in that case, I think that's due to no changes being fetched.