Skip to content

Implement stacked changes support #699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LMG merged 1 commit into from
Apr 19, 2022
Merged

Implement stacked changes support #699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions josh-proxy/src/bin/josh-proxy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ async fn call_service(
base_ns: josh::to_ns(&parsed_url.upstream_repo),
git_ns: temp_ns.name().to_string(),
git_dir: repo_path.to_string(),
stacked_changes: ARGS.is_present("stacked-changes"),
Copy link
Member Author

@christian-schilling christian-schilling Apr 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure on how to name this feature 🤔

};

let mut cmd = Command::new("git");
Expand Down Expand Up @@ -850,6 +851,11 @@ fn make_app() -> clap::App<'static> {
.long("no-background")
.takes_value(false),
)
.arg(
clap::Arg::new("stacked-changes")
.long("stacked-changes")
.takes_value(false),
)
.arg(
clap::Arg::new("graphql-root")
.long("graphql-root")
Expand Down
181 changes: 121 additions & 60 deletions josh-proxy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub mod juniper_hyper;
#[macro_use]
extern crate lazy_static;

fn baseref_and_options(refname: &str) -> josh::JoshResult<(String, String, Vec<String>)> {
fn baseref_and_options(refname: &str) -> josh::JoshResult<(String, String, Vec<String>, bool)> {
let mut split = refname.splitn(2, '%');
let push_to = split.next().ok_or(josh::josh_error("no next"))?.to_owned();

Expand All @@ -15,14 +15,16 @@ fn baseref_and_options(refname: &str) -> josh::JoshResult<(String, String, Vec<S
};

let mut baseref = push_to.to_owned();
let mut for_review = false;

if baseref.starts_with("refs/for") {
for_review = true;
baseref = baseref.replacen("refs/for", "refs/heads", 1)
}
if baseref.starts_with("refs/drafts") {
baseref = baseref.replacen("refs/drafts", "refs/heads", 1)
}
Ok((baseref, push_to, options))
Ok((baseref, push_to, options, for_review))
}

#[derive(serde::Serialize, serde::Deserialize, Debug)]
Expand All @@ -35,11 +37,10 @@ pub struct RepoUpdate {
pub base_ns: String,
pub git_ns: String,
pub git_dir: String,
pub stacked_changes: bool,
}

pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String> {
let mut resp = String::new();

let p = std::path::PathBuf::from(&repo_update.git_dir)
.join("refs/namespaces")
.join(&repo_update.git_ns)
Expand All @@ -59,7 +60,7 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>

let old = git2::Oid::from_str(old)?;

let (baseref, push_to, options) = baseref_and_options(refname)?;
let (baseref, push_to, options, for_review) = baseref_and_options(refname)?;
let josh_merge = push_options.contains_key("merge");

tracing::debug!("push options: {:?}", push_options);
Expand Down Expand Up @@ -120,26 +121,9 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>
None
};

let amends = std::collections::HashMap::new();
//let amends = {
// let gerrit_changes = format!(
// "refs/josh/upstream/{}/refs/changes/*",
// repo_update.base_ns,
// );
// let mut amends = std::collections::HashMap::new();
// for reference in
// transaction.repo().references_glob(&gerrit_changes)?
// {
// if let Ok(commit) = transaction.repo().find_commit(
// reference?.target().unwrap_or(git2::Oid::zero()),
// ) {
// if let Some(id) = josh::get_change_id(&commit) {
// amends.insert(id, commit.id());
// }
// }
// }
// amends
//};
let stacked_changes = repo_update.stacked_changes && for_review;

let mut change_ids = if stacked_changes { Some(vec![]) } else { None };

let filterobj = josh::filter::parse(&repo_update.filter_spec)?;
let new_oid = git2::Oid::from_str(new)?;
Expand All @@ -156,7 +140,7 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>
new_oid,
josh_merge,
reparent_orphans,
&amends,
&mut change_ids,
)? {
josh::UnapplyResult::Done(rewritten) => {
tracing::debug!("rewritten");
Expand Down Expand Up @@ -203,45 +187,68 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>
backward_new_oid
};

let push_with_options = if !options.is_empty() {
let ref_with_options = if !options.is_empty() {
format!("{}{}{}", push_to, "%", options.join(","))
} else {
push_to
};

let reapply = josh::filter::apply_to_commit(
filterobj,
&transaction.repo().find_commit(oid_to_push)?,
&transaction,
)?;
let author = if let Some(p) = push_options.get("author") {
p.to_string()
} else {
"".to_string()
};

let (text, status) = push_head_url(
transaction.repo(),
oid_to_push,
&push_with_options,
&repo_update.remote_url,
&repo_update.auth,
&repo_update.git_ns,
)?;
let to_push = if let Some(change_ids) = change_ids {
let mut v = vec![(
format!(
"refs/heads/@heads/{}/{}",
baseref.replacen("refs/heads/", "", 1),
author,
),
oid_to_push,
)];
v.append(&mut change_ids_to_refs(baseref, author, change_ids)?);
v
} else {
vec![(ref_with_options, oid_to_push)]
};

let warnings = josh::filter::compute_warnings(
&transaction,
filterobj,
transaction.repo().find_commit(oid_to_push)?.tree()?,
);
let mut resp = vec![];

for (reference, oid) in to_push {
let (text, status) = push_head_url(
transaction.repo(),
oid,
&reference,
&repo_update.remote_url,
&repo_update.auth,
&repo_update.git_ns,
stacked_changes,
)?;
if status != 0 {
return Err(josh::josh_error(&text));
}

let mut warning_str = "".to_owned();
if !warnings.is_empty() {
let warnings = warnings.iter();
resp.push(text.to_string());

warning_str += "\nwarnings:";
for warn in warnings {
warning_str += "\n";
warning_str.push_str(warn);
let mut warnings = josh::filter::compute_warnings(
&transaction,
filterobj,
transaction.repo().find_commit(oid)?.tree()?,
);

if !warnings.is_empty() {
resp.push("warnings:".to_string());
resp.append(&mut warnings);
}
}

resp = format!("{}{}{}", resp, text, warning_str);
let reapply = josh::filter::apply_to_commit(
filterobj,
&transaction.repo().find_commit(oid_to_push)?,
&transaction,
)?;

if new_oid != reapply {
transaction.repo().reference(
Expand All @@ -255,14 +262,12 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>
true,
"reapply",
)?;
resp = format!("{}\nREWRITE({} -> {})", resp, new_oid, reapply);
tracing::debug!("REWRITE({} -> {})", new_oid, reapply);
let text = format!("REWRITE({} -> {})", new_oid, reapply);
tracing::debug!("{}", text);
resp.push(text);
}

if status == 0 {
return Ok(resp);
}
return Err(josh::josh_error(&resp));
return Ok(resp.join("\n"));
}

Ok("".to_string())
Expand All @@ -275,6 +280,7 @@ fn push_head_url(
url: &str,
auth: &auth::Handle,
namespace: &str,
force: bool,
) -> josh::JoshResult<(String, i32)> {
let rn = format!("refs/{}", &namespace);

Expand All @@ -284,7 +290,12 @@ fn push_head_url(
cwd: repo.path().to_owned(),
};
let (username, password) = auth.parse()?;
let cmd = format!("git push {} '{}'", &url, &spec);
let cmd = format!(
"git push {} {} '{}'",
if force { "-f" } else { "" },
&url,
&spec
);
let mut fakehead = repo.reference(&rn, oid, true, "push_head_url")?;
let (stdout, stderr, status) = shell.command_env(
&cmd,
Expand Down Expand Up @@ -468,3 +479,53 @@ impl Drop for TmpGitNamespace {
});
}
}

fn change_ids_to_refs(
baseref: String,
change_author: String,
change_ids: Vec<josh::Change>,
) -> josh::JoshResult<Vec<(String, git2::Oid)>> {
let mut seen = vec![];
let mut change_ids = change_ids;
change_ids.retain(|change| change.author == change_author);
if !change_author.contains('@') {
return Err(josh::josh_error(
"Push option 'author' needs to be set to a valid email address",
));
};

for change in change_ids.iter() {
if let Some(id) = &change.id {
if id.contains('@') {
return Err(josh::josh_error("Change-Id must not contain '@'"));
}
if seen.contains(&id) {
return Err(josh::josh_error(&format!(
"rejecting to push {:?} with duplicate Change-Id",
change.commit
)));
}
seen.push(&id);
} else {
return Err(josh::josh_error(&format!(
"rejecting to push {:?} without Change-Id",
change.commit
)));
}
}

Ok(change_ids
.iter()
.map(|change| {
(
format!(
"refs/heads/@changes/{}/{}/{}",
baseref.replacen("refs/heads/", "", 1),
change.author,
change.id.as_ref().unwrap_or(&"".to_string()),
),
change.commit,
)
})
.collect())
}
2 changes: 1 addition & 1 deletion src/bin/josh-filter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ fn run_filter(args: Vec<String>) -> josh::JoshResult<i32> {
new,
false,
None,
&std::collections::HashMap::new(),
&mut None,
)? {
josh::UnapplyResult::Done(rewritten) => {
repo.reference(&src, rewritten, true, "unapply_filter")?;
Expand Down
32 changes: 4 additions & 28 deletions src/history.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ fn find_new_branch_base(
return Ok(git2::Oid::zero());
}

#[tracing::instrument(skip(transaction))]
#[tracing::instrument(skip(transaction, change_ids))]
pub fn unapply_filter(
transaction: &cache::Transaction,
filterobj: filter::Filter,
Expand All @@ -277,7 +277,7 @@ pub fn unapply_filter(
new: git2::Oid,
keep_orphans: bool,
reparent_orphans: Option<git2::Oid>,
amends: &std::collections::HashMap<String, git2::Oid>,
change_ids: &mut Option<Vec<Change>>,
) -> JoshResult<UnapplyResult> {
let mut bm = std::collections::HashMap::new();
let mut ret = original_target;
Expand Down Expand Up @@ -473,32 +473,8 @@ pub fn unapply_filter(
&new_tree,
)?;

if let Some(id) = super::get_change_id(&module_commit) {
if let Some(commit_id) = amends.get(&id) {
let mut merged_index = transaction.repo().merge_commits(
&transaction.repo().find_commit(*commit_id)?,
&transaction.repo().find_commit(ret)?,
Some(git2::MergeOptions::new().file_favor(git2::FileFavor::Theirs)),
)?;

if merged_index.has_conflicts() {
return Ok(UnapplyResult::RejectAmend(
module_commit
.summary()
.unwrap_or("<no message>")
.to_string(),
));
}

let merged_tree = merged_index.write_tree_to(transaction.repo())?;

ret = rewrite_commit(
transaction.repo(),
&module_commit,
&original_parents_refs,
&transaction.repo().find_tree(merged_tree)?,
)?;
}
if let Some(ref mut change_ids) = change_ids {
change_ids.push(super::get_change_id(&module_commit, ret));
}

bm.insert(module_commit.id(), ret);
Expand Down
20 changes: 17 additions & 3 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ pub enum UnapplyResult {
BranchDoesNotExist,
}

pub struct Change {
pub author: String,
pub id: Option<String>,
pub commit: git2::Oid,
}

const FRAGMENT: &percent_encoding::AsciiSet = &percent_encoding::CONTROLS
.add(b'/')
.add(b'*')
Expand Down Expand Up @@ -153,14 +159,22 @@ lazy_static! {
}
}

pub fn get_change_id(commit: &git2::Commit) -> Option<String> {
pub fn get_change_id(commit: &git2::Commit, sha: git2::Oid) -> Change {
for line in commit.message().unwrap_or("").split('\n') {
if line.starts_with("Change-Id: ") {
let id = line.replace("Change-Id: ", "");
return Some(id);
return Change {
author: commit.author().email().unwrap_or("").to_string(),
id: Some(id),
commit: sha,
};
}
}
None
return Change {
author: commit.author().email().unwrap_or("").to_string(),
id: None,
commit: sha,
};
}

#[tracing::instrument(skip(transaction))]
Expand Down
Loading