From 33bc92abc89b885ace13b6b4460e445ba0b1eaeb Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Fri, 6 Jul 2018 15:23:30 -0700 Subject: [PATCH 01/11] Rename BranchName type Prior to this commit, we were wrapping strings representing branch names into BranchName types to reduce potential for misuse. This commit renames that to ReferenceName to reflect upcoming tag support. --- src/bin/git-secure-fetch.rs | 4 ++-- src/bin/git-secure-push.rs | 4 ++-- src/lib.rs | 16 ++++++++-------- tests/property_tests.rs | 6 +++--- tests/utils/model.rs | 6 +++--- tests/utils/rsl.rs | 8 ++++---- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/bin/git-secure-fetch.rs b/src/bin/git-secure-fetch.rs index 19add6c..a144585 100644 --- a/src/bin/git-secure-fetch.rs +++ b/src/bin/git-secure-fetch.rs @@ -9,7 +9,7 @@ use std::process; use clap::{App, Arg}; use git_rsl::errors::*; use git_rsl::utils::git; -use git_rsl::{secure_fetch_with_cleanup, BranchName, RemoteName}; +use git_rsl::{secure_fetch_with_cleanup, ReferenceName, RemoteName}; fn main() { let matches = App::new("git-secure-fetch") @@ -43,7 +43,7 @@ fn main() { if let Err(ref e) = secure_fetch_with_cleanup( &mut repo, &RemoteName::new(&remote), - &BranchName::new(&branch), + &ReferenceName::new(&branch), ) { handle_error(e); process::exit(1); diff --git a/src/bin/git-secure-push.rs b/src/bin/git-secure-push.rs index 56d9ebe..37d0f61 100644 --- a/src/bin/git-secure-push.rs +++ b/src/bin/git-secure-push.rs @@ -7,7 +7,7 @@ extern crate git_rsl; use clap::{App, Arg}; use git_rsl::errors::*; use git_rsl::utils::git; -use git_rsl::{secure_push_with_cleanup, BranchName, RemoteName}; +use git_rsl::{secure_push_with_cleanup, ReferenceName, RemoteName}; use std::process; fn main() { @@ -42,7 +42,7 @@ fn main() { if let Err(ref e) = secure_push_with_cleanup( &mut repo, &RemoteName::new(&remote), - &BranchName::new(&branch), + &ReferenceName::new(&branch), ) { handle_error(e); process::exit(1); diff --git a/src/lib.rs b/src/lib.rs index c15a9cf..1e6d616 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,18 +31,18 @@ use git2::{Oid, Repository}; use std::env; use std::path::PathBuf; -/// Wrapper around a string reference to a branch name to reduce the odds of +/// Wrapper around a string reference to a typed reference to reduce the odds of /// parameter mismatch #[derive(Clone, Debug)] -pub struct BranchName<'a>(&'a str); +pub struct ReferenceName<'a>(&'a str); -impl<'a> BranchName<'a> { - pub fn new(source: &'a str) -> BranchName<'a> { - BranchName(source) +impl<'a> ReferenceName<'a> { + pub fn new(source: &'a str) -> ReferenceName<'a> { + ReferenceName(source) } } -impl<'a> AsRef for BranchName<'a> { +impl<'a> AsRef for ReferenceName<'a> { fn as_ref(&self) -> &str { self.0 } @@ -80,7 +80,7 @@ pub fn rsl_init_with_cleanup(repo: &mut Repository, remote_name: &RemoteName) -> pub fn secure_fetch_with_cleanup( repo: &mut Repository, remote_name: &RemoteName, - branch: &BranchName, + branch: &ReferenceName, ) -> Result<()> { ensure!(remote_name.0 == "origin", "Remote name must be \"origin\""); let ws = Workspace::new(repo)?; @@ -93,7 +93,7 @@ pub fn secure_fetch_with_cleanup( pub fn secure_push_with_cleanup( repo: &mut Repository, remote_name: &RemoteName, - branch: &BranchName, + branch: &ReferenceName, ) -> Result<()> { ensure!(remote_name.0 == "origin", "Remote name must be \"origin\""); let ws = Workspace::new(repo)?; diff --git a/tests/property_tests.rs b/tests/property_tests.rs index bfbac4c..26ced19 100644 --- a/tests/property_tests.rs +++ b/tests/property_tests.rs @@ -18,7 +18,7 @@ mod utils; use utils::model::{Action, Branch, Commit, Repo, State, Tool}; use git_rsl::utils::test_helper::*; -use git_rsl::{BranchName, RemoteName}; +use git_rsl::{ReferenceName, RemoteName}; use proptest::prelude::*; use proptest::sample::select; use std::collections::HashMap; @@ -232,7 +232,7 @@ proptest!{ let remote_name = RemoteName::new("origin"); git_rsl::rsl_init_with_cleanup(&mut context.local, &remote_name).expect("failed to init rsl"); - git_rsl::secure_push_with_cleanup(&mut context.local, &remote_name, &BranchName::new("master")).expect("failed to secure push initial commit"); + git_rsl::secure_push_with_cleanup(&mut context.local, &remote_name, &ReferenceName::new("master")).expect("failed to secure push initial commit"); let mut locals = utils::setup_local_repos(&context, state.locals.len()); @@ -266,7 +266,7 @@ proptest!{ let num_successful_rsl_actions = { let remote_name = RemoteName::new("origin"); git_rsl::rsl_init_with_cleanup(&mut context.local, &remote_name).expect("failed to init rsl"); - git_rsl::secure_push_with_cleanup(&mut context.local, &remote_name, &BranchName::new("master")).expect("failed to secure push initial commit"); + git_rsl::secure_push_with_cleanup(&mut context.local, &remote_name, &ReferenceName::new("master")).expect("failed to secure push initial commit"); let mut locals = utils::setup_local_repos(&context, state.locals.len()); diff --git a/tests/utils/model.rs b/tests/utils/model.rs index ee3709d..ce998c9 100644 --- a/tests/utils/model.rs +++ b/tests/utils/model.rs @@ -1,5 +1,5 @@ extern crate git_rsl; -use self::git_rsl::BranchName; +use self::git_rsl::ReferenceName; use git2::Repository; use names::Generator; use std::collections::{HashMap, HashSet}; @@ -77,11 +77,11 @@ impl Action { &Action::Commit(repo_num, ref message) => git::commit(&locals[repo_num], message), &Action::Push(repo_num, ref branch) => match tool { Tool::Git => git::push(&locals[repo_num], branch), - Tool::RSL => rsl::push(&mut locals[repo_num], &BranchName::new(branch)), + Tool::RSL => rsl::push(&mut locals[repo_num], &ReferenceName::new(branch)), }, &Action::Pull(repo_num, ref branch) => match tool { Tool::Git => git::pull(&locals[repo_num], branch), - Tool::RSL => rsl::pull(&mut locals[repo_num], &BranchName::new(branch)), + Tool::RSL => rsl::pull(&mut locals[repo_num], &ReferenceName::new(branch)), }, &Action::Branch(repo_num, ref name) => git::branch(&locals[repo_num], name), &Action::Checkout(repo_num, ref branch) => git::checkout(&locals[repo_num], branch), diff --git a/tests/utils/rsl.rs b/tests/utils/rsl.rs index 988893e..f07d6dd 100644 --- a/tests/utils/rsl.rs +++ b/tests/utils/rsl.rs @@ -1,14 +1,14 @@ extern crate git2; extern crate git_rsl; -use self::git_rsl::{BranchName, RemoteName}; +use self::git_rsl::{ReferenceName, RemoteName}; use git2::Repository; use git_rsl::errors::{Error, ErrorKind}; use std::process::{Command, Stdio}; const INVALID_FETCH_RSL: &str = "Couldn\'t fetch; No push entry for latest commit on target branch. It is likely that someone pushed without using git-rsl. Please have that developer secure-push the branch and try again."; -pub fn push(mut repo: &mut Repository, branch_name: &BranchName) -> bool { +pub fn push(mut repo: &mut Repository, branch_name: &ReferenceName) -> bool { match git_rsl::secure_push_with_cleanup(&mut repo, &RemoteName::new("origin"), branch_name) { Ok(()) => true, Err(error) => match error { @@ -28,7 +28,7 @@ pub fn push(mut repo: &mut Repository, branch_name: &BranchName) -> bool { } } -fn merge(repo: &Repository, branch_name: &BranchName) -> bool { +fn merge(repo: &Repository, branch_name: &ReferenceName) -> bool { Command::new("git") .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -39,7 +39,7 @@ fn merge(repo: &Repository, branch_name: &BranchName) -> bool { .success() } -pub fn pull(mut repo: &mut Repository, branch_name: &BranchName) -> bool { +pub fn pull(mut repo: &mut Repository, branch_name: &ReferenceName) -> bool { match git_rsl::secure_fetch_with_cleanup(&mut repo, &RemoteName::new("origin"), &branch_name) { Ok(()) => merge(&repo, branch_name), Err(error) => match error { From 52b791e8538120c8e33e1a7009553af622677140 Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Fri, 6 Jul 2018 15:25:46 -0700 Subject: [PATCH 02/11] Add tag support for git push operation and test Prior to this commit, the git methods for push were bound to using branches. This commit removes that and generalizes it to enable pushing a branch or tag ref. It adds a new test for tag pushing as well as a helper method to tag the latest commit in a repo. It also updates the integration tests to use the ReferenceName type. --- src/utils/git.rs | 36 ++++++++++++++++++++------------ src/utils/test_helper.rs | 9 +++++++- tests/integration_test.rs | 44 +++++++++++++++++++++++++++------------ 3 files changed, 62 insertions(+), 27 deletions(-) diff --git a/src/utils/git.rs b/src/utils/git.rs index e17876f..f1b751c 100644 --- a/src/utils/git.rs +++ b/src/utils/git.rs @@ -1,11 +1,11 @@ -use std::env; +use std::{env, str}; use std::path::Path; use git2; use git2::BranchType; use git2::build::CheckoutBuilder; use git2::{Commit, DiffOptions, FetchOptions, Oid, PushOptions, Remote, RemoteCallbacks, - Repository, RepositoryState, Signature, Tree}; + Repository, RepositoryState, Signature, Tree, Reference}; use git2::CredentialType; use git2::MergeAnalysis; @@ -267,21 +267,31 @@ pub fn fetch( }) } +fn get_ref_from_name<'repo>(repo: &'repo Repository, name: &str) -> Option> { + match repo.find_branch(name, BranchType::Local) { + Ok(branch) => Some(branch.into_reference()), + Err(e) => { // toDo may be other errors here... + let tag_ref = repo.find_reference(&format!("refs/tags/{}", name)).expect("reference not found"); + if tag_ref.is_tag() { + Some(tag_ref) + } else { // not a branch or a tag + None + } + } + } +} + /// Push the branches or tags given in ref_names pub fn push(repo: &Repository, remote: &mut Remote, ref_names: &[&str]) -> Result<()> { - let refs: Vec = ref_names - .iter() - .map(|name: &&str| { - format!( - "refs/heads/{}:refs/heads/{}", - name.to_string(), - name.to_string() - ) - }) - .collect(); + let mut full_names: Vec = vec![]; + for name in ref_names { + let reference = get_ref_from_name(&repo, name).expect("failed to find reference from name"); + let full_name = String::from(reference.name().expect("failed to get full name of ref")); + full_names.push(full_name) + } let mut refspecs: Vec<&str> = vec![]; - for name in &refs { + for name in &full_names { refspecs.push(name) } diff --git a/src/utils/test_helper.rs b/src/utils/test_helper.rs index 6b52118..08f6347 100644 --- a/src/utils/test_helper.rs +++ b/src/utils/test_helper.rs @@ -10,7 +10,7 @@ use super::git; use fs_extra::dir::*; use tempdir::TempDir; -use git2::Repository; +use git2::{Repository, Object, ObjectType, Reference}; pub struct Context { pub local: Repository, @@ -87,6 +87,13 @@ pub fn create_file_with_text>(path: P, text: &str) -> () { file.write_all(text.as_bytes()).unwrap(); } + +pub fn tag_lightweight<'repo>(repo: &'repo Repository, tag_name: &str) -> Reference<'repo> { + let tag_target = &repo.head().expect("failed to get head").peel(ObjectType::Commit).expect("failed to peel"); + let tag_oid = repo.tag_lightweight(tag_name, tag_target, false); + repo.find_reference(&format!("refs/tags/{}", tag_name)).expect("tag ref not found") +} + pub fn do_work_on_branch(repo: &Repository, branch_name: &str) -> () { git::checkout_branch(&repo, branch_name).unwrap(); git::add_and_commit(&repo, None, "a commit with some work", branch_name).unwrap(); diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 8c64651..2b6c31f 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -9,7 +9,7 @@ extern crate tempdir; mod utils; use git_rsl::utils::test_helper::*; -use git_rsl::{BranchName, RemoteName}; +use git_rsl::{ReferenceName, RemoteName}; use std::process::Command; use std::sync::Mutex; use utils::attack; @@ -30,36 +30,54 @@ macro_rules! sequential_test { } sequential_test! { - fn push_and_fetch() { + fn push_and_fetch_branch() { let mut context = setup_fresh(); { assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) .expect("Could not rsl-init")); - let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect("Could not run first push"); + let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Could not run first push"); assert_eq!(res, ()); do_work_on_branch(&context.local, "refs/heads/master"); - let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect("Could not run second push"); + let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Could not run second push"); assert_eq!(res2, ()); - let res3 = git_rsl::secure_fetch_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect("Could not run fetch"); + let res3 = git_rsl::secure_fetch_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Could not run fetch"); assert_eq!(res3, ()); do_work_on_branch(&context.local, "refs/heads/master"); - let res4 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect("Could not run third push"); + let res4 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Could not run third push"); assert_eq!(res4, ()); // TODO check that the git log of RSL looks how we want it to } } } +sequential_test! { + fn push_and_fetch_tag() { + let mut context = setup_fresh(); + { + assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) + .expect("Could not rsl-init")); + do_work_on_branch(&context.local, "refs/heads/master"); + // let local_tag = tag_lightweight(&mut context.local, "v6.66"); + + // git_rsl::utils::git::push(&context.local, &mut context.local.find_remote("origin").expect("failed to find remote"), &["v6.66"]); + assert_eq!((), git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("v6.66")).expect("Could not run third push")); + let remote_tag = &context.remote.find_reference("refs/tags/v6.66").expect("reference not found"); + // assert_eq!(local_tag.target(), remote_tag.target()) + assert!(remote_tag.is_tag()); + } + } +} + sequential_test! { fn error_handling() { let mut context = setup_fresh(); { assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) .expect("Could not rsl-init")); - let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).unwrap(); + let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).unwrap(); assert_eq!(res, ()); let nonce_file = context.repo_dir.join(".git/NONCE"); @@ -70,7 +88,7 @@ sequential_test! { .expect("failed to change permissions"); do_work_on_branch(&context.local, "refs/heads/master"); - let _res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).unwrap_err(); + let _res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).unwrap_err(); let head = context.local.head().unwrap().name().unwrap().to_owned(); assert_eq!(head, "refs/heads/master"); @@ -84,11 +102,11 @@ sequential_test! { { assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) .expect("Could not rsl-init")); - let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect("First push failed"); + let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("First push failed"); assert_eq!(res, ()); do_work_on_branch(&context.local, "refs/heads/master"); - let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect("Second push failed"); + let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Second push failed"); assert_eq!(res2, ()); } } @@ -100,17 +118,17 @@ sequential_test! { { assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) .expect("Could not rsl-init")); - let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect("First push failed"); + let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("First push failed"); assert_eq!(res, ()); do_work_on_branch(&context.local, "refs/heads/master"); - let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect("Second push failed"); + let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Second push failed"); assert_eq!(res2, ()); attack::rollback(&context.remote, "master"); do_work_on_branch(&context.local, "refs/heads/master"); - let res3 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &BranchName::new("master")).expect_err("Checking for invalid RSL detection"); + let res3 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect_err("Checking for invalid RSL detection"); assert_eq!(res3.description(), "invalid remote RSL"); } } From 5315bc73a7fbeccc05a8aaf3c8a8767773a7e217 Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Fri, 6 Jul 2018 16:51:12 -0700 Subject: [PATCH 03/11] Change bin docs to REFERENCE Prior to this commit, the help prompts for the binaries still told the user that they need to supply BRANCH references. This commit generalizes that to include branches OR tags by using REFERENCE --- src/bin/git-secure-fetch.rs | 12 ++++++------ src/bin/git-secure-push.rs | 12 ++++++------ src/lib.rs | 8 ++++---- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/bin/git-secure-fetch.rs b/src/bin/git-secure-fetch.rs index a144585..460d935 100644 --- a/src/bin/git-secure-fetch.rs +++ b/src/bin/git-secure-fetch.rs @@ -14,13 +14,13 @@ use git_rsl::{secure_fetch_with_cleanup, ReferenceName, RemoteName}; fn main() { let matches = App::new("git-secure-fetch") .bin_name("git secure-fetch") - .about("Securely fetch from checking the reference state log to protect against metadata attacks") + .about("Securely fetch from checking the reference state log to protect against metadata attacks") .arg(Arg::with_name("REMOTE") .help("The remote repository that is the source of the fetch operation.") .takes_value(false) .required(true)) - .arg(Arg::with_name("BRANCH") - .help("The target branch to fetch.") + .arg(Arg::with_name("REFERENCE") + .help("The target ref (branch or tag) to fetch.") .takes_value(false) .required(true)) .version(crate_version!()) @@ -32,8 +32,8 @@ fn main() { Some(v) => v.to_owned(), }; - let branch = match matches.value_of("BRANCH") { - None => panic!("Must supply a BRANCH argument"), + let reference = match matches.value_of("REFERENCE") { + None => panic!("Must supply a REFERENCE argument"), Some(v) => v.to_owned(), }; // TODO - reduce code duplication across the top level of the binaries @@ -43,7 +43,7 @@ fn main() { if let Err(ref e) = secure_fetch_with_cleanup( &mut repo, &RemoteName::new(&remote), - &ReferenceName::new(&branch), + &ReferenceName::new(&reference), ) { handle_error(e); process::exit(1); diff --git a/src/bin/git-secure-push.rs b/src/bin/git-secure-push.rs index 37d0f61..98a8061 100644 --- a/src/bin/git-secure-push.rs +++ b/src/bin/git-secure-push.rs @@ -13,13 +13,13 @@ use std::process; fn main() { let matches = App::new("git-secure-push") .bin_name("git secure-push") - .about("Securely push to while checking and updating the reference state log to protect against metadata attacks") + .about("Securely push to while checking and updating the reference state log to protect against metadata attacks") .arg(Arg::with_name("REMOTE") .help("The remote repository that is the target of the push operation. (example: origin)") .takes_value(false) .required(true)) - .arg(Arg::with_name("BRANCH") - .help("The target branch to push. (example: master)") + .arg(Arg::with_name("REFERENCE") + .help("The target reference (branch or tag) to push.") .takes_value(false) .required(true)) .version(crate_version!()) @@ -31,8 +31,8 @@ fn main() { Some(v) => v.to_owned(), }; - let branch = match matches.value_of("BRANCH") { - None => panic!("Must supply a BRANCH argument"), + let reference = match matches.value_of("REFERENCE") { + None => panic!("Must supply a REFERENCE argument"), Some(v) => v.to_owned(), }; // TODO - reduce code duplication across the top level of the binaries @@ -42,7 +42,7 @@ fn main() { if let Err(ref e) = secure_push_with_cleanup( &mut repo, &RemoteName::new(&remote), - &ReferenceName::new(&branch), + &ReferenceName::new(&reference), ) { handle_error(e); process::exit(1); diff --git a/src/lib.rs b/src/lib.rs index 1e6d616..e3abc68 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,27 +80,27 @@ pub fn rsl_init_with_cleanup(repo: &mut Repository, remote_name: &RemoteName) -> pub fn secure_fetch_with_cleanup( repo: &mut Repository, remote_name: &RemoteName, - branch: &ReferenceName, + ref_name: &ReferenceName, ) -> Result<()> { ensure!(remote_name.0 == "origin", "Remote name must be \"origin\""); let ws = Workspace::new(repo)?; let mut remote = ws.repo .find_remote(remote_name.as_ref()) .chain_err(|| format!("unable to find remote named {}", remote_name.as_ref()))?; - fetch::secure_fetch(ws.repo, &mut remote, &[branch.as_ref()]) + fetch::secure_fetch(ws.repo, &mut remote, &[ref_name.as_ref()]) } pub fn secure_push_with_cleanup( repo: &mut Repository, remote_name: &RemoteName, - branch: &ReferenceName, + ref_name: &ReferenceName, ) -> Result<()> { ensure!(remote_name.0 == "origin", "Remote name must be \"origin\""); let ws = Workspace::new(repo)?; let mut remote = ws.repo .find_remote(remote_name.as_ref()) .chain_err(|| format!("unable to find remote named {}", remote_name.as_ref()))?; - push::secure_push(ws.repo, &mut remote, &[branch.as_ref()]) + push::secure_push(ws.repo, &mut remote, &[ref_name.as_ref()]) } pub struct Workspace<'repo> { From e368a1cd259e7f8f8d5fac6786da4a66ab28395d Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Fri, 6 Jul 2018 17:16:28 -0700 Subject: [PATCH 04/11] Use reference name to find target Prior to this commit, push entries would try to locate a branch using the shorthand name provided from the user. This was problematic because if the user was attempting to push a tag, the operation would fail. This commit changes it to instead use the git helper method to find a git2::Reference using the shorthand name, so that we can stash the target git2::Oid of either a branch OR a tag in the RSL push entry. --- src/push_entry.rs | 11 ++++------- src/utils/git.rs | 2 +- tests/integration_test.rs | 4 +--- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/push_entry.rs b/src/push_entry.rs index 298b261..0f15404 100644 --- a/src/push_entry.rs +++ b/src/push_entry.rs @@ -53,15 +53,12 @@ impl PushEntry { prev: String, nonce_bag: NonceBag, ) -> PushEntry { - let branch_head = repo.find_branch(branch_str, BranchType::Local) - .unwrap() - .get() - .target() - .unwrap(); + let full_ref = utils::git::get_ref_from_name(repo, branch_str).expect("failed to get reference"); + let target_oid = full_ref.target().expect("failed to get target oid"); PushEntry { - ref_name: format!("refs/heads/{}", branch_str), - oid: branch_head, + ref_name: String::from(full_ref.name().expect("failed to get ref's full name")), + oid: target_oid, prev_hash: prev, nonce_bag, } diff --git a/src/utils/git.rs b/src/utils/git.rs index f1b751c..962c23e 100644 --- a/src/utils/git.rs +++ b/src/utils/git.rs @@ -267,7 +267,7 @@ pub fn fetch( }) } -fn get_ref_from_name<'repo>(repo: &'repo Repository, name: &str) -> Option> { +pub fn get_ref_from_name<'repo>(repo: &'repo Repository, name: &str) -> Option> { match repo.find_branch(name, BranchType::Local) { Ok(branch) => Some(branch.into_reference()), Err(e) => { // toDo may be other errors here... diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 2b6c31f..ec9eadc 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -60,12 +60,10 @@ sequential_test! { assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) .expect("Could not rsl-init")); do_work_on_branch(&context.local, "refs/heads/master"); - // let local_tag = tag_lightweight(&mut context.local, "v6.66"); + tag_lightweight(&mut context.local, "v6.66"); - // git_rsl::utils::git::push(&context.local, &mut context.local.find_remote("origin").expect("failed to find remote"), &["v6.66"]); assert_eq!((), git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("v6.66")).expect("Could not run third push")); let remote_tag = &context.remote.find_reference("refs/tags/v6.66").expect("reference not found"); - // assert_eq!(local_tag.target(), remote_tag.target()) assert!(remote_tag.is_tag()); } } From 478d4d3b1adcc523277f84dae14f5421a629f585 Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Tue, 10 Jul 2018 12:19:10 -0700 Subject: [PATCH 05/11] Generalize finding remote push entries Prior to this commit, the `find_remote_push_entry_for_branch` method tried to use the shorthand name to locate the full reference name. This commit changes it to instead find the reference, either a branch or a tag, and get that name instead. This allows this method to locate RSL push entries for tags as well as branches. --- src/rsl.rs | 10 ++++++++-- tests/integration_test.rs | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/rsl.rs b/src/rsl.rs index 1823785..159d199 100644 --- a/src/rsl.rs +++ b/src/rsl.rs @@ -38,7 +38,12 @@ pub fn all_push_entries_in_fetch_head(repo: &Repository, rsl: &RSL, ref_names: & println!("ref_name: {:?}", ref_name); match repo.find_branch(&format!("origin/{}", ref_name), BranchType::Remote) { Ok(branch) => branch.get().target(), - Err(_) => None, + Err(_) => { + match repo.find_reference(&format!("refs/tags/{}", ref_name)) { + Ok(tag_ref) => tag_ref.target(), + Err(_) => None + } + }, } }) .collect(); @@ -232,7 +237,8 @@ impl<'remote, 'repo> RSL<'remote, 'repo> { let mut revwalk: Revwalk = self.repo.revwalk()?; revwalk.push(self.remote_head)?; let mut current = Some(self.remote_head); - let ref_name = format!("refs/heads/{}", branch); + let reference = git::get_ref_from_name(self.repo, branch).expect("failed to get ref"); + let ref_name = reference.name().expect("failed to get ref name"); while current != None { if let Some(pe) = PushEntry::from_oid(self.repo, ¤t.unwrap())? { if pe.ref_name() == ref_name { diff --git a/tests/integration_test.rs b/tests/integration_test.rs index ec9eadc..3eddfd2 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -62,9 +62,11 @@ sequential_test! { do_work_on_branch(&context.local, "refs/heads/master"); tag_lightweight(&mut context.local, "v6.66"); - assert_eq!((), git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("v6.66")).expect("Could not run third push")); + assert_eq!((), git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("v6.66")).expect("Could not push tag")); let remote_tag = &context.remote.find_reference("refs/tags/v6.66").expect("reference not found"); assert!(remote_tag.is_tag()); + + assert_eq!((), git_rsl::secure_fetch_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("v6.66")).expect("could not fetch tag")); } } } From 9febba09084e8b69abcf186e84745cd964472707 Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Tue, 10 Jul 2018 12:24:18 -0700 Subject: [PATCH 06/11] Change some naming Prior to this commit, methods to locate refererences that had been previously changed from locating branches had not been renamed to reflect their new functionality. This commit does all that stuff. --- src/fetch.rs | 4 ++-- src/rsl.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/fetch.rs b/src/fetch.rs index 89ff85f..a34da7c 100644 --- a/src/fetch.rs +++ b/src/fetch.rs @@ -35,8 +35,8 @@ pub fn secure_fetch<'remote, 'repo: 'remote>( let mut rsl = RSL::read(repo, &mut remote_2).chain_err(|| "couldn't read RSL")?; // reject if one of the branches has no rsl push entry - for branch in ref_names { - match rsl.find_last_remote_push_entry_for_branch(&branch) { + for reference in ref_names { + match rsl.find_last_remote_push_entry_for_reference(&reference) { Ok(None) => bail!("no push records for the ref you are attempting to fetch"), Err(e) => { return Err(e.chain_err(|| "couldn't check that provided refs are valid")) diff --git a/src/rsl.rs b/src/rsl.rs index 159d199..efc5e51 100644 --- a/src/rsl.rs +++ b/src/rsl.rs @@ -20,18 +20,18 @@ pub enum RSLType { } pub fn all_push_entries_in_fetch_head(repo: &Repository, rsl: &RSL, ref_names: &[&str]) -> bool { - // find the last push entry for each branch + // find the last push entry for each reference let latest_push_entries: Vec = ref_names .into_iter() .filter_map( - |ref_name| match rsl.find_last_remote_push_entry_for_branch(ref_name).ok() { + |ref_name| match rsl.find_last_remote_push_entry_for_reference(ref_name).ok() { Some(Some(pe)) => Some(pe.oid()), Some(None) | None => None, }, ) .collect(); - // find the Oid of the tip of each remote fetched branch + // find the Oid of the tip of each remote fetched reference let fetch_heads: Vec = ref_names .into_iter() .filter_map(|ref_name| { @@ -230,14 +230,14 @@ impl<'remote, 'repo> RSL<'remote, 'repo> { Ok(()) } - pub fn find_last_remote_push_entry_for_branch( + pub fn find_last_remote_push_entry_for_reference( &self, - branch: &str, + reference: &str, ) -> Result> { let mut revwalk: Revwalk = self.repo.revwalk()?; revwalk.push(self.remote_head)?; let mut current = Some(self.remote_head); - let reference = git::get_ref_from_name(self.repo, branch).expect("failed to get ref"); + let reference = git::get_ref_from_name(self.repo, reference).expect("failed to get ref"); let ref_name = reference.name().expect("failed to get ref name"); while current != None { if let Some(pe) = PushEntry::from_oid(self.repo, ¤t.unwrap())? { From 2fc4cdca5549f4015f8da9b87485c614431e574c Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Tue, 10 Jul 2018 16:11:51 -0700 Subject: [PATCH 07/11] Code cleanup This commit cleans up some formatting issues and implements a macro for checking that method calls are ok in the integration tests file. --- src/push_entry.rs | 13 +++-- src/rsl.rs | 10 ++-- src/utils/git.rs | 16 +++--- src/utils/test_helper.rs | 13 +++-- tests/integration_test.rs | 103 ++++++++++++++++++++++++++------------ tests/property_tests.rs | 8 ++- 6 files changed, 103 insertions(+), 60 deletions(-) diff --git a/src/push_entry.rs b/src/push_entry.rs index 0f15404..61308c5 100644 --- a/src/push_entry.rs +++ b/src/push_entry.rs @@ -2,8 +2,7 @@ use std::fmt; use crypto::digest::Digest; use crypto::sha3::Sha3; -use git2::{self, BranchType, Oid, Reference, Repository}; -//use libgit2_sys::GIT_OID_RAWSZ; +use git2::{self, Oid, Reference, Repository}; use nonce_bag::NonceBag; @@ -14,8 +13,11 @@ use utils; #[serde(remote = "Oid")] #[derive(Serialize, Deserialize)] struct OidDef { - #[serde(serialize_with = "utils::buffer_to_hex", deserialize_with = "utils::hex_to_buffer", - getter = "get_raw_oid")] + #[serde( + serialize_with = "utils::buffer_to_hex", + deserialize_with = "utils::hex_to_buffer", + getter = "get_raw_oid" + )] raw: Vec, } @@ -53,7 +55,8 @@ impl PushEntry { prev: String, nonce_bag: NonceBag, ) -> PushEntry { - let full_ref = utils::git::get_ref_from_name(repo, branch_str).expect("failed to get reference"); + let full_ref = + utils::git::get_ref_from_name(repo, branch_str).expect("failed to get reference"); let target_oid = full_ref.target().expect("failed to get target oid"); PushEntry { diff --git a/src/rsl.rs b/src/rsl.rs index efc5e51..790c0b4 100644 --- a/src/rsl.rs +++ b/src/rsl.rs @@ -38,11 +38,9 @@ pub fn all_push_entries_in_fetch_head(repo: &Repository, rsl: &RSL, ref_names: & println!("ref_name: {:?}", ref_name); match repo.find_branch(&format!("origin/{}", ref_name), BranchType::Remote) { Ok(branch) => branch.get().target(), - Err(_) => { - match repo.find_reference(&format!("refs/tags/{}", ref_name)) { - Ok(tag_ref) => tag_ref.target(), - Err(_) => None - } + Err(_) => match repo.find_reference(&format!("refs/tags/{}", ref_name)) { + Ok(tag_ref) => tag_ref.target(), + Err(_) => None, }, } }) @@ -238,7 +236,7 @@ impl<'remote, 'repo> RSL<'remote, 'repo> { revwalk.push(self.remote_head)?; let mut current = Some(self.remote_head); let reference = git::get_ref_from_name(self.repo, reference).expect("failed to get ref"); - let ref_name = reference.name().expect("failed to get ref name"); + let ref_name = reference.name().expect("failed to get ref name"); while current != None { if let Some(pe) = PushEntry::from_oid(self.repo, ¤t.unwrap())? { if pe.ref_name() == ref_name { diff --git a/src/utils/git.rs b/src/utils/git.rs index 962c23e..a440d63 100644 --- a/src/utils/git.rs +++ b/src/utils/git.rs @@ -1,11 +1,11 @@ -use std::{env, str}; use std::path::Path; +use std::{env, str}; use git2; -use git2::BranchType; use git2::build::CheckoutBuilder; -use git2::{Commit, DiffOptions, FetchOptions, Oid, PushOptions, Remote, RemoteCallbacks, - Repository, RepositoryState, Signature, Tree, Reference}; +use git2::BranchType; +use git2::{Commit, DiffOptions, FetchOptions, Oid, PushOptions, Reference, Remote, + RemoteCallbacks, Repository, RepositoryState, Signature, Tree}; use git2::CredentialType; use git2::MergeAnalysis; @@ -270,11 +270,13 @@ pub fn fetch( pub fn get_ref_from_name<'repo>(repo: &'repo Repository, name: &str) -> Option> { match repo.find_branch(name, BranchType::Local) { Ok(branch) => Some(branch.into_reference()), - Err(e) => { // toDo may be other errors here... - let tag_ref = repo.find_reference(&format!("refs/tags/{}", name)).expect("reference not found"); + Err(_) => { + let tag_ref = repo.find_reference(&format!("refs/tags/{}", name)) + .expect("reference not found"); if tag_ref.is_tag() { Some(tag_ref) - } else { // not a branch or a tag + } else { + // not a branch or a tag None } } diff --git a/src/utils/test_helper.rs b/src/utils/test_helper.rs index 08f6347..6da38b2 100644 --- a/src/utils/test_helper.rs +++ b/src/utils/test_helper.rs @@ -10,7 +10,7 @@ use super::git; use fs_extra::dir::*; use tempdir::TempDir; -use git2::{Repository, Object, ObjectType, Reference}; +use git2::{ObjectType, Reference, Repository}; pub struct Context { pub local: Repository, @@ -87,11 +87,14 @@ pub fn create_file_with_text>(path: P, text: &str) -> () { file.write_all(text.as_bytes()).unwrap(); } - pub fn tag_lightweight<'repo>(repo: &'repo Repository, tag_name: &str) -> Reference<'repo> { - let tag_target = &repo.head().expect("failed to get head").peel(ObjectType::Commit).expect("failed to peel"); - let tag_oid = repo.tag_lightweight(tag_name, tag_target, false); - repo.find_reference(&format!("refs/tags/{}", tag_name)).expect("tag ref not found") + let tag_target = &repo.head() + .expect("failed to get head") + .peel(ObjectType::Commit) + .expect("failed to peel"); + let _tag_oid = repo.tag_lightweight(tag_name, tag_target, false); + repo.find_reference(&format!("refs/tags/{}", tag_name)) + .expect("tag ref not found") } pub fn do_work_on_branch(repo: &Repository, branch_name: &str) -> () { diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 3eddfd2..5abac38 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -29,25 +29,40 @@ macro_rules! sequential_test { }; } +macro_rules! assert_ok { + ($fn:expr, $msg:expr) => { + assert_eq!((), $fn.expect($msg)) + } +} + sequential_test! { fn push_and_fetch_branch() { let mut context = setup_fresh(); { - assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) - .expect("Could not rsl-init")); - let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Could not run first push"); - assert_eq!(res, ()); + let remote = RemoteName::new("origin"); + let master = ReferenceName::new("master"); + + assert_ok!( + git_rsl::rsl_init_with_cleanup(&mut context.local, &remote), + "Could not rsl-init"); + + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master), + "Could not run first push"); do_work_on_branch(&context.local, "refs/heads/master"); - let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Could not run second push"); - assert_eq!(res2, ()); + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master), + "Could not run second push"); - let res3 = git_rsl::secure_fetch_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Could not run fetch"); - assert_eq!(res3, ()); + assert_ok!( + git_rsl::secure_fetch_with_cleanup(&mut context.local, &remote, &master), + "Could not run fetch"); do_work_on_branch(&context.local, "refs/heads/master"); - let res4 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Could not run third push"); - assert_eq!(res4, ()); + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master), + "Could not run third push"); // TODO check that the git log of RSL looks how we want it to } } @@ -57,16 +72,23 @@ sequential_test! { fn push_and_fetch_tag() { let mut context = setup_fresh(); { - assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) - .expect("Could not rsl-init")); + let remote = RemoteName::new("origin"); + let tag = ReferenceName::new("v6.66"); + + assert_ok!(git_rsl::rsl_init_with_cleanup(&mut context.local, &remote), + "Could not rsl-init"); do_work_on_branch(&context.local, "refs/heads/master"); tag_lightweight(&mut context.local, "v6.66"); - assert_eq!((), git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("v6.66")).expect("Could not push tag")); + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &tag), + "Could not push tag"); let remote_tag = &context.remote.find_reference("refs/tags/v6.66").expect("reference not found"); assert!(remote_tag.is_tag()); - assert_eq!((), git_rsl::secure_fetch_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("v6.66")).expect("could not fetch tag")); + assert_ok!( + git_rsl::secure_fetch_with_cleanup(&mut context.local, &remote, &tag), + "could not fetch tag"); } } } @@ -75,10 +97,14 @@ sequential_test! { fn error_handling() { let mut context = setup_fresh(); { - assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) - .expect("Could not rsl-init")); - let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).unwrap(); - assert_eq!(res, ()); + let remote = RemoteName::new("origin"); + let master = ReferenceName::new("master"); + + assert_ok!(git_rsl::rsl_init_with_cleanup(&mut context.local, &remote), + "Could not rsl-init"); + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master), + "failed to secure push"); let nonce_file = context.repo_dir.join(".git/NONCE"); Command::new("chmod") @@ -88,7 +114,8 @@ sequential_test! { .expect("failed to change permissions"); do_work_on_branch(&context.local, "refs/heads/master"); - let _res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).unwrap_err(); + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master) + .unwrap_err(); let head = context.local.head().unwrap().name().unwrap().to_owned(); assert_eq!(head, "refs/heads/master"); @@ -100,14 +127,20 @@ sequential_test! { fn check_rsl() { let mut context = setup_fresh(); { - assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) - .expect("Could not rsl-init")); - let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("First push failed"); - assert_eq!(res, ()); + let remote = RemoteName::new("origin"); + let master = ReferenceName::new("master"); + + assert_ok!( + git_rsl::rsl_init_with_cleanup(&mut context.local, &remote), + "Could not rsl-init"); + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master), + "First push failed"); do_work_on_branch(&context.local, "refs/heads/master"); - let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Second push failed"); - assert_eq!(res2, ()); + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master), + "Second push failed"); } } } @@ -116,19 +149,25 @@ sequential_test! { fn attack_detected_on_push() { let mut context = setup_fresh(); { - assert_eq!((), git_rsl::rsl_init_with_cleanup(&mut context.local, &RemoteName::new("origin")) - .expect("Could not rsl-init")); - let res = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("First push failed"); - assert_eq!(res, ()); + let remote = RemoteName::new("origin"); + let master = ReferenceName::new("master"); + + assert_ok!(git_rsl::rsl_init_with_cleanup(&mut context.local, &remote), + "Could not rsl-init"); + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master), + "First push failed"); do_work_on_branch(&context.local, "refs/heads/master"); - let res2 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect("Second push failed"); - assert_eq!(res2, ()); + assert_ok!( + git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master), + "Second push failed"); attack::rollback(&context.remote, "master"); do_work_on_branch(&context.local, "refs/heads/master"); - let res3 = git_rsl::secure_push_with_cleanup(&mut context.local, &RemoteName::new("origin"), &ReferenceName::new("master")).expect_err("Checking for invalid RSL detection"); + let res3 = git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &master) + .expect_err("Checking for invalid RSL detection"); assert_eq!(res3.description(), "invalid remote RSL"); } } diff --git a/tests/property_tests.rs b/tests/property_tests.rs index 26ced19..af4902a 100644 --- a/tests/property_tests.rs +++ b/tests/property_tests.rs @@ -29,11 +29,9 @@ pub fn repos(repo_count: Range) -> BoxedStrategy> { } pub fn repo() -> BoxedStrategy { - let commits = vec![ - Commit { - message: "Initial Commit".to_string(), - }, - ]; + let commits = vec![Commit { + message: "Initial Commit".to_string(), + }]; let mut branches = HashMap::new(); branches.insert("master".to_string(), Branch { commits }); From f687fb87035e8897cebc090b3ea79a74b66497ac Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Tue, 10 Jul 2018 16:19:05 -0700 Subject: [PATCH 08/11] Change to annotated tag Prior to this commit, the tag test created a lightweight tag for tagging a specific release. This commit changes the helper method and test to instead create a more informative annotated tag. --- src/utils/test_helper.rs | 5 +++-- tests/integration_test.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/utils/test_helper.rs b/src/utils/test_helper.rs index 6da38b2..6cdec66 100644 --- a/src/utils/test_helper.rs +++ b/src/utils/test_helper.rs @@ -87,12 +87,13 @@ pub fn create_file_with_text>(path: P, text: &str) -> () { file.write_all(text.as_bytes()).unwrap(); } -pub fn tag_lightweight<'repo>(repo: &'repo Repository, tag_name: &str) -> Reference<'repo> { +pub fn tag_latest_commit<'repo>(repo: &'repo Repository, tag_name: &str, tag_msg: &str) -> Reference<'repo> { let tag_target = &repo.head() .expect("failed to get head") .peel(ObjectType::Commit) .expect("failed to peel"); - let _tag_oid = repo.tag_lightweight(tag_name, tag_target, false); + let sig = repo.signature().expect("failed to get signature"); + let _tag_oid = repo.tag(tag_name, tag_target, &sig, tag_msg, false); repo.find_reference(&format!("refs/tags/{}", tag_name)) .expect("tag ref not found") } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 5abac38..9d8d4f5 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -78,7 +78,7 @@ sequential_test! { assert_ok!(git_rsl::rsl_init_with_cleanup(&mut context.local, &remote), "Could not rsl-init"); do_work_on_branch(&context.local, "refs/heads/master"); - tag_lightweight(&mut context.local, "v6.66"); + tag_latest_commit(&mut context.local, "v6.66", "coolest release ever"); assert_ok!( git_rsl::secure_push_with_cleanup(&mut context.local, &remote, &tag), From b776d8fbb204d6dac3114ce3c73331eb9f8ca2e1 Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Tue, 10 Jul 2018 16:26:11 -0700 Subject: [PATCH 09/11] Use map for converting refnames Prior to this commit, I was doing this awkwardly. This commit uses map instead. --- src/utils/git.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/utils/git.rs b/src/utils/git.rs index a440d63..5c59427 100644 --- a/src/utils/git.rs +++ b/src/utils/git.rs @@ -285,15 +285,16 @@ pub fn get_ref_from_name<'repo>(repo: &'repo Repository, name: &str) -> Option Result<()> { - let mut full_names: Vec = vec![]; - for name in ref_names { - let reference = get_ref_from_name(&repo, name).expect("failed to find reference from name"); - let full_name = String::from(reference.name().expect("failed to get full name of ref")); - full_names.push(full_name) - } + let refs: Vec = ref_names + .iter() + .map(|name: &&str| { + let reference = get_ref_from_name(&repo, name).expect("failed to find reference from name"); + String::from(reference.name().expect("failed to get full name of ref")) + }) + .collect(); let mut refspecs: Vec<&str> = vec![]; - for name in &full_names { + for name in &refs { refspecs.push(name) } From 9acbc9fbeaba02d6474a9546c3479e0cb70e868b Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Tue, 10 Jul 2018 16:31:31 -0700 Subject: [PATCH 10/11] Remove toDo's Prior to this commit, the src files for the binaries still had toDo's in them. Those toDos are still things that we need toDo, but would be better served being represented as issues in the issue tracker, so this commit removes them from the code. --- src/bin/git-rsl-init.rs | 2 +- src/bin/git-secure-fetch.rs | 2 +- src/bin/git-secure-push.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bin/git-rsl-init.rs b/src/bin/git-rsl-init.rs index f6dae6d..987b5fd 100644 --- a/src/bin/git-rsl-init.rs +++ b/src/bin/git-rsl-init.rs @@ -27,7 +27,7 @@ fn main() { None => panic!("Must supply a REMOTE argument"), Some(v) => v.to_owned(), }; - // TODO - reduce code duplication across the top level of the binaries + let mut repo = git::discover_repo() .expect("You don't appear to be in a git project. Please check yourself and try again"); diff --git a/src/bin/git-secure-fetch.rs b/src/bin/git-secure-fetch.rs index 460d935..f903252 100644 --- a/src/bin/git-secure-fetch.rs +++ b/src/bin/git-secure-fetch.rs @@ -36,7 +36,7 @@ fn main() { None => panic!("Must supply a REFERENCE argument"), Some(v) => v.to_owned(), }; - // TODO - reduce code duplication across the top level of the binaries + let mut repo = git::discover_repo() .expect("You don't appear to be in a git project. Please check yourself and try again"); diff --git a/src/bin/git-secure-push.rs b/src/bin/git-secure-push.rs index 98a8061..d40d5f0 100644 --- a/src/bin/git-secure-push.rs +++ b/src/bin/git-secure-push.rs @@ -35,7 +35,7 @@ fn main() { None => panic!("Must supply a REFERENCE argument"), Some(v) => v.to_owned(), }; - // TODO - reduce code duplication across the top level of the binaries + let mut repo = git::discover_repo() .expect("You don't appear to be in a git project. Please check yourself and try again"); From 638721c4d9ad669539dbe47f801a684c37d96e3b Mon Sep 17 00:00:00 2001 From: Katie Cleary Date: Fri, 13 Jul 2018 14:01:17 -0700 Subject: [PATCH 11/11] Add error handling to lib functions Prior to this commit, there were lots of panics in the new lib functions. This commit adds in error handling for those cases. --- src/push_entry.rs | 16 +++++++++------- src/rsl.rs | 8 ++++---- src/utils/git.rs | 17 ++++++----------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/push_entry.rs b/src/push_entry.rs index 61308c5..1c32442 100644 --- a/src/push_entry.rs +++ b/src/push_entry.rs @@ -54,17 +54,19 @@ impl PushEntry { branch_str: &str, prev: String, nonce_bag: NonceBag, - ) -> PushEntry { + ) -> Result { + // NONE OF THIS IS FINE bc this method doesn't return an error :P let full_ref = - utils::git::get_ref_from_name(repo, branch_str).expect("failed to get reference"); - let target_oid = full_ref.target().expect("failed to get target oid"); + utils::git::get_ref_from_name(repo, branch_str).ok_or("failed to get reference")?; + let target_oid = full_ref.target().ok_or("failed to get target oid")?; + let ref_name = String::from(full_ref.name().ok_or("failed to get ref's full name")?); - PushEntry { - ref_name: String::from(full_ref.name().expect("failed to get ref's full name")), + Ok(PushEntry { + ref_name: ref_name, oid: target_oid, prev_hash: prev, nonce_bag, - } + }) } pub fn prev_hash(&self) -> String { @@ -186,7 +188,7 @@ mod tests { &"master", String::from("fwjjk42ofw093j"), NonceBag::default(), - ); + ).unwrap(); let oid = repo.commit_push_entry(&entry, "refs/heads/RSL").unwrap(); assert_eq!(PushEntry::from_oid(&repo, &oid).unwrap().unwrap(), entry); diff --git a/src/rsl.rs b/src/rsl.rs index 790c0b4..2db20df 100644 --- a/src/rsl.rs +++ b/src/rsl.rs @@ -173,7 +173,7 @@ impl<'remote, 'repo> RSL<'remote, 'repo> { ref_names.first().unwrap(), prev_hash, self.nonce_bag.clone(), - ); + )?; // commit new pushentry (TODO commit to detached HEAD instead of local RSL branch, in case someone else has updated and a fastforward is not possible) self.repo @@ -363,7 +363,7 @@ impl<'repo> HasRSL<'repo> for Repository { self.commit_nonce_bag()?; // create initial bootstrapping push entry - let initial_pe = PushEntry::new(self, "RSL", String::from("First Push Entry"), nonce_bag); + let initial_pe = PushEntry::new(self, "RSL", String::from("First Push Entry"), nonce_bag)?; self.commit_push_entry(&initial_pe, "refs/heads/RSL")?; // push new rsl branch @@ -563,7 +563,7 @@ mod tests { &"master", // branch String::from("hash_of_last_pushentry"), // prev NonceBag::default(), - ); + ).unwrap(); let oid = repo.commit_push_entry(&entry, "refs/heads/RSL").unwrap(); // we are on the correct branch with new commit at the tip @@ -609,7 +609,7 @@ mod tests { // create push entry manuallly and commit it to the remote rsl branch let prev_hash = rsl.last_remote_push_entry.hash(); - let push_entry = PushEntry::new(&repo, &"master", prev_hash, rsl.nonce_bag); + let push_entry = PushEntry::new(&repo, &"master", prev_hash, rsl.nonce_bag).unwrap(); let oid = repo.commit_push_entry(&push_entry, "refs/remotes/origin/RSL") .unwrap(); diff --git a/src/utils/git.rs b/src/utils/git.rs index 5c59427..63d7246 100644 --- a/src/utils/git.rs +++ b/src/utils/git.rs @@ -285,19 +285,14 @@ pub fn get_ref_from_name<'repo>(repo: &'repo Repository, name: &str) -> Option Result<()> { - let refs: Vec = ref_names - .iter() - .map(|name: &&str| { - let reference = get_ref_from_name(&repo, name).expect("failed to find reference from name"); - String::from(reference.name().expect("failed to get full name of ref")) - }) - .collect(); - - let mut refspecs: Vec<&str> = vec![]; - for name in &refs { - refspecs.push(name) + let mut refs: Vec = vec![]; + for name in ref_names { + let reference = get_ref_from_name(&repo, name).ok_or("couldn't find reference")?; + refs.push(reference.name().ok_or("couldn't get name from reference")?.to_string()) } + let refspecs: Vec<&str> = refs.iter().map(AsRef::as_ref).collect(); + push_refspecs(repo, remote, &refspecs) }