Skip to content

Use "Change" instead of "Change-Id" for changes mechanisms #926

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
merged 1 commit into from
Sep 23, 2022
Merged

Use "Change" instead of "Change-Id" for changes mechanisms #926

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
38 changes: 19 additions & 19 deletions josh-proxy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>
None
};

let mut change_ids = if push_mode == PushMode::Stack || push_mode == PushMode::Split {
let mut changes = if push_mode == PushMode::Stack || push_mode == PushMode::Split {
Some(vec![])
} else {
None
Expand All @@ -179,7 +179,7 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>
new_oid,
josh_merge,
reparent_orphans,
&mut change_ids,
&mut changes,
)?
};

Expand Down Expand Up @@ -222,9 +222,9 @@ pub fn process_repo_update(repo_update: RepoUpdate) -> josh::JoshResult<String>
"".to_string()
};

let to_push = if let Some(change_ids) = change_ids {
let to_push = if let Some(changes) = changes {
let mut v = vec![];
v.append(&mut change_ids_to_refs(&baseref, &author, change_ids)?);
v.append(&mut changes_to_refs(&baseref, &author, changes)?);

if push_mode == PushMode::Split {
split_changes(transaction.repo(), &mut v, old)?;
Expand Down Expand Up @@ -608,53 +608,53 @@ impl Drop for TmpGitNamespace {
}
}

fn change_ids_to_refs(
fn changes_to_refs(
baseref: &str,
change_author: &str,
change_ids: Vec<josh::Change>,
changes: Vec<josh::Change>,
) -> josh::JoshResult<Vec<(String, git2::Oid, String)>> {
let mut seen = vec![];
let mut change_ids = change_ids;
change_ids.retain(|change| change.author == change_author);
let mut changes = changes;
changes.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 '@'"));
for change in changes.iter() {
if let Some(label) = &change.label {
if label.contains('@') {
return Err(josh::josh_error("Change label must not contain '@'"));
}
if seen.contains(&id) {
if seen.contains(&label) {
return Err(josh::josh_error(&format!(
"rejecting to push {:?} with duplicate Change-Id",
"rejecting to push {:?} with duplicate label",
change.commit
)));
}
seen.push(&id);
seen.push(&label);
} else {
return Err(josh::josh_error(&format!(
"rejecting to push {:?} without Change-Id",
"rejecting to push {:?} without label",
change.commit
)));
}
}

Ok(change_ids
Ok(changes
.iter()
.map(|change| {
(
format!(
"refs/heads/@changes/{}/{}/{}",
baseref.replacen("refs/heads/", "", 1),
change.author,
change.id.as_ref().unwrap_or(&"".to_string()),
change.label.as_ref().unwrap_or(&"".to_string()),
),
change.commit,
change
.id
.label
.as_ref()
.unwrap_or(&"JOSH_PUSH".to_string())
.to_string(),
Expand Down
31 changes: 20 additions & 11 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,21 @@ pub mod shell;
pub struct Change {
pub author: String,
pub id: Option<String>,
pub label: Option<String>,
pub commit: git2::Oid,
}

impl Change {
fn new(commit: git2::Oid) -> Self {
Self {
author: Default::default(),
label: Default::default(),
id: Default::default(),
commit: commit,
}
}
}

pub const VERSION: &str =
git_version::git_version!(args = ["--tags", "--always", "--dirty=-modified"]);

Expand Down Expand Up @@ -150,21 +162,18 @@ lazy_static! {
}

pub fn get_change_id(commit: &git2::Commit, sha: git2::Oid) -> Change {
let mut change = Change::new(sha);
change.author = commit.author().email().unwrap_or("").to_string();

for line in commit.message().unwrap_or("").split('\n') {
if line.starts_with("Change-Id: ") {
let id = line.replace("Change-Id: ", "");
return Change {
author: commit.author().email().unwrap_or("").to_string(),
id: Some(id),
commit: sha,
};
change.id = Some(line.replacen("Change-Id: ", "", 1));
}
if line.starts_with("Change: ") {
change.label = Some(line.replacen("Change: ", "", 1));
}
}
return Change {
author: commit.author().email().unwrap_or("").to_string(),
id: None,
commit: sha,
};
return change;
}

#[tracing::instrument(skip(transaction))]
Expand Down
64 changes: 31 additions & 33 deletions tests/proxy/push_stacked.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@

$ echo contents2 > file2
$ git add file2
$ git commit -m "Change-Id: 1234" 1> /dev/null
$ git commit -m "Change: 1234" 1> /dev/null
$ echo contents2 > file7
$ git add file7
$ git commit -m "Change-Id: foo7" 1> /dev/null
$ git commit -m "Change: foo7" 1> /dev/null
$ git log --decorate --graph --pretty="%s %d"
* Change-Id: foo7 (HEAD -> master)
* Change-Id: 1234
* Change: foo7 (HEAD -> master)
* Change: 1234
* add file1 (origin/master, origin/HEAD)
$ git push -o author=foo@example.com origin master:refs/stack/for/master
remote: josh-proxy
Expand Down Expand Up @@ -57,7 +57,7 @@
$ git push -o author=josh@example.com origin master:refs/stack/for/master
remote: josh-proxy
remote: response from upstream:
remote: rejecting to push 3ad32b3bd3bb778441e7eae43930d8dc6293eddc without Change-Id
remote: rejecting to push 23c67dccc160a868cefdab0bd9d7141b7276c4f3 without label
remote:
remote:
remote: error: hook declined to update refs/stack/for/master
Expand All @@ -69,7 +69,7 @@
remote: josh-proxy
remote: response from upstream:
remote: To http://localhost:8001/real_repo.git
remote: ec41aad..3ad32b3 master -> @heads/master/foo@example.com
remote: 4903654..23c67dc master -> @heads/master/foo@example.com
remote:
remote:
To http://localhost:8002/real_repo.git
Expand All @@ -86,8 +86,8 @@

$ git log --decorate --graph --pretty="%s %d"
* add file3 (HEAD -> master, origin/@heads/master/foo@example.com)
* Change-Id: foo7 (origin/@heads/master/josh@example.com, origin/@changes/master/josh@example.com/foo7)
* Change-Id: 1234 (origin/@changes/master/josh@example.com/1234)
* Change: foo7 (origin/@heads/master/josh@example.com, origin/@changes/master/josh@example.com/foo7)
* Change: 1234 (origin/@changes/master/josh@example.com/1234)
* add file1 (origin/master, origin/HEAD)

$ cd ${TESTTMP}/real_repo
Expand Down Expand Up @@ -116,10 +116,10 @@ get listed if they differ from HEAD

$ git ls-remote http://localhost:8002/real_repo.git
4950fa502f51b7bfda0d7975dbff9b0f9a9481ca\tHEAD (esc)
3b0e3dbefd779ec54d92286047f32d3129161c0d\trefs/heads/@changes/master/josh@example.com/1234 (esc)
ec41aad70b4b898baf48efeb795a7753d9674152\trefs/heads/@changes/master/josh@example.com/foo7 (esc)
3ad32b3bd3bb778441e7eae43930d8dc6293eddc\trefs/heads/@heads/master/foo@example.com (esc)
ec41aad70b4b898baf48efeb795a7753d9674152\trefs/heads/@heads/master/josh@example.com (esc)
82599da2054669a020103a7bd8aa456540a0c5ee\trefs/heads/@changes/master/josh@example.com/1234 (esc)
4903654ec80c5cff86ab37a0b9d7bcf8332e8c54\trefs/heads/@changes/master/josh@example.com/foo7 (esc)
23c67dccc160a868cefdab0bd9d7141b7276c4f3\trefs/heads/@heads/master/foo@example.com (esc)
4903654ec80c5cff86ab37a0b9d7bcf8332e8c54\trefs/heads/@heads/master/josh@example.com (esc)
4950fa502f51b7bfda0d7975dbff9b0f9a9481ca\trefs/heads/master (esc)

$ git ls-remote http://localhost:8002/real_repo.git:/sub1.git
Expand All @@ -128,9 +128,9 @@ get listed if they differ from HEAD
$ git ls-remote http://localhost:8002/real_repo.git::file2.git
$ git ls-remote http://localhost:8002/real_repo.git::file7.git
23b2396b6521abcd906f16d8492c5aeacaee06ed\tHEAD (esc)
08c82a20e92d548ff32f86d634b82da6756e1f5f\trefs/heads/@changes/master/josh@example.com/foo7 (esc)
08c82a20e92d548ff32f86d634b82da6756e1f5f\trefs/heads/@heads/master/foo@example.com (esc)
08c82a20e92d548ff32f86d634b82da6756e1f5f\trefs/heads/@heads/master/josh@example.com (esc)
57a39cc050b1493cf6313822f0de3cee3ca4f39a\trefs/heads/@changes/master/josh@example.com/foo7 (esc)
57a39cc050b1493cf6313822f0de3cee3ca4f39a\trefs/heads/@heads/master/foo@example.com (esc)
57a39cc050b1493cf6313822f0de3cee3ca4f39a\trefs/heads/@heads/master/josh@example.com (esc)
23b2396b6521abcd906f16d8492c5aeacaee06ed\trefs/heads/master (esc)

Make sure all temporary namespace got removed
Expand Down Expand Up @@ -158,16 +158,17 @@ Make sure all temporary namespace got removed
| |-- info
| | `-- exclude
| |-- objects
| | |-- 3a
| | | `-- d32b3bd3bb778441e7eae43930d8dc6293eddc
| | |-- 3b
| | | `-- 0e3dbefd779ec54d92286047f32d3129161c0d
| | |-- 23
| | | `-- c67dccc160a868cefdab0bd9d7141b7276c4f3
| | |-- 3d
| | | `-- 77ff51363c9825cc2a221fc0ba5a883a1a2c72
| | |-- 49
| | | |-- 03654ec80c5cff86ab37a0b9d7bcf8332e8c54
| | | `-- 50fa502f51b7bfda0d7975dbff9b0f9a9481ca
| | |-- 6b
| | | `-- 46faacade805991bcaea19382c9d941828ce80
| | |-- 82
| | | `-- 599da2054669a020103a7bd8aa456540a0c5ee
| | |-- 85
| | | `-- 90a3b0b3086ab857b91581c320e377dc9780ea
| | |-- 90
Expand All @@ -178,8 +179,6 @@ Make sure all temporary namespace got removed
| | | `-- 24003ee1acc6bf70318a46e7b6df651b9dc246
| | |-- b2
| | | `-- dd517c55420a48cb543e0195b4751bf514b941
| | |-- ec
| | | `-- 41aad70b4b898baf48efeb795a7753d9674152
| | |-- ed
| | | `-- b2a5b9c65fae1d20c1b1fb777d1ea025456faa
| | |-- info
Expand Down Expand Up @@ -210,22 +209,23 @@ Make sure all temporary namespace got removed
|-- info
| `-- exclude
|-- objects
| |-- 08
| | `-- c82a20e92d548ff32f86d634b82da6756e1f5f
| |-- 0b
| | `-- 4cf6c9efbbda1eada39fa9c1d21d2525b027bb
| |-- 0f
| | `-- 8d6bce4783eb0366cc72cb5f6fb7952c360d89
| |-- 23
| | `-- b2396b6521abcd906f16d8492c5aeacaee06ed
| |-- 38
| | `-- 25d17d9f46345e352ea6d1d0a89f2be16a1b2c
| |-- 3a
| | `-- d32b3bd3bb778441e7eae43930d8dc6293eddc
| |-- 3b
| | `-- 0e3dbefd779ec54d92286047f32d3129161c0d
| | |-- b2396b6521abcd906f16d8492c5aeacaee06ed
| | `-- c67dccc160a868cefdab0bd9d7141b7276c4f3
| |-- 49
| | `-- 03654ec80c5cff86ab37a0b9d7bcf8332e8c54
| |-- 57
| | `-- a39cc050b1493cf6313822f0de3cee3ca4f39a
| |-- 6b
| | `-- 46faacade805991bcaea19382c9d941828ce80
| |-- 82
| | `-- 599da2054669a020103a7bd8aa456540a0c5ee
| |-- 8f
| | `-- f5dfb4fb8ef665cf128d90b68f0ed2f612cafd
| |-- 96
| | `-- 539577d449c8e5446b5339c20436a13ec51f41
| |-- 9a
Expand All @@ -234,8 +234,6 @@ Make sure all temporary namespace got removed
| | `-- a557394ce29f000108607abd97f19fed4d1b7c
| |-- b2
| | `-- dd517c55420a48cb543e0195b4751bf514b941
| |-- ec
| | `-- 41aad70b4b898baf48efeb795a7753d9674152
| |-- ed
| | `-- b2a5b9c65fae1d20c1b1fb777d1ea025456faa
| |-- info
Expand All @@ -245,7 +243,7 @@ Make sure all temporary namespace got removed
|-- namespaces
`-- tags

57 directories, 43 files
55 directories, 43 files

$ cat ${TESTTMP}/josh-proxy.out
$ cat ${TESTTMP}/josh-proxy.out | grep REPO_UPDATE
Expand Down
Loading