From 871a3c054d4fe6c1e92b6f2e260b19463404509f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Aug 2022 10:50:29 +0800 Subject: [PATCH 01/36] empty `git-refspec` crate for name reservation prior to implementation (#450) --- Cargo.lock | 9 +++++++++ Cargo.toml | 1 + git-refspec/Cargo.toml | 20 ++++++++++++++++++++ git-refspec/src/lib.rs | 3 +++ git-refspec/tests/refspec.rs | 1 + 5 files changed, 34 insertions(+) create mode 100644 git-refspec/Cargo.toml create mode 100644 git-refspec/src/lib.rs create mode 100644 git-refspec/tests/refspec.rs diff --git a/Cargo.lock b/Cargo.lock index 83cf0debbe0..f69b160c236 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1484,6 +1484,15 @@ dependencies = [ "tempfile", ] +[[package]] +name = "git-refspec" +version = "0.0.0" +dependencies = [ + "bstr", + "git-testtools", + "thiserror", +] + [[package]] name = "git-repository" version = "0.20.0" diff --git a/Cargo.toml b/Cargo.toml index 145aebfa155..0b55bfb6423 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -162,6 +162,7 @@ members = [ "git-lock", "git-attributes", "git-pathspec", + "git-refspec", "git-path", "git-repository", "gitoxide-core", diff --git a/git-refspec/Cargo.toml b/git-refspec/Cargo.toml new file mode 100644 index 00000000000..0647cfe5f21 --- /dev/null +++ b/git-refspec/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "git-refspec" +version = "0.0.0" +repository = "https://github.com/Byron/gitoxide" +license = "MIT/Apache-2.0" +description = "A WIP crate of the gitoxide project for parsing and representing refspecs" +authors = ["Sebastian Thiel "] +edition = "2018" + +[lib] +doctest = false + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +bstr = { version = "0.2.13", default-features = false, features = ["std"]} +thiserror = "1.0.26" + +[dev-dependencies] +git-testtools = { path = "../tests/tools" } diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs new file mode 100644 index 00000000000..ff723740fa2 --- /dev/null +++ b/git-refspec/src/lib.rs @@ -0,0 +1,3 @@ +//! Parse [ref specs]() and represent them. +#![forbid(unsafe_code, rust_2018_idioms)] +#![deny(missing_docs)] diff --git a/git-refspec/tests/refspec.rs b/git-refspec/tests/refspec.rs new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/git-refspec/tests/refspec.rs @@ -0,0 +1 @@ + From 3383408ce22ca9c7502ad2d1fab51cf12dc5ee72 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Aug 2022 10:52:08 +0800 Subject: [PATCH 02/36] prepare git-refspec changelog prior to release (#450) --- etc/check-package-size.sh | 1 + git-refspec/CHANGELOG.md | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 git-refspec/CHANGELOG.md diff --git a/etc/check-package-size.sh b/etc/check-package-size.sh index a6117e9b250..4431725ab0a 100755 --- a/etc/check-package-size.sh +++ b/etc/check-package-size.sh @@ -18,6 +18,7 @@ echo "in root: gitoxide CLI" (enter cargo-smart-release && indent cargo diet -n --package-size-limit 95KB) (enter git-actor && indent cargo diet -n --package-size-limit 5KB) (enter git-pathspec && indent cargo diet -n --package-size-limit 25KB) +(enter git-refspec && indent cargo diet -n --package-size-limit 5KB) (enter git-path && indent cargo diet -n --package-size-limit 15KB) (enter git-attributes && indent cargo diet -n --package-size-limit 15KB) (enter git-discover && indent cargo diet -n --package-size-limit 20KB) diff --git a/git-refspec/CHANGELOG.md b/git-refspec/CHANGELOG.md new file mode 100644 index 00000000000..1bd0a78226c --- /dev/null +++ b/git-refspec/CHANGELOG.md @@ -0,0 +1,29 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## Unreleased + +Initial release for name reservation. + +### Commit Statistics + + + + - 1 commit contributed to the release. + - 0 commits where understood as [conventional](https://www.conventionalcommits.org). + - 1 unique issue was worked on: [#450](https://github.com/Byron/gitoxide/issues/450) + +### Commit Details + + + +
view details + + * **[#450](https://github.com/Byron/gitoxide/issues/450)** + - empty `git-refspec` crate for name reservation prior to implementation ([`871a3c0`](https://github.com/Byron/gitoxide/commit/871a3c054d4fe6c1e92b6f2e260b19463404509f)) +
+ From d406689f01c8fa7cc81b52d6500a44303e719ec2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Aug 2022 10:52:26 +0800 Subject: [PATCH 03/36] Release git-refspec v0.0.0 --- git-refspec/CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/git-refspec/CHANGELOG.md b/git-refspec/CHANGELOG.md index 1bd0a78226c..64f6c97c7bc 100644 --- a/git-refspec/CHANGELOG.md +++ b/git-refspec/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## 0.0.0 (2022-08-05) Initial release for name reservation. @@ -13,7 +13,7 @@ Initial release for name reservation. - - 1 commit contributed to the release. + - 2 commits contributed to the release. - 0 commits where understood as [conventional](https://www.conventionalcommits.org). - 1 unique issue was worked on: [#450](https://github.com/Byron/gitoxide/issues/450) @@ -24,6 +24,7 @@ Initial release for name reservation.
view details * **[#450](https://github.com/Byron/gitoxide/issues/450)** + - prepare git-refspec changelog prior to release ([`3383408`](https://github.com/Byron/gitoxide/commit/3383408ce22ca9c7502ad2d1fab51cf12dc5ee72)) - empty `git-refspec` crate for name reservation prior to implementation ([`871a3c0`](https://github.com/Byron/gitoxide/commit/871a3c054d4fe6c1e92b6f2e260b19463404509f))
From 362bd4651751960b3062fd1c65d58b986b46cc97 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Aug 2022 15:25:32 +0800 Subject: [PATCH 04/36] all baseline test cases from git's test-suite (#450) --- .../generated-archives/make_baseline.tar.xz | 3 + git-refspec/tests/fixtures/make_baseline.sh | 91 +++++++++++++++++++ git-refspec/tests/parse/mod.rs | 7 ++ git-refspec/tests/refspec.rs | 2 +- 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz create mode 100644 git-refspec/tests/fixtures/make_baseline.sh create mode 100644 git-refspec/tests/parse/mod.rs diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz new file mode 100644 index 00000000000..c8c94890503 --- /dev/null +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0a85fe0b4d9fb179baab26ea008ddbb48a657d4e92561c42a213269468481a66 +size 9308 diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh new file mode 100644 index 00000000000..cb9fcfc3bc6 --- /dev/null +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -0,0 +1,91 @@ +#!/bin/bash +set -eu -o pipefail + +git init; + +function baseline() { + local kind=$1 + local refspec=$2 + + cat <.git/config +[remote "test"] + url = . + $kind = "$refspec" +EOF + + git ls-remote "test" && status=0 || status=$? + { + echo "$kind" "$refspec" + echo "$status" + } >> baseline.git +} + + +# invalid + +baseline push '' +baseline push '::' +baseline fetch '::' + +baseline push 'refs/heads/*:refs/remotes/frotz' +baseline push 'refs/heads:refs/remotes/frotz/*' + +baseline fetch 'refs/heads/*:refs/remotes/frotz' +baseline fetch 'refs/heads:refs/remotes/frotz/*' +baseline fetch 'refs/heads/main::refs/remotes/frotz/xyzzy' +baseline fetch 'refs/heads/maste :refs/remotes/frotz/xyzzy' +baseline fetch 'main~1:refs/remotes/frotz/backup' +baseline fetch 'HEAD~4:refs/remotes/frotz/new' +baseline push 'refs/heads/ nitfol' +baseline fetch 'refs/heads/ nitfol' +baseline push 'HEAD:' +baseline push 'refs/heads/ nitfol:' +baseline fetch 'refs/heads/ nitfol:' +baseline push ':refs/remotes/frotz/delete me' +baseline fetch ':refs/remotes/frotz/HEAD to me' +baseline fetch 'refs/heads/*/*/for-linus:refs/remotes/mine/*' +baseline push 'refs/heads/*/*/for-linus:refs/remotes/mine/*' + +baseline fetch 'refs/heads/*g*/for-linus:refs/remotes/mine/*' +baseline push 'refs/heads/*g*/for-linus:refs/remotes/mine/*' +bad=$(printf '\011tab') +baseline fetch "refs/heads/${bad}" + +# valid +baseline push '+:' +baseline push ':' + +baseline fetch '' +baseline fetch ':' +baseline push 'refs/heads/main:refs/remotes/frotz/xyzzy' +baseline push 'refs/heads/*:refs/remotes/frotz/*' + + +baseline fetch 'refs/heads/*:refs/remotes/frotz/*' +baseline fetch 'refs/heads/main:refs/remotes/frotz/xyzzy' + +baseline push 'main~1:refs/remotes/frotz/backup' +baseline push 'HEAD~4:refs/remotes/frotz/new' + +baseline push 'HEAD' +baseline fetch 'HEAD' +baseline push '@' +baseline fetch '@' + +baseline fetch 'HEAD:' + +baseline push ':refs/remotes/frotz/deleteme' +baseline fetch ':refs/remotes/frotz/HEAD-to-me' + +baseline fetch 'refs/heads/*/for-linus:refs/remotes/mine/*-blah' +baseline push 'refs/heads/*/for-linus:refs/remotes/mine/*-blah' + +baseline fetch 'refs/heads*/for-linus:refs/remotes/mine/*' +baseline push 'refs/heads*/for-linus:refs/remotes/mine/*' + + +baseline fetch 'refs/heads/*/for-linus:refs/remotes/mine/*' +baseline push 'refs/heads/*/for-linus:refs/remotes/mine/*' + +good=$(printf '\303\204') +baseline fetch "refs/heads/${good}" diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs new file mode 100644 index 00000000000..a388d8284cb --- /dev/null +++ b/git-refspec/tests/parse/mod.rs @@ -0,0 +1,7 @@ +use git_testtools::scripted_fixture_repo_read_only; + +#[test] +#[ignore] +fn baseline() { + let _dir = scripted_fixture_repo_read_only("make_baseline.sh").unwrap(); +} diff --git a/git-refspec/tests/refspec.rs b/git-refspec/tests/refspec.rs index 8b137891791..06f1a3c69d4 100644 --- a/git-refspec/tests/refspec.rs +++ b/git-refspec/tests/refspec.rs @@ -1 +1 @@ - +mod parse; From 5c823dcbfd3aca0a8846300629e01aed8d7b7e66 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Aug 2022 16:31:42 +0800 Subject: [PATCH 05/36] sketch data structure that should do the trick (#450) That way the most complex thing will be the validation along with the matching. --- git-refspec/src/lib.rs | 56 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index ff723740fa2..af645881f1e 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -1,3 +1,57 @@ //! Parse [ref specs]() and represent them. #![forbid(unsafe_code, rust_2018_idioms)] -#![deny(missing_docs)] +#![allow(missing_docs)] + +use bstr::{BStr, BString}; + +/// The way to interpret a refspec. +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] +pub enum Mode { + /// Even though according to normal rules a non-fastforward would be denied, override this and reset a ref forcefully in the destination. + Force, + /// Instead of considering matching refs included, we consider them excluded. This applies only to the source side of a refspec. + Negative, +} + +/// What operation to perform with the refspec. +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] +pub enum Operation { + /// The `src` side is local and the `dst` side is remote. + Push, + /// The `src` side is remote and the `dst` side is local. + Fetch, +} + +/// A refspec with references to the memory it was parsed from. +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] +pub struct RefSpecRef<'a> { + mode: Mode, + op: Operation, + src: Option<&'a BStr>, + dest: Option<&'a BStr>, +} + +/// An owned refspec. +#[derive(PartialEq, Eq, Clone, Hash, Debug)] +pub struct RefSpec { + mode: Mode, + op: Operation, + src: Option, + dest: Option, +} + +mod spec { + use crate::{RefSpec, RefSpecRef}; + + impl RefSpecRef<'_> { + /// Convert this ref into a standalone, owned copy. + pub fn to_owned(&self) -> RefSpec { + RefSpec { + mode: self.mode, + op: self.op, + src: self.src.map(ToOwned::to_owned), + dest: self.dest.map(ToOwned::to_owned), + } + } + } +} From b9a4bdca41c074364b7bc26523784c35ac3196ce Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Aug 2022 17:01:20 +0800 Subject: [PATCH 06/36] frame for basic parsing (#450) --- git-refspec/src/lib.rs | 5 +++++ git-refspec/src/parse.rs | 18 ++++++++++++++++++ git-refspec/tests/parse/mod.rs | 28 ++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 git-refspec/src/parse.rs diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index af645881f1e..7c0848b06f6 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -7,6 +7,8 @@ use bstr::{BStr, BString}; /// The way to interpret a refspec. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Mode { + /// Apply standard rules for refspecs which are including refs with specific rules related to allowing fast forwards of destinations. + Normal, /// Even though according to normal rules a non-fastforward would be denied, override this and reset a ref forcefully in the destination. Force, /// Instead of considering matching refs included, we consider them excluded. This applies only to the source side of a refspec. @@ -40,6 +42,9 @@ pub struct RefSpec { dest: Option, } +pub mod parse; +pub use parse::function::parse; + mod spec { use crate::{RefSpec, RefSpecRef}; diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs new file mode 100644 index 00000000000..def942213e3 --- /dev/null +++ b/git-refspec/src/parse.rs @@ -0,0 +1,18 @@ +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("Empty refspecs are invalid")] + Empty, + #[error("Negative refspecs cannot have destinations as they exclude sources")] + NegativeWithDestination, +} + +pub(crate) mod function { + use crate::parse::Error; + use crate::{Operation, RefSpecRef}; + use bstr::BStr; + + /// Parse `spec` for use in `operation` and return it if it is valid. + pub fn parse(mut _spec: &BStr, _operation: Operation) -> Result, Error> { + todo!() + } +} diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index a388d8284cb..1eaca6d7cbc 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -5,3 +5,31 @@ use git_testtools::scripted_fixture_repo_read_only; fn baseline() { let _dir = scripted_fixture_repo_read_only("make_baseline.sh").unwrap(); } + +mod invalid { + use git_refspec::{parse, parse::Error, Operation}; + + #[test] + #[ignore] + fn empty() { + assert!(matches!(parse("".into(), Operation::Fetch).unwrap_err(), Error::Empty)); + assert!(matches!(parse("".into(), Operation::Push).unwrap_err(), Error::Empty)); + } + + #[test] + #[ignore] + fn negative_with_destination() { + assert!(matches!( + parse("^a:b".into(), Operation::Fetch).unwrap_err(), + Error::NegativeWithDestination + )); + assert!(matches!( + parse("a:b".into(), Operation::Fetch).unwrap_err(), + Error::NegativeWithDestination + )); + } + + mod fetch {} + + mod push {} +} From 9c5fed2e2a6ea388acde73be32f8b7f8687c415b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Aug 2022 21:11:18 +0800 Subject: [PATCH 07/36] first few bits of error handling in parser (#450) --- git-refspec/src/parse.rs | 26 +++++++++++++--- git-refspec/tests/fixtures/make_baseline.sh | 4 +++ git-refspec/tests/parse/mod.rs | 34 +++++++++++++-------- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index def942213e3..a437246d306 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -8,11 +8,29 @@ pub enum Error { pub(crate) mod function { use crate::parse::Error; - use crate::{Operation, RefSpecRef}; - use bstr::BStr; + use crate::{Mode, Operation, RefSpecRef}; + use bstr::{BStr, ByteSlice}; /// Parse `spec` for use in `operation` and return it if it is valid. - pub fn parse(mut _spec: &BStr, _operation: Operation) -> Result, Error> { - todo!() + pub fn parse(mut spec: &BStr, _operation: Operation) -> Result, Error> { + let mode = match spec.get(0) { + Some(&b'^') => { + spec = &spec[1..]; + Mode::Negative + } + Some(_) => Mode::Normal, + None => return Err(Error::Empty), + }; + + match spec.find_byte(b':') { + Some(pos) => { + let (_src, _dst) = spec.split_at(pos); + if mode == Mode::Negative { + return Err(Error::NegativeWithDestination); + } + todo!("with colon") + } + None => todo!("no colon"), + } } } diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh index cb9fcfc3bc6..58752d38698 100644 --- a/git-refspec/tests/fixtures/make_baseline.sh +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -26,6 +26,10 @@ EOF baseline push '' baseline push '::' baseline fetch '::' +baseline fetch '^a:' +baseline fetch '^a:b' +baseline fetch '^:' +baseline fetch '^:b' baseline push 'refs/heads/*:refs/remotes/frotz' baseline push 'refs/heads:refs/remotes/frotz/*' diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 1eaca6d7cbc..d53b2ecf22c 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -7,29 +7,37 @@ fn baseline() { } mod invalid { - use git_refspec::{parse, parse::Error, Operation}; + use crate::parse::try_parse; + use git_refspec::{parse::Error, Operation}; #[test] - #[ignore] fn empty() { - assert!(matches!(parse("".into(), Operation::Fetch).unwrap_err(), Error::Empty)); - assert!(matches!(parse("".into(), Operation::Push).unwrap_err(), Error::Empty)); + assert!(matches!(try_parse("", Operation::Fetch).unwrap_err(), Error::Empty)); + assert!(matches!(try_parse("", Operation::Push).unwrap_err(), Error::Empty)); } #[test] - #[ignore] fn negative_with_destination() { - assert!(matches!( - parse("^a:b".into(), Operation::Fetch).unwrap_err(), - Error::NegativeWithDestination - )); - assert!(matches!( - parse("a:b".into(), Operation::Fetch).unwrap_err(), - Error::NegativeWithDestination - )); + for op in [Operation::Fetch, Operation::Push] { + for spec in ["^a:b", "^a:", "^:", "^:b"] { + assert!(matches!( + try_parse(spec, op).unwrap_err(), + Error::NegativeWithDestination + )); + } + } } mod fetch {} mod push {} } + +mod util { + use git_refspec::{Operation, RefSpecRef}; + + pub fn try_parse(spec: &str, op: Operation) -> Result, git_refspec::parse::Error> { + git_refspec::parse(spec.into(), op) + } +} +pub use util::*; From 5e4ee9ba422cac9eef2b558746f3a3aa4b67a5e4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 08:42:02 +0800 Subject: [PATCH 08/36] run the baseline test and gather some information (#450) --- .../generated-archives/make_baseline.tar.xz | 4 +- git-refspec/tests/parse/mod.rs | 44 ++++++++++++++++++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz index c8c94890503..82ac607fae7 100644 --- a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:0a85fe0b4d9fb179baab26ea008ddbb48a657d4e92561c42a213269468481a66 -size 9308 +oid sha256:5aac946f32882b76ca1a1d980cdd56fb812e9059ba59074ef141dd0bb4b32903 +size 9320 diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index d53b2ecf22c..3818814936f 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -1,9 +1,49 @@ +use bstr::ByteSlice; +use git_refspec::Operation; use git_testtools::scripted_fixture_repo_read_only; +use std::panic::catch_unwind; #[test] -#[ignore] +#[should_panic] fn baseline() { - let _dir = scripted_fixture_repo_read_only("make_baseline.sh").unwrap(); + let dir = scripted_fixture_repo_read_only("make_baseline.sh").unwrap(); + let baseline = std::fs::read(dir.join("baseline.git")).unwrap(); + let mut lines = baseline.lines(); + let mut panics = 0; + let mut mismatch = 0; + let mut count = 0; + while let Some(kind_spec) = lines.next() { + count += 1; + let (kind, spec) = kind_spec.split_at(kind_spec.find_byte(b' ').expect("space between kind and spec")); + let err_code: usize = lines + .next() + .expect("err code") + .to_str() + .unwrap() + .parse() + .expect("number"); + let op = match kind { + b"fetch" => Operation::Fetch, + b"push" => Operation::Push, + _ => unreachable!("{} unexpected", kind.as_bstr()), + }; + let res = catch_unwind(|| try_parse(spec.to_str().unwrap(), op)); + match res { + Ok(res) => match (res.is_ok(), err_code == 0) { + (true, true) | (false, false) => {} + _ => mismatch += 1, + }, + Err(_) => { + panics += 1; + } + } + } + if panics != 0 || mismatch != 0 { + panic!( + "Out of {} baseline entries, got {} mismatches and {} panics", + count, mismatch, panics + ); + } } mod invalid { From 0ba1b73bf988357f4b27753b87432618edec697a Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 10:38:46 +0800 Subject: [PATCH 09/36] sketch all possible instructions of fetch and push refspecs (#450) --- git-refspec/src/lib.rs | 93 ++++++++++++++++++- .../generated-archives/make_baseline.tar.xz | 4 +- git-refspec/tests/fixtures/make_baseline.sh | 3 + git-refspec/tests/parse/mod.rs | 18 +++- 4 files changed, 113 insertions(+), 5 deletions(-) diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index 7c0848b06f6..6d0f7b32bd1 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -24,6 +24,83 @@ pub enum Operation { Fetch, } +pub enum Instruction<'a> { + Push(Push<'a>), + Fetch(Fetch<'a>), +} + +pub enum Push<'a> { + /// Push a single ref knowing only one ref name. + SingleMatching { + /// The name of the ref to push from `src` to `dest`. + src_and_dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, + /// Exclude a single ref. + ExcludeSingle { + /// A single full ref name to exclude. + src: &'a BStr, + }, + /// Exclude multiple refs with single `*` glob. + ExcludeMultipleWithGlob { + /// A ref pattern with a single `*`. + src: &'a BStr, + }, + /// Push a single ref or refspec to a known destination ref. + Single { + /// The source ref or refspec to push. + src: &'a BStr, + /// The ref to update with the object from `src`. + dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, + /// Push a multiple refs to matching destination refs, with exactly a single glob on both sides. + MultipleWithGlob { + /// The source ref to match against all refs for pushing. + src: &'a BStr, + /// The ref to update with object obtained from `src`, filling in the `*` with the portion that matched in `src`. + dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, +} + +pub enum Fetch<'a> { + Only { + /// The ref name to fetch on the remote side, without updating the local side. + src: &'a BStr, + }, + /// Exclude a single ref. + ExcludeSingle { + /// A single full ref name to exclude. + src: &'a BStr, + }, + /// Exclude multiple refs with single `*` glob. + ExcludeMultipleWithGlob { + /// A ref pattern with a single `*`. + src: &'a BStr, + }, + AndUpdateSingle { + /// The ref name to fetch on the remote side. + src: &'a BStr, + /// The local destination to update with what was fetched. + dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, + /// Similar to `FetchAndUpdate`, but src and destination contain a single glob to fetch and update multiple refs. + AndUpdateMultipleWithGlob { + /// The ref glob to match against all refs on the remote side for fetching. + src: &'a BStr, + /// The local destination to update with what was fetched by replacing the single `*` with the matching portion from `src`. + dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, +} + /// A refspec with references to the memory it was parsed from. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub struct RefSpecRef<'a> { @@ -46,8 +123,22 @@ pub mod parse; pub use parse::function::parse; mod spec { - use crate::{RefSpec, RefSpecRef}; + use crate::{Instruction, Mode, RefSpec, RefSpecRef}; + + /// Access + impl RefSpecRef<'_> { + /// Return the refspec mode. + pub fn mode(&self) -> Mode { + self.mode + } + + /// Transform the state of the refspec into an instruction making clear what to do with it. + pub fn instruction(&self) -> Instruction<'_> { + todo!() + } + } + /// Conversion impl RefSpecRef<'_> { /// Convert this ref into a standalone, owned copy. pub fn to_owned(&self) -> RefSpec { diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz index 82ac607fae7..6ec8641a7a1 100644 --- a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:5aac946f32882b76ca1a1d980cdd56fb812e9059ba59074ef141dd0bb4b32903 -size 9320 +oid sha256:588dd4f20c618e60aff39c9cd47dd1300c83c39572f0c1db2c751ccbaca5709c +size 9328 diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh index 58752d38698..68d28e37e00 100644 --- a/git-refspec/tests/fixtures/make_baseline.sh +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -31,6 +31,9 @@ baseline fetch '^a:b' baseline fetch '^:' baseline fetch '^:b' +baseline fetch '^refs/heads/qa/*/*' +baseline push '^refs/heads/qa/*/*' + baseline push 'refs/heads/*:refs/remotes/frotz' baseline push 'refs/heads:refs/remotes/frotz/*' diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 3818814936f..b5b3bb11729 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -70,14 +70,28 @@ mod invalid { mod fetch {} - mod push {} + mod push { + use crate::parse::assert_parse; + use git_refspec::{Mode, Operation}; + + #[test] + #[ignore] + fn colon_alone_is_for_pushing_matching_refs() { + assert_parse(":", Operation::Push, None, None, Mode::Normal); + } + } } mod util { - use git_refspec::{Operation, RefSpecRef}; + use git_refspec::{Mode, Operation, RefSpecRef}; pub fn try_parse(spec: &str, op: Operation) -> Result, git_refspec::parse::Error> { git_refspec::parse(spec.into(), op) } + + pub fn assert_parse(spec: &str, op: Operation, _src: Option<&str>, _dest: Option<&str>, mode: Mode) { + let spec = try_parse(spec, op).expect("no error"); + assert_eq!(spec.mode(), mode); + } } pub use util::*; From 6713793dca4054a7f8717e70c8e9e4b7e625e9b4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 10:43:58 +0800 Subject: [PATCH 10/36] refactor (#450) --- git-refspec/src/lib.rs | 112 +++------------------------------------ git-refspec/src/types.rs | 98 ++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 104 deletions(-) create mode 100644 git-refspec/src/types.rs diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index 6d0f7b32bd1..3245571277d 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -2,112 +2,16 @@ #![forbid(unsafe_code, rust_2018_idioms)] #![allow(missing_docs)] -use bstr::{BStr, BString}; - -/// The way to interpret a refspec. -#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] -pub enum Mode { - /// Apply standard rules for refspecs which are including refs with specific rules related to allowing fast forwards of destinations. - Normal, - /// Even though according to normal rules a non-fastforward would be denied, override this and reset a ref forcefully in the destination. - Force, - /// Instead of considering matching refs included, we consider them excluded. This applies only to the source side of a refspec. - Negative, -} - -/// What operation to perform with the refspec. -#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] -pub enum Operation { - /// The `src` side is local and the `dst` side is remote. - Push, - /// The `src` side is remote and the `dst` side is local. - Fetch, -} - -pub enum Instruction<'a> { - Push(Push<'a>), - Fetch(Fetch<'a>), -} - -pub enum Push<'a> { - /// Push a single ref knowing only one ref name. - SingleMatching { - /// The name of the ref to push from `src` to `dest`. - src_and_dest: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, - /// Exclude a single ref. - ExcludeSingle { - /// A single full ref name to exclude. - src: &'a BStr, - }, - /// Exclude multiple refs with single `*` glob. - ExcludeMultipleWithGlob { - /// A ref pattern with a single `*`. - src: &'a BStr, - }, - /// Push a single ref or refspec to a known destination ref. - Single { - /// The source ref or refspec to push. - src: &'a BStr, - /// The ref to update with the object from `src`. - dest: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, - /// Push a multiple refs to matching destination refs, with exactly a single glob on both sides. - MultipleWithGlob { - /// The source ref to match against all refs for pushing. - src: &'a BStr, - /// The ref to update with object obtained from `src`, filling in the `*` with the portion that matched in `src`. - dest: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, -} - -pub enum Fetch<'a> { - Only { - /// The ref name to fetch on the remote side, without updating the local side. - src: &'a BStr, - }, - /// Exclude a single ref. - ExcludeSingle { - /// A single full ref name to exclude. - src: &'a BStr, - }, - /// Exclude multiple refs with single `*` glob. - ExcludeMultipleWithGlob { - /// A ref pattern with a single `*`. - src: &'a BStr, - }, - AndUpdateSingle { - /// The ref name to fetch on the remote side. - src: &'a BStr, - /// The local destination to update with what was fetched. - dest: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, - /// Similar to `FetchAndUpdate`, but src and destination contain a single glob to fetch and update multiple refs. - AndUpdateMultipleWithGlob { - /// The ref glob to match against all refs on the remote side for fetching. - src: &'a BStr, - /// The local destination to update with what was fetched by replacing the single `*` with the matching portion from `src`. - dest: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, -} +pub mod parse; +pub use parse::function::parse; /// A refspec with references to the memory it was parsed from. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub struct RefSpecRef<'a> { mode: Mode, op: Operation, - src: Option<&'a BStr>, - dest: Option<&'a BStr>, + src: Option<&'a bstr::BStr>, + dest: Option<&'a bstr::BStr>, } /// An owned refspec. @@ -115,12 +19,12 @@ pub struct RefSpecRef<'a> { pub struct RefSpec { mode: Mode, op: Operation, - src: Option, - dest: Option, + src: Option, + dest: Option, } -pub mod parse; -pub use parse::function::parse; +mod types; +pub use types::{Instruction, Mode, Operation}; mod spec { use crate::{Instruction, Mode, RefSpec, RefSpecRef}; diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs new file mode 100644 index 00000000000..f15ec9240fc --- /dev/null +++ b/git-refspec/src/types.rs @@ -0,0 +1,98 @@ +use bstr::BStr; + +/// The way to interpret a refspec. +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] +pub enum Mode { + /// Apply standard rules for refspecs which are including refs with specific rules related to allowing fast forwards of destinations. + Normal, + /// Even though according to normal rules a non-fastforward would be denied, override this and reset a ref forcefully in the destination. + Force, + /// Instead of considering matching refs included, we consider them excluded. This applies only to the source side of a refspec. + Negative, +} + +/// What operation to perform with the refspec. +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] +pub enum Operation { + /// The `src` side is local and the `dst` side is remote. + Push, + /// The `src` side is remote and the `dst` side is local. + Fetch, +} + +pub enum Instruction<'a> { + Push(Push<'a>), + Fetch(Fetch<'a>), +} + +pub enum Push<'a> { + /// Push a single ref knowing only one ref name. + SingleMatching { + /// The name of the ref to push from `src` to `dest`. + src_and_dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, + /// Exclude a single ref. + ExcludeSingle { + /// A single full ref name to exclude. + src: &'a BStr, + }, + /// Exclude multiple refs with single `*` glob. + ExcludeMultipleWithGlob { + /// A ref pattern with a single `*`. + src: &'a BStr, + }, + /// Push a single ref or refspec to a known destination ref. + Single { + /// The source ref or refspec to push. + src: &'a BStr, + /// The ref to update with the object from `src`. + dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, + /// Push a multiple refs to matching destination refs, with exactly a single glob on both sides. + MultipleWithGlob { + /// The source ref to match against all refs for pushing. + src: &'a BStr, + /// The ref to update with object obtained from `src`, filling in the `*` with the portion that matched in `src`. + dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, +} + +pub enum Fetch<'a> { + Only { + /// The ref name to fetch on the remote side, without updating the local side. + src: &'a BStr, + }, + /// Exclude a single ref. + ExcludeSingle { + /// A single full ref name to exclude. + src: &'a BStr, + }, + /// Exclude multiple refs with single `*` glob. + ExcludeMultipleWithGlob { + /// A ref pattern with a single `*`. + src: &'a BStr, + }, + AndUpdateSingle { + /// The ref name to fetch on the remote side. + src: &'a BStr, + /// The local destination to update with what was fetched. + dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, + /// Similar to `FetchAndUpdate`, but src and destination contain a single glob to fetch and update multiple refs. + AndUpdateMultipleWithGlob { + /// The ref glob to match against all refs on the remote side for fetching. + src: &'a BStr, + /// The local destination to update with what was fetched by replacing the single `*` with the matching portion from `src`. + dest: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, +} From 3f264afda02235dbcdf712d957e37c71ce749f01 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 11:06:50 +0800 Subject: [PATCH 11/36] sort out how expectations can be expressed in test suite (#450) Some possible states are still missing though, like deletion in pushes. --- git-refspec/src/lib.rs | 34 ++---------------- git-refspec/src/spec.rs | 63 ++++++++++++++++++++++++++++++++++ git-refspec/src/types.rs | 20 ++++++++--- git-refspec/tests/parse/mod.rs | 21 ++++++++---- 4 files changed, 96 insertions(+), 42 deletions(-) create mode 100644 git-refspec/src/spec.rs diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index 3245571277d..2d38f3931b9 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -23,35 +23,7 @@ pub struct RefSpec { dest: Option, } -mod types; -pub use types::{Instruction, Mode, Operation}; - -mod spec { - use crate::{Instruction, Mode, RefSpec, RefSpecRef}; - - /// Access - impl RefSpecRef<'_> { - /// Return the refspec mode. - pub fn mode(&self) -> Mode { - self.mode - } +mod spec; - /// Transform the state of the refspec into an instruction making clear what to do with it. - pub fn instruction(&self) -> Instruction<'_> { - todo!() - } - } - - /// Conversion - impl RefSpecRef<'_> { - /// Convert this ref into a standalone, owned copy. - pub fn to_owned(&self) -> RefSpec { - RefSpec { - mode: self.mode, - op: self.op, - src: self.src.map(ToOwned::to_owned), - dest: self.dest.map(ToOwned::to_owned), - } - } - } -} +mod types; +pub use types::{Fetch, Instruction, Mode, Operation, Push}; diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs new file mode 100644 index 00000000000..d9488d2b683 --- /dev/null +++ b/git-refspec/src/spec.rs @@ -0,0 +1,63 @@ +use crate::types::Push; +use crate::{Instruction, Mode, Operation, RefSpec, RefSpecRef}; +use bstr::BStr; + +/// Access +impl RefSpecRef<'_> { + /// Return the refspec mode. + pub fn mode(&self) -> Mode { + self.mode + } + + /// Transform the state of the refspec into an instruction making clear what to do with it. + pub fn instruction(&self) -> Instruction<'_> { + fn has_pattern(item: &BStr) -> bool { + item.contains(&b'*') + } + match (self.op, self.mode, self.src, self.dest) { + (Operation::Push, Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Single { + src, + dest: src, + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }), + (Operation::Push, Mode::Normal | Mode::Force, None, None) => Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }), + (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dest)) if has_pattern(src) => { + Instruction::Push(Push::MultipleWithGlob { + src, + dest, + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }) + } + (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dest)) => Instruction::Push(Push::Single { + src, + dest, + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }), + (Operation::Push, Mode::Negative, Some(src), None) if has_pattern(src) => { + Instruction::Push(Push::ExcludeMultipleWithGlob { src }) + } + (Operation::Push, Mode::Negative, Some(src), None) => Instruction::Push(Push::ExcludeSingle { src }), + (op, mode, src, dest) => { + unreachable!( + "BUG: instructions with {:?} {:?} {:?} {:?} are not possible", + op, mode, src, dest + ) + } + } + } +} + +/// Conversion +impl RefSpecRef<'_> { + /// Convert this ref into a standalone, owned copy. + pub fn to_owned(&self) -> RefSpec { + RefSpec { + mode: self.mode, + op: self.op, + src: self.src.map(ToOwned::to_owned), + dest: self.dest.map(ToOwned::to_owned), + } + } +} diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index f15ec9240fc..ac129f75cf2 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -20,17 +20,17 @@ pub enum Operation { Fetch, } +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Instruction<'a> { Push(Push<'a>), Fetch(Fetch<'a>), } +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Push<'a> { - /// Push a single ref knowing only one ref name. - SingleMatching { - /// The name of the ref to push from `src` to `dest`. - src_and_dest: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. + /// Push all local branches to the matching destination on the remote, which has to exist to be updated. + AllMatchingBranches { + /// If true, allow non-fast-forward updates of the matched destination branch. allow_non_fast_forward: bool, }, /// Exclude a single ref. @@ -63,6 +63,7 @@ pub enum Push<'a> { }, } +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Fetch<'a> { Only { /// The ref name to fetch on the remote side, without updating the local side. @@ -96,3 +97,12 @@ pub enum Fetch<'a> { allow_non_fast_forward: bool, }, } + +impl Instruction<'_> { + pub fn operation(&self) -> Operation { + match self { + Instruction::Push(_) => Operation::Push, + Instruction::Fetch(_) => Operation::Fetch, + } + } +} diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index b5b3bb11729..4948708e338 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -72,26 +72,35 @@ mod invalid { mod push { use crate::parse::assert_parse; - use git_refspec::{Mode, Operation}; + use git_refspec::{Instruction, Push}; #[test] #[ignore] fn colon_alone_is_for_pushing_matching_refs() { - assert_parse(":", Operation::Push, None, None, Mode::Normal); + assert_parse( + ":", + Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: false, + }), + ); } } } mod util { - use git_refspec::{Mode, Operation, RefSpecRef}; + use git_refspec::{Instruction, Operation, RefSpecRef}; + + // pub fn b(input: &str) -> &bstr::BStr { + // input.into() + // } pub fn try_parse(spec: &str, op: Operation) -> Result, git_refspec::parse::Error> { git_refspec::parse(spec.into(), op) } - pub fn assert_parse(spec: &str, op: Operation, _src: Option<&str>, _dest: Option<&str>, mode: Mode) { - let spec = try_parse(spec, op).expect("no error"); - assert_eq!(spec.mode(), mode); + pub fn assert_parse(spec: &str, expected: Instruction<'_>) { + let spec = try_parse(spec, expected.operation()).expect("no error"); + assert_eq!(spec.instruction(), expected) } } pub use util::*; From 6e5bd5c152403d76ba1cf3da2b984689cb6fe8c5 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 13:20:27 +0800 Subject: [PATCH 12/36] first successful test for returning a refspec. (#450) It's the simplest possible one, but it shows the test framework is up to the task now so it can be test-driven. We should be able to construct a test for each possible instruction and eventually pass all tests, including the baseline ones. --- git-refspec/src/lib.rs | 4 +-- git-refspec/src/parse.rs | 28 +++++++++++++++---- git-refspec/src/spec.rs | 14 +++++----- git-refspec/src/types.rs | 13 ++++++--- git-refspec/tests/parse/mod.rs | 7 ++++- .../src/repository/revision/resolve.rs | 4 +-- 6 files changed, 49 insertions(+), 21 deletions(-) diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index 2d38f3931b9..3da9be64f14 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -11,7 +11,7 @@ pub struct RefSpecRef<'a> { mode: Mode, op: Operation, src: Option<&'a bstr::BStr>, - dest: Option<&'a bstr::BStr>, + dst: Option<&'a bstr::BStr>, } /// An owned refspec. @@ -20,7 +20,7 @@ pub struct RefSpec { mode: Mode, op: Operation, src: Option, - dest: Option, + dst: Option, } mod spec; diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index a437246d306..b736abfa2d9 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -12,25 +12,43 @@ pub(crate) mod function { use bstr::{BStr, ByteSlice}; /// Parse `spec` for use in `operation` and return it if it is valid. - pub fn parse(mut spec: &BStr, _operation: Operation) -> Result, Error> { + pub fn parse(mut spec: &BStr, operation: Operation) -> Result, Error> { let mode = match spec.get(0) { Some(&b'^') => { spec = &spec[1..]; Mode::Negative } + Some(&b'+') => { + spec = &spec[1..]; + Mode::Force + } Some(_) => Mode::Normal, None => return Err(Error::Empty), }; - match spec.find_byte(b':') { + let (src, dst) = match spec.find_byte(b':') { Some(pos) => { - let (_src, _dst) = spec.split_at(pos); + let (src, dst) = spec.split_at(pos); + let dst = &dst[1..]; if mode == Mode::Negative { return Err(Error::NegativeWithDestination); } - todo!("with colon") + let src = (!src.is_empty()).then(|| src.as_bstr()); + let dst = (!dst.is_empty()).then(|| dst.as_bstr()); + match (src, dst) { + (None, None) => {} + _ => todo!("src or dst handling"), + }; + (src, dst) } None => todo!("no colon"), - } + }; + + Ok(RefSpecRef { + op: operation, + mode, + src, + dst, + }) } } diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index d9488d2b683..161f0fd9192 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -14,25 +14,25 @@ impl RefSpecRef<'_> { fn has_pattern(item: &BStr) -> bool { item.contains(&b'*') } - match (self.op, self.mode, self.src, self.dest) { + match (self.op, self.mode, self.src, self.dst) { (Operation::Push, Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Single { src, - dest: src, + dst: src, allow_non_fast_forward: matches!(self.mode, Mode::Force), }), (Operation::Push, Mode::Normal | Mode::Force, None, None) => Instruction::Push(Push::AllMatchingBranches { allow_non_fast_forward: matches!(self.mode, Mode::Force), }), - (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dest)) if has_pattern(src) => { + (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => { Instruction::Push(Push::MultipleWithGlob { src, - dest, + dst, allow_non_fast_forward: matches!(self.mode, Mode::Force), }) } - (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dest)) => Instruction::Push(Push::Single { + (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Push(Push::Single { src, - dest, + dst, allow_non_fast_forward: matches!(self.mode, Mode::Force), }), (Operation::Push, Mode::Negative, Some(src), None) if has_pattern(src) => { @@ -57,7 +57,7 @@ impl RefSpecRef<'_> { mode: self.mode, op: self.op, src: self.src.map(ToOwned::to_owned), - dest: self.dest.map(ToOwned::to_owned), + dst: self.dst.map(ToOwned::to_owned), } } } diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index ac129f75cf2..6ca621aacf7 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -33,6 +33,11 @@ pub enum Push<'a> { /// If true, allow non-fast-forward updates of the matched destination branch. allow_non_fast_forward: bool, }, + /// Delete the destination ref or glob pattern, with only a single `*` allowed. + Delete { + /// The reference or pattern to delete on the remote. + ref_or_pattern: &'a BStr, + }, /// Exclude a single ref. ExcludeSingle { /// A single full ref name to exclude. @@ -48,7 +53,7 @@ pub enum Push<'a> { /// The source ref or refspec to push. src: &'a BStr, /// The ref to update with the object from `src`. - dest: &'a BStr, + dst: &'a BStr, /// If true, allow non-fast-forward updates of `dest`. allow_non_fast_forward: bool, }, @@ -57,7 +62,7 @@ pub enum Push<'a> { /// The source ref to match against all refs for pushing. src: &'a BStr, /// The ref to update with object obtained from `src`, filling in the `*` with the portion that matched in `src`. - dest: &'a BStr, + dst: &'a BStr, /// If true, allow non-fast-forward updates of `dest`. allow_non_fast_forward: bool, }, @@ -83,7 +88,7 @@ pub enum Fetch<'a> { /// The ref name to fetch on the remote side. src: &'a BStr, /// The local destination to update with what was fetched. - dest: &'a BStr, + dst: &'a BStr, /// If true, allow non-fast-forward updates of `dest`. allow_non_fast_forward: bool, }, @@ -92,7 +97,7 @@ pub enum Fetch<'a> { /// The ref glob to match against all refs on the remote side for fetching. src: &'a BStr, /// The local destination to update with what was fetched by replacing the single `*` with the matching portion from `src`. - dest: &'a BStr, + dst: &'a BStr, /// If true, allow non-fast-forward updates of `dest`. allow_non_fast_forward: bool, }, diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 4948708e338..35ae4b3d54d 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -75,7 +75,6 @@ mod invalid { use git_refspec::{Instruction, Push}; #[test] - #[ignore] fn colon_alone_is_for_pushing_matching_refs() { assert_parse( ":", @@ -83,6 +82,12 @@ mod invalid { allow_non_fast_forward: false, }), ); + assert_parse( + "+:", + Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: true, + }), + ); } } } diff --git a/gitoxide-core/src/repository/revision/resolve.rs b/gitoxide-core/src/repository/revision/resolve.rs index d58851715c9..e20097f7e79 100644 --- a/gitoxide-core/src/repository/revision/resolve.rs +++ b/gitoxide-core/src/repository/revision/resolve.rs @@ -7,7 +7,7 @@ pub struct Options { } pub(crate) mod function { - use anyhow::{bail, Context}; + use anyhow::Context; use std::ffi::OsString; use git_repository as git; @@ -45,7 +45,7 @@ pub(crate) mod function { #[cfg(feature = "serde1")] OutputFormat::Json => { if explain { - bail!("Explanations are only for human consumption") + anyhow::bail!("Explanations are only for human consumption") } serde_json::to_writer_pretty( &mut out, From 701d46f020db5c5f86a0184ff345f30d077be8ed Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 14:04:48 +0800 Subject: [PATCH 13/36] add include directive (#450) --- git-refspec/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/git-refspec/Cargo.toml b/git-refspec/Cargo.toml index 0647cfe5f21..caac3992c06 100644 --- a/git-refspec/Cargo.toml +++ b/git-refspec/Cargo.toml @@ -6,6 +6,7 @@ license = "MIT/Apache-2.0" description = "A WIP crate of the gitoxide project for parsing and representing refspecs" authors = ["Sebastian Thiel "] edition = "2018" +include = ["src/**/*", "CHANGELOG.md", "README.md"] [lib] doctest = false From 966a9e93b3afdcdc15af95c9fa3037d71af6e0ee Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 14:41:31 +0800 Subject: [PATCH 14/36] support for deletion (#450) Even though for now everything is without validation --- git-refspec/src/parse.rs | 9 ++- git-refspec/src/spec.rs | 17 +++- git-refspec/src/types.rs | 5 ++ .../generated-archives/make_baseline.tar.xz | 4 +- git-refspec/tests/fixtures/make_baseline.sh | 6 ++ git-refspec/tests/parse/mod.rs | 80 ++++++++++++++++--- 6 files changed, 104 insertions(+), 17 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index b736abfa2d9..e8b85575660 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -36,10 +36,13 @@ pub(crate) mod function { let src = (!src.is_empty()).then(|| src.as_bstr()); let dst = (!dst.is_empty()).then(|| dst.as_bstr()); match (src, dst) { - (None, None) => {} + (None, None) => (None, None), // match all + (None, Some(dst)) => match operation { + Operation::Push => (None, Some(dst)), + Operation::Fetch => (Some("HEAD".into()), Some(dst)), + }, _ => todo!("src or dst handling"), - }; - (src, dst) + } } None => todo!("no colon"), }; diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index 161f0fd9192..4215a7cd124 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -1,5 +1,5 @@ use crate::types::Push; -use crate::{Instruction, Mode, Operation, RefSpec, RefSpecRef}; +use crate::{Fetch, Instruction, Mode, Operation, RefSpec, RefSpecRef}; use bstr::BStr; /// Access @@ -15,14 +15,29 @@ impl RefSpecRef<'_> { item.contains(&b'*') } match (self.op, self.mode, self.src, self.dst) { + (Operation::Fetch, Mode::Normal | Mode::Force, Some(src), Some(dst)) => { + Instruction::Fetch(Fetch::AndUpdateSingle { + src, + dst, + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }) + } (Operation::Push, Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Single { src, dst: src, allow_non_fast_forward: matches!(self.mode, Mode::Force), }), + (Operation::Push, Mode::Normal | Mode::Force, None, Some(dst)) => { + Instruction::Push(Push::Delete { ref_or_pattern: dst }) + } (Operation::Push, Mode::Normal | Mode::Force, None, None) => Instruction::Push(Push::AllMatchingBranches { allow_non_fast_forward: matches!(self.mode, Mode::Force), }), + (Operation::Fetch, Mode::Normal | Mode::Force, None, None) => { + Instruction::Fetch(Fetch::AllMatchingBranches { + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }) + } (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => { Instruction::Push(Push::MultipleWithGlob { src, diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 6ca621aacf7..9588cafbc6d 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -70,6 +70,11 @@ pub enum Push<'a> { #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Fetch<'a> { + /// TODO: figure out what this actually does - it's valid for sure and only fetches HEAD -> FETCH_HEAD apparently + AllMatchingBranches { + /// Unclear what this does, but it's allowed + allow_non_fast_forward: bool, + }, Only { /// The ref name to fetch on the remote side, without updating the local side. src: &'a BStr, diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz index 6ec8641a7a1..73c87d7d28c 100644 --- a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:588dd4f20c618e60aff39c9cd47dd1300c83c39572f0c1db2c751ccbaca5709c -size 9328 +oid sha256:515045d1eade39e1aeee1f171216ac389b8adc54af541b2ff49bf1b1ab68eae2 +size 9340 diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh index 68d28e37e00..a87d40cbf14 100644 --- a/git-refspec/tests/fixtures/make_baseline.sh +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -84,6 +84,12 @@ baseline fetch 'HEAD:' baseline push ':refs/remotes/frotz/deleteme' baseline fetch ':refs/remotes/frotz/HEAD-to-me' +baseline push ':a' +baseline push '+:a' + +baseline fetch ':a' +baseline fetch '+:a' + baseline fetch 'refs/heads/*/for-linus:refs/remotes/mine/*-blah' baseline push 'refs/heads/*/for-linus:refs/remotes/mine/*-blah' diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 35ae4b3d54d..1cb3f76f727 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -31,7 +31,10 @@ fn baseline() { match res { Ok(res) => match (res.is_ok(), err_code == 0) { (true, true) | (false, false) => {} - _ => mismatch += 1, + _ => { + eprintln!("{res:?} {err_code}"); + mismatch += 1; + } }, Err(_) => { panics += 1; @@ -40,8 +43,11 @@ fn baseline() { } if panics != 0 || mismatch != 0 { panic!( - "Out of {} baseline entries, got {} mismatches and {} panics", - count, mismatch, panics + "Out of {} baseline entries, got {} right, ({} mismatches and {} panics)", + count, + count - (mismatch + panics), + mismatch, + panics ); } } @@ -68,11 +74,51 @@ mod invalid { } } - mod fetch {} + mod fetch { + use crate::parse::{assert_parse, b}; + use git_refspec::{Fetch, Instruction}; + + #[test] + fn empty_lhs_colon_rhs_fetches_head_to_destination() { + assert_parse( + ":a", + Instruction::Fetch(Fetch::AndUpdateSingle { + src: b("HEAD"), + dst: b("a"), + allow_non_fast_forward: false, + }), + ); + + assert_parse( + "+:a", + Instruction::Fetch(Fetch::AndUpdateSingle { + src: b("HEAD"), + dst: b("a"), + allow_non_fast_forward: true, + }), + ); + } + + #[test] + fn colon_alone_is_for_fetching_into_fetchhead() { + assert_parse( + ":", + Instruction::Fetch(Fetch::AllMatchingBranches { + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+:", + Instruction::Fetch(Fetch::AllMatchingBranches { + allow_non_fast_forward: true, + }), + ); + } + } mod push { - use crate::parse::assert_parse; - use git_refspec::{Instruction, Push}; + use crate::parse::{assert_parse, b}; + use git_refspec::{Instruction, Mode, Push}; #[test] fn colon_alone_is_for_pushing_matching_refs() { @@ -89,23 +135,35 @@ mod invalid { }), ); } + + #[test] + fn delete() { + assert_parse(":a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); + let spec = assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); + assert_eq!( + spec.mode(), + Mode::Force, + "force is set, even though it has no effect in the actual instruction" + ); + } } } mod util { use git_refspec::{Instruction, Operation, RefSpecRef}; - // pub fn b(input: &str) -> &bstr::BStr { - // input.into() - // } + pub fn b(input: &str) -> &bstr::BStr { + input.into() + } pub fn try_parse(spec: &str, op: Operation) -> Result, git_refspec::parse::Error> { git_refspec::parse(spec.into(), op) } - pub fn assert_parse(spec: &str, expected: Instruction<'_>) { + pub fn assert_parse<'a>(spec: &'a str, expected: Instruction<'_>) -> RefSpecRef<'a> { let spec = try_parse(spec, expected.operation()).expect("no error"); - assert_eq!(spec.instruction(), expected) + assert_eq!(spec.instruction(), expected); + spec } } pub use util::*; From 7afebb778b93611d924843c95acfd6b36f284fb2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 14:54:03 +0800 Subject: [PATCH 15/36] handle colon and empty on the right side (#450) --- git-refspec/src/parse.rs | 9 +- git-refspec/src/spec.rs | 1 + git-refspec/tests/parse/mod.rs | 155 ++++++++++++++++++--------------- 3 files changed, 96 insertions(+), 69 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index e8b85575660..19545d5cb66 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -4,6 +4,8 @@ pub enum Error { Empty, #[error("Negative refspecs cannot have destinations as they exclude sources")] NegativeWithDestination, + #[error("Cannot push into an empty destination")] + PushToEmpty, } pub(crate) mod function { @@ -33,14 +35,19 @@ pub(crate) mod function { if mode == Mode::Negative { return Err(Error::NegativeWithDestination); } + let src = (!src.is_empty()).then(|| src.as_bstr()); let dst = (!dst.is_empty()).then(|| dst.as_bstr()); match (src, dst) { - (None, None) => (None, None), // match all + (None, None) => (None, None), (None, Some(dst)) => match operation { Operation::Push => (None, Some(dst)), Operation::Fetch => (Some("HEAD".into()), Some(dst)), }, + (Some(src), None) => match operation { + Operation::Push => return Err(Error::PushToEmpty), + Operation::Fetch => (Some(src), None), + }, _ => todo!("src or dst handling"), } } diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index 4215a7cd124..e982db2b842 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -15,6 +15,7 @@ impl RefSpecRef<'_> { item.contains(&b'*') } match (self.op, self.mode, self.src, self.dst) { + (Operation::Fetch, Mode::Normal | Mode::Force, Some(src), None) => Instruction::Fetch(Fetch::Only { src }), (Operation::Fetch, Mode::Normal | Mode::Force, Some(src), Some(dst)) => { Instruction::Fetch(Fetch::AndUpdateSingle { src, diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 1cb3f76f727..296a31fde5d 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -74,78 +74,97 @@ mod invalid { } } - mod fetch { - use crate::parse::{assert_parse, b}; - use git_refspec::{Fetch, Instruction}; - - #[test] - fn empty_lhs_colon_rhs_fetches_head_to_destination() { - assert_parse( - ":a", - Instruction::Fetch(Fetch::AndUpdateSingle { - src: b("HEAD"), - dst: b("a"), - allow_non_fast_forward: false, - }), - ); - - assert_parse( - "+:a", - Instruction::Fetch(Fetch::AndUpdateSingle { - src: b("HEAD"), - dst: b("a"), - allow_non_fast_forward: true, - }), - ); - } + #[test] + fn push_to_empty() { + assert!(matches!( + try_parse("HEAD:", Operation::Push).unwrap_err(), + Error::PushToEmpty + )); + } +} - #[test] - fn colon_alone_is_for_fetching_into_fetchhead() { - assert_parse( - ":", - Instruction::Fetch(Fetch::AllMatchingBranches { - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+:", - Instruction::Fetch(Fetch::AllMatchingBranches { - allow_non_fast_forward: true, - }), - ); - } +mod fetch { + use crate::parse::{assert_parse, b}; + use git_refspec::{Fetch, Instruction, Mode}; + + #[test] + fn lhs_colon_empty_fetches_only() { + assert_parse("src:", Instruction::Fetch(Fetch::Only { src: b("src") })); + let spec = assert_parse("+src:", Instruction::Fetch(Fetch::Only { src: b("src") })); + assert_eq!( + spec.mode(), + Mode::Force, + "force is set, even though it has no effect in the actual instruction" + ); } - mod push { - use crate::parse::{assert_parse, b}; - use git_refspec::{Instruction, Mode, Push}; - - #[test] - fn colon_alone_is_for_pushing_matching_refs() { - assert_parse( - ":", - Instruction::Push(Push::AllMatchingBranches { - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+:", - Instruction::Push(Push::AllMatchingBranches { - allow_non_fast_forward: true, - }), - ); - } + #[test] + fn empty_lhs_colon_rhs_fetches_head_to_destination() { + assert_parse( + ":a", + Instruction::Fetch(Fetch::AndUpdateSingle { + src: b("HEAD"), + dst: b("a"), + allow_non_fast_forward: false, + }), + ); - #[test] - fn delete() { - assert_parse(":a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); - let spec = assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); - assert_eq!( - spec.mode(), - Mode::Force, - "force is set, even though it has no effect in the actual instruction" - ); - } + assert_parse( + "+:a", + Instruction::Fetch(Fetch::AndUpdateSingle { + src: b("HEAD"), + dst: b("a"), + allow_non_fast_forward: true, + }), + ); + } + + #[test] + fn colon_alone_is_for_fetching_into_fetchhead() { + assert_parse( + ":", + Instruction::Fetch(Fetch::AllMatchingBranches { + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+:", + Instruction::Fetch(Fetch::AllMatchingBranches { + allow_non_fast_forward: true, + }), + ); + } +} + +mod push { + use crate::parse::{assert_parse, b}; + use git_refspec::{Instruction, Mode, Push}; + + #[test] + fn colon_alone_is_for_pushing_matching_refs() { + assert_parse( + ":", + Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+:", + Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: true, + }), + ); + } + + #[test] + fn delete() { + assert_parse(":a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); + let spec = assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); + assert_eq!( + spec.mode(), + Mode::Force, + "force is set, even though it has no effect in the actual instruction" + ); } } From e4227d6ddd4cd021245bf6f352a0798457c37aae Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 19:13:06 +0800 Subject: [PATCH 16/36] basic validation and detection of patterns (#450) --- git-refspec/src/parse.rs | 26 ++++++++++++++++++++++++-- git-refspec/tests/parse/mod.rs | 24 +++++++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 19545d5cb66..a242bff43f5 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -6,6 +6,10 @@ pub enum Error { NegativeWithDestination, #[error("Cannot push into an empty destination")] PushToEmpty, + #[error("glob patterns may only involved a single '*' character, found {pattern:?}")] + PatternUnsupported { pattern: bstr::BString }, + #[error("Both sides of the specification need a pattern, like 'a/*:b/*'")] + PatternUnbalanced, } pub(crate) mod function { @@ -48,12 +52,17 @@ pub(crate) mod function { Operation::Push => return Err(Error::PushToEmpty), Operation::Fetch => (Some(src), None), }, - _ => todo!("src or dst handling"), + (Some(src), Some(dst)) => (Some(src), Some(dst)), } } - None => todo!("no colon"), + None => (Some(spec), None), }; + let (src, src_had_pattern) = validated(src)?; + let (dst, dst_had_pattern) = validated(dst)?; + if src_had_pattern != dst_had_pattern { + return Err(Error::PatternUnbalanced); + } Ok(RefSpecRef { op: operation, mode, @@ -61,4 +70,17 @@ pub(crate) mod function { dst, }) } + + fn validated(spec: Option<&BStr>) -> Result<(Option<&BStr>, bool), Error> { + match spec { + Some(spec) => { + let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count(); + if glob_count == 2 { + return Err(Error::PatternUnsupported { pattern: spec.into() }); + } + Ok((Some(spec), glob_count == 1)) + } + None => Ok((None, false)), + } + } } diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 296a31fde5d..84bdc38e0e6 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -15,6 +15,7 @@ fn baseline() { while let Some(kind_spec) = lines.next() { count += 1; let (kind, spec) = kind_spec.split_at(kind_spec.find_byte(b' ').expect("space between kind and spec")); + let spec = &spec[1..]; let err_code: usize = lines .next() .expect("err code") @@ -32,7 +33,7 @@ fn baseline() { Ok(res) => match (res.is_ok(), err_code == 0) { (true, true) | (false, false) => {} _ => { - eprintln!("{res:?} {err_code}"); + eprintln!("{res:?} {err_code} {} {:?}", kind.as_bstr(), spec.as_bstr()); mismatch += 1; } }, @@ -74,6 +75,27 @@ mod invalid { } } + #[test] + fn complex_patterns_with_more_than_one_asterisk() { + for op in [Operation::Fetch, Operation::Push] { + for spec in ["^*/*", "a/*/c/*", "a**:**b", "+:**/"] { + assert!(matches!( + try_parse(spec, op).unwrap_err(), + Error::PatternUnsupported { .. } + )); + } + } + } + + #[test] + fn both_sides_need_pattern_if_one_uses_it() { + for op in [Operation::Fetch, Operation::Push] { + for spec in ["/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] { + assert!(matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced)); + } + } + } + #[test] fn push_to_empty() { assert!(matches!( From c99f5750967a835afd9a99211b3520b441ae1881 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 19:39:05 +0800 Subject: [PATCH 17/36] Better handling of special cases (#450) --- git-refspec/src/parse.rs | 17 +++++++++++-- git-refspec/src/spec.rs | 5 ---- git-refspec/src/types.rs | 5 ---- .../generated-archives/make_baseline.tar.xz | 4 +-- git-refspec/tests/fixtures/make_baseline.sh | 1 + git-refspec/tests/parse/mod.rs | 25 ++++++++----------- 6 files changed, 28 insertions(+), 29 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index a242bff43f5..7690cb7175a 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -29,7 +29,17 @@ pub(crate) mod function { Mode::Force } Some(_) => Mode::Normal, - None => return Err(Error::Empty), + None => { + return match operation { + Operation::Push => Err(Error::Empty), + Operation::Fetch => Ok(RefSpecRef { + mode: Mode::Normal, + op: operation, + src: Some("HEAD".into()), + dst: None, + }), + } + } }; let (src, dst) = match spec.find_byte(b':') { @@ -43,7 +53,10 @@ pub(crate) mod function { let src = (!src.is_empty()).then(|| src.as_bstr()); let dst = (!dst.is_empty()).then(|| dst.as_bstr()); match (src, dst) { - (None, None) => (None, None), + (None, None) => match operation { + Operation::Push => (None, None), + Operation::Fetch => (Some("HEAD".into()), None), + }, (None, Some(dst)) => match operation { Operation::Push => (None, Some(dst)), Operation::Fetch => (Some("HEAD".into()), Some(dst)), diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index e982db2b842..49de225ccca 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -34,11 +34,6 @@ impl RefSpecRef<'_> { (Operation::Push, Mode::Normal | Mode::Force, None, None) => Instruction::Push(Push::AllMatchingBranches { allow_non_fast_forward: matches!(self.mode, Mode::Force), }), - (Operation::Fetch, Mode::Normal | Mode::Force, None, None) => { - Instruction::Fetch(Fetch::AllMatchingBranches { - allow_non_fast_forward: matches!(self.mode, Mode::Force), - }) - } (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => { Instruction::Push(Push::MultipleWithGlob { src, diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 9588cafbc6d..6ca621aacf7 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -70,11 +70,6 @@ pub enum Push<'a> { #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Fetch<'a> { - /// TODO: figure out what this actually does - it's valid for sure and only fetches HEAD -> FETCH_HEAD apparently - AllMatchingBranches { - /// Unclear what this does, but it's allowed - allow_non_fast_forward: bool, - }, Only { /// The ref name to fetch on the remote side, without updating the local side. src: &'a BStr, diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz index 73c87d7d28c..5bad0ada60b 100644 --- a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:515045d1eade39e1aeee1f171216ac389b8adc54af541b2ff49bf1b1ab68eae2 -size 9340 +oid sha256:d68f02fcf17b72dfb953ab02f3d448d7b859d822aa71835856045a291a645d5a +size 9344 diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh index a87d40cbf14..1f8812b038d 100644 --- a/git-refspec/tests/fixtures/make_baseline.sh +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -64,6 +64,7 @@ baseline push ':' baseline fetch '' baseline fetch ':' +baseline fetch '+' baseline push 'refs/heads/main:refs/remotes/frotz/xyzzy' baseline push 'refs/heads/*:refs/remotes/frotz/*' diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 84bdc38e0e6..47020942b11 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -33,7 +33,7 @@ fn baseline() { Ok(res) => match (res.is_ok(), err_code == 0) { (true, true) | (false, false) => {} _ => { - eprintln!("{res:?} {err_code} {} {:?}", kind.as_bstr(), spec.as_bstr()); + eprintln!("{err_code} {res:?} {} {:?}", kind.as_bstr(), spec.as_bstr()); mismatch += 1; } }, @@ -59,7 +59,6 @@ mod invalid { #[test] fn empty() { - assert!(matches!(try_parse("", Operation::Fetch).unwrap_err(), Error::Empty)); assert!(matches!(try_parse("", Operation::Push).unwrap_err(), Error::Empty)); } @@ -142,19 +141,15 @@ mod fetch { } #[test] - fn colon_alone_is_for_fetching_into_fetchhead() { - assert_parse( - ":", - Instruction::Fetch(Fetch::AllMatchingBranches { - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+:", - Instruction::Fetch(Fetch::AllMatchingBranches { - allow_non_fast_forward: true, - }), - ); + fn colon_alone_is_for_fetching_head_into_fetchhead() { + assert_parse(":", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); + let spec = assert_parse("+:", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); + assert_eq!(spec.mode(), Mode::Force, "it's set even though it's not useful"); + } + + #[test] + fn empty_refspec_is_enough_for_fetching_head_into_fetchhead() { + assert_parse("", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); } } From c4499ce13aa9e71c7b0024ad8658bdbcbccf5c14 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 19:57:51 +0800 Subject: [PATCH 18/36] tests for handling exclusions (#450) --- git-refspec/src/parse.rs | 2 +- git-refspec/src/spec.rs | 84 +++++++++++++++++++--------------- git-refspec/src/types.rs | 10 ++-- git-refspec/tests/parse/mod.rs | 23 ++++++++++ 4 files changed, 79 insertions(+), 40 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 7690cb7175a..cf0f63f0166 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -73,7 +73,7 @@ pub(crate) mod function { let (src, src_had_pattern) = validated(src)?; let (dst, dst_had_pattern) = validated(dst)?; - if src_had_pattern != dst_had_pattern { + if mode != Mode::Negative && src_had_pattern != dst_had_pattern { return Err(Error::PatternUnbalanced); } Ok(RefSpecRef { diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index 49de225ccca..d7c46c9c169 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -14,48 +14,60 @@ impl RefSpecRef<'_> { fn has_pattern(item: &BStr) -> bool { item.contains(&b'*') } - match (self.op, self.mode, self.src, self.dst) { - (Operation::Fetch, Mode::Normal | Mode::Force, Some(src), None) => Instruction::Fetch(Fetch::Only { src }), - (Operation::Fetch, Mode::Normal | Mode::Force, Some(src), Some(dst)) => { - Instruction::Fetch(Fetch::AndUpdateSingle { + match self.op { + Operation::Fetch => match (self.mode, self.src, self.dst) { + (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Fetch(Fetch::Only { src }), + (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Fetch(Fetch::AndUpdateSingle { src, dst, allow_non_fast_forward: matches!(self.mode, Mode::Force), - }) - } - (Operation::Push, Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Single { - src, - dst: src, - allow_non_fast_forward: matches!(self.mode, Mode::Force), - }), - (Operation::Push, Mode::Normal | Mode::Force, None, Some(dst)) => { - Instruction::Push(Push::Delete { ref_or_pattern: dst }) - } - (Operation::Push, Mode::Normal | Mode::Force, None, None) => Instruction::Push(Push::AllMatchingBranches { - allow_non_fast_forward: matches!(self.mode, Mode::Force), - }), - (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => { - Instruction::Push(Push::MultipleWithGlob { + }), + (Mode::Negative, Some(src), None) if has_pattern(src) => { + Instruction::Fetch(Fetch::ExcludeMultipleWithGlob { src }) + } + (Mode::Negative, Some(src), None) => Instruction::Fetch(Fetch::ExcludeSingle { src }), + (mode, src, dest) => { + unreachable!( + "BUG: fetch instructions with {:?} {:?} {:?} are not possible", + mode, src, dest + ) + } + }, + Operation::Push => match (self.mode, self.src, self.dst) { + (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Single { + src, + dst: src, + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }), + (Mode::Normal | Mode::Force, None, Some(dst)) => { + Instruction::Push(Push::Delete { ref_or_pattern: dst }) + } + (Mode::Normal | Mode::Force, None, None) => Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }), + (Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => { + Instruction::Push(Push::MultipleWithGlob { + src, + dst, + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }) + } + (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Push(Push::Single { src, dst, allow_non_fast_forward: matches!(self.mode, Mode::Force), - }) - } - (Operation::Push, Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Push(Push::Single { - src, - dst, - allow_non_fast_forward: matches!(self.mode, Mode::Force), - }), - (Operation::Push, Mode::Negative, Some(src), None) if has_pattern(src) => { - Instruction::Push(Push::ExcludeMultipleWithGlob { src }) - } - (Operation::Push, Mode::Negative, Some(src), None) => Instruction::Push(Push::ExcludeSingle { src }), - (op, mode, src, dest) => { - unreachable!( - "BUG: instructions with {:?} {:?} {:?} {:?} are not possible", - op, mode, src, dest - ) - } + }), + (Mode::Negative, Some(src), None) if has_pattern(src) => { + Instruction::Push(Push::ExcludeMultipleWithGlob { src }) + } + (Mode::Negative, Some(src), None) => Instruction::Push(Push::ExcludeSingle { src }), + (mode, src, dest) => { + unreachable!( + "BUG: push instructions with {:?} {:?} {:?} are not possible", + mode, src, dest + ) + } + }, } } } diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 6ca621aacf7..348dfc5427d 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -26,6 +26,7 @@ pub enum Instruction<'a> { Fetch(Fetch<'a>), } +/// Note that all sources can either be a ref-name, partial or full, or a rev-spec. Destinations can only be a partial or full ref name. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Push<'a> { /// Push all local branches to the matching destination on the remote, which has to exist to be updated. @@ -59,7 +60,7 @@ pub enum Push<'a> { }, /// Push a multiple refs to matching destination refs, with exactly a single glob on both sides. MultipleWithGlob { - /// The source ref to match against all refs for pushing. + /// The source ref to match against all refs for pushing, as pattern with a single `*`. src: &'a BStr, /// The ref to update with object obtained from `src`, filling in the `*` with the portion that matched in `src`. dst: &'a BStr, @@ -68,10 +69,13 @@ pub enum Push<'a> { }, } +/// Note that any source can either be a ref name (full or partial) or a fully spelled out hex-sha for an object. +/// +/// Destinations can only be a partial or full ref-name. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Fetch<'a> { Only { - /// The ref name to fetch on the remote side, without updating the local side. + /// The ref name to fetch on the remote side, without updating the local side. This will write the result into `FETCH_HEAD`. src: &'a BStr, }, /// Exclude a single ref. @@ -94,7 +98,7 @@ pub enum Fetch<'a> { }, /// Similar to `FetchAndUpdate`, but src and destination contain a single glob to fetch and update multiple refs. AndUpdateMultipleWithGlob { - /// The ref glob to match against all refs on the remote side for fetching. + /// The ref glob to match against all refs on the remote side for fetching, as pattern with a single `*`. src: &'a BStr, /// The local destination to update with what was fetched by replacing the single `*` with the matching portion from `src`. dst: &'a BStr, diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 47020942b11..2c2fa801267 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -108,6 +108,19 @@ mod fetch { use crate::parse::{assert_parse, b}; use git_refspec::{Fetch, Instruction, Mode}; + #[test] + fn exclude_single() { + assert_parse("^a", Instruction::Fetch(Fetch::ExcludeSingle { src: b("a") })); + } + + #[test] + fn exclude_multiple() { + assert_parse( + "^a*", + Instruction::Fetch(Fetch::ExcludeMultipleWithGlob { src: b("a*") }), + ); + } + #[test] fn lhs_colon_empty_fetches_only() { assert_parse("src:", Instruction::Fetch(Fetch::Only { src: b("src") })); @@ -157,6 +170,16 @@ mod push { use crate::parse::{assert_parse, b}; use git_refspec::{Instruction, Mode, Push}; + #[test] + fn exclude_single() { + assert_parse("^a", Instruction::Push(Push::ExcludeSingle { src: b("a") })); + } + + #[test] + fn exclude_multiple() { + assert_parse("^a*", Instruction::Push(Push::ExcludeMultipleWithGlob { src: b("a*") })); + } + #[test] fn colon_alone_is_for_pushing_matching_refs() { assert_parse( From c23a21d3e50e62d29bef4e638049b0398d3fb20e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 20:10:15 +0800 Subject: [PATCH 19/36] tests causing all instrucitons (#450) --- git-refspec/src/spec.rs | 7 +++ git-refspec/tests/parse/mod.rs | 86 +++++++++++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index d7c46c9c169..25e1ce5c74d 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -17,6 +17,13 @@ impl RefSpecRef<'_> { match self.op { Operation::Fetch => match (self.mode, self.src, self.dst) { (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Fetch(Fetch::Only { src }), + (Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => { + Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob { + src, + dst, + allow_non_fast_forward: matches!(self.mode, Mode::Force), + }) + } (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Fetch(Fetch::AndUpdateSingle { src, dst, diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 2c2fa801267..6b2cdac8db5 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -31,7 +31,11 @@ fn baseline() { let res = catch_unwind(|| try_parse(spec.to_str().unwrap(), op)); match res { Ok(res) => match (res.is_ok(), err_code == 0) { - (true, true) | (false, false) => {} + (true, true) | (false, false) => { + if let Ok(spec) = res { + spec.instruction(); // should not panic + } + } _ => { eprintln!("{err_code} {res:?} {} {:?}", kind.as_bstr(), spec.as_bstr()); mismatch += 1; @@ -132,6 +136,46 @@ mod fetch { ); } + #[test] + fn lhs_colon_rhs_updates_single_ref() { + assert_parse( + "a:b", + Instruction::Fetch(Fetch::AndUpdateSingle { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a:b", + Instruction::Fetch(Fetch::AndUpdateSingle { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: true, + }), + ); + } + + #[test] + fn lhs_colon_rhs_with_glob_updates_multiple_refs() { + assert_parse( + "a/*:b/*", + Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a/*:b/*", + Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: true, + }), + ); + } + #[test] fn empty_lhs_colon_rhs_fetches_head_to_destination() { assert_parse( @@ -180,6 +224,46 @@ mod push { assert_parse("^a*", Instruction::Push(Push::ExcludeMultipleWithGlob { src: b("a*") })); } + #[test] + fn lhs_colon_rhs_pushes_single_ref() { + assert_parse( + "a:b", + Instruction::Push(Push::Single { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a:b", + Instruction::Push(Push::Single { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: true, + }), + ); + } + + #[test] + fn lhs_colon_rhs_with_glob_pushes_multiple_refs() { + assert_parse( + "a/*:b/*", + Instruction::Push(Push::MultipleWithGlob { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a/*:b/*", + Instruction::Push(Push::MultipleWithGlob { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: true, + }), + ); + } + #[test] fn colon_alone_is_for_pushing_matching_refs() { assert_parse( From e8c072e99e845ed1b4a0cc0a0ec7146c53561dcd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 20:41:04 +0800 Subject: [PATCH 20/36] refactor (#450) --- git-refspec/src/spec.rs | 34 +----- git-refspec/src/types.rs | 56 +++------- git-refspec/tests/parse/fetch.rs | 89 +++++++++++++++ git-refspec/tests/parse/mod.rs | 185 +------------------------------ git-refspec/tests/parse/push.rs | 71 ++++++++++++ 5 files changed, 182 insertions(+), 253 deletions(-) create mode 100644 git-refspec/tests/parse/fetch.rs create mode 100644 git-refspec/tests/parse/push.rs diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index 25e1ce5c74d..f74b6697766 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -1,6 +1,5 @@ use crate::types::Push; use crate::{Fetch, Instruction, Mode, Operation, RefSpec, RefSpecRef}; -use bstr::BStr; /// Access impl RefSpecRef<'_> { @@ -11,28 +10,15 @@ impl RefSpecRef<'_> { /// Transform the state of the refspec into an instruction making clear what to do with it. pub fn instruction(&self) -> Instruction<'_> { - fn has_pattern(item: &BStr) -> bool { - item.contains(&b'*') - } match self.op { Operation::Fetch => match (self.mode, self.src, self.dst) { (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Fetch(Fetch::Only { src }), - (Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => { - Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob { - src, - dst, - allow_non_fast_forward: matches!(self.mode, Mode::Force), - }) - } - (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Fetch(Fetch::AndUpdateSingle { + (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Fetch(Fetch::AndUpdate { src, dst, allow_non_fast_forward: matches!(self.mode, Mode::Force), }), - (Mode::Negative, Some(src), None) if has_pattern(src) => { - Instruction::Fetch(Fetch::ExcludeMultipleWithGlob { src }) - } - (Mode::Negative, Some(src), None) => Instruction::Fetch(Fetch::ExcludeSingle { src }), + (Mode::Negative, Some(src), None) => Instruction::Fetch(Fetch::Exclude { src }), (mode, src, dest) => { unreachable!( "BUG: fetch instructions with {:?} {:?} {:?} are not possible", @@ -41,7 +27,7 @@ impl RefSpecRef<'_> { } }, Operation::Push => match (self.mode, self.src, self.dst) { - (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Single { + (Mode::Normal | Mode::Force, Some(src), None) => Instruction::Push(Push::Matching { src, dst: src, allow_non_fast_forward: matches!(self.mode, Mode::Force), @@ -52,22 +38,12 @@ impl RefSpecRef<'_> { (Mode::Normal | Mode::Force, None, None) => Instruction::Push(Push::AllMatchingBranches { allow_non_fast_forward: matches!(self.mode, Mode::Force), }), - (Mode::Normal | Mode::Force, Some(src), Some(dst)) if has_pattern(src) => { - Instruction::Push(Push::MultipleWithGlob { - src, - dst, - allow_non_fast_forward: matches!(self.mode, Mode::Force), - }) - } - (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Push(Push::Single { + (Mode::Normal | Mode::Force, Some(src), Some(dst)) => Instruction::Push(Push::Matching { src, dst, allow_non_fast_forward: matches!(self.mode, Mode::Force), }), - (Mode::Negative, Some(src), None) if has_pattern(src) => { - Instruction::Push(Push::ExcludeMultipleWithGlob { src }) - } - (Mode::Negative, Some(src), None) => Instruction::Push(Push::ExcludeSingle { src }), + (Mode::Negative, Some(src), None) => Instruction::Push(Push::Exclude { src }), (mode, src, dest) => { unreachable!( "BUG: push instructions with {:?} {:?} {:?} are not possible", diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 348dfc5427d..969663c0342 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -26,7 +26,8 @@ pub enum Instruction<'a> { Fetch(Fetch<'a>), } -/// Note that all sources can either be a ref-name, partial or full, or a rev-spec. Destinations can only be a partial or full ref name. +/// Note that all sources can either be a ref-name, partial or full, or a rev-spec, unless specified otherwise, on the local side. +/// Destinations can only be a partial or full ref names on the remote side. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Push<'a> { /// Push all local branches to the matching destination on the remote, which has to exist to be updated. @@ -40,38 +41,24 @@ pub enum Push<'a> { ref_or_pattern: &'a BStr, }, /// Exclude a single ref. - ExcludeSingle { - /// A single full ref name to exclude. - src: &'a BStr, - }, - /// Exclude multiple refs with single `*` glob. - ExcludeMultipleWithGlob { - /// A ref pattern with a single `*`. + Exclude { + /// A full or partial ref name to exclude, or multiple if a single `*` is used. src: &'a BStr, }, /// Push a single ref or refspec to a known destination ref. - Single { - /// The source ref or refspec to push. - src: &'a BStr, - /// The ref to update with the object from `src`. - dst: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, - /// Push a multiple refs to matching destination refs, with exactly a single glob on both sides. - MultipleWithGlob { - /// The source ref to match against all refs for pushing, as pattern with a single `*`. + Matching { + /// The source ref or refspec to push. If pattern, it contains a single `*`. src: &'a BStr, - /// The ref to update with object obtained from `src`, filling in the `*` with the portion that matched in `src`. + /// The ref to update with the object from `src`. If `src` is a pattern, this is a pattern too. dst: &'a BStr, /// If true, allow non-fast-forward updates of `dest`. allow_non_fast_forward: bool, }, } -/// Note that any source can either be a ref name (full or partial) or a fully spelled out hex-sha for an object. +/// Note that any source can either be a ref name (full or partial) or a fully spelled out hex-sha for an object, on the remote side. /// -/// Destinations can only be a partial or full ref-name. +/// Destinations can only be a partial or full ref-names on the local side. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Fetch<'a> { Only { @@ -79,28 +66,15 @@ pub enum Fetch<'a> { src: &'a BStr, }, /// Exclude a single ref. - ExcludeSingle { - /// A single full ref name to exclude. + Exclude { + /// A single partial or full ref name to exclude on the remote, or a pattern with a single `*`. src: &'a BStr, }, - /// Exclude multiple refs with single `*` glob. - ExcludeMultipleWithGlob { - /// A ref pattern with a single `*`. - src: &'a BStr, - }, - AndUpdateSingle { - /// The ref name to fetch on the remote side. - src: &'a BStr, - /// The local destination to update with what was fetched. - dst: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, - /// Similar to `FetchAndUpdate`, but src and destination contain a single glob to fetch and update multiple refs. - AndUpdateMultipleWithGlob { - /// The ref glob to match against all refs on the remote side for fetching, as pattern with a single `*`. + AndUpdate { + /// The ref name to fetch on the remote side, or a pattern with a single `*` to match against. src: &'a BStr, - /// The local destination to update with what was fetched by replacing the single `*` with the matching portion from `src`. + /// The local destination to update with what was fetched, or a pattern whose single `*` will be replaced with the matching portion + /// of the `*` from `src`. dst: &'a BStr, /// If true, allow non-fast-forward updates of `dest`. allow_non_fast_forward: bool, diff --git a/git-refspec/tests/parse/fetch.rs b/git-refspec/tests/parse/fetch.rs new file mode 100644 index 00000000000..0a9ec05ef69 --- /dev/null +++ b/git-refspec/tests/parse/fetch.rs @@ -0,0 +1,89 @@ +use crate::parse::{assert_parse, b}; +use git_refspec::{Fetch, Instruction, Mode}; + +#[test] +fn exclude() { + assert_parse("^a", Instruction::Fetch(Fetch::Exclude { src: b("a") })); + assert_parse("^a*", Instruction::Fetch(Fetch::Exclude { src: b("a*") })); +} + +#[test] +fn lhs_colon_empty_fetches_only() { + assert_parse("src:", Instruction::Fetch(Fetch::Only { src: b("src") })); + let spec = assert_parse("+src:", Instruction::Fetch(Fetch::Only { src: b("src") })); + assert_eq!( + spec.mode(), + Mode::Force, + "force is set, even though it has no effect in the actual instruction" + ); +} + +#[test] +fn lhs_colon_rhs_updates_single_ref() { + assert_parse( + "a:b", + Instruction::Fetch(Fetch::AndUpdate { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a:b", + Instruction::Fetch(Fetch::AndUpdate { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: true, + }), + ); + + assert_parse( + "a/*:b/*", + Instruction::Fetch(Fetch::AndUpdate { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a/*:b/*", + Instruction::Fetch(Fetch::AndUpdate { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: true, + }), + ); +} + +#[test] +fn empty_lhs_colon_rhs_fetches_head_to_destination() { + assert_parse( + ":a", + Instruction::Fetch(Fetch::AndUpdate { + src: b("HEAD"), + dst: b("a"), + allow_non_fast_forward: false, + }), + ); + + assert_parse( + "+:a", + Instruction::Fetch(Fetch::AndUpdate { + src: b("HEAD"), + dst: b("a"), + allow_non_fast_forward: true, + }), + ); +} + +#[test] +fn colon_alone_is_for_fetching_head_into_fetchhead() { + assert_parse(":", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); + let spec = assert_parse("+:", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); + assert_eq!(spec.mode(), Mode::Force, "it's set even though it's not useful"); +} + +#[test] +fn empty_refspec_is_enough_for_fetching_head_into_fetchhead() { + assert_parse("", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); +} diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 6b2cdac8db5..174019a9ccb 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -108,189 +108,8 @@ mod invalid { } } -mod fetch { - use crate::parse::{assert_parse, b}; - use git_refspec::{Fetch, Instruction, Mode}; - - #[test] - fn exclude_single() { - assert_parse("^a", Instruction::Fetch(Fetch::ExcludeSingle { src: b("a") })); - } - - #[test] - fn exclude_multiple() { - assert_parse( - "^a*", - Instruction::Fetch(Fetch::ExcludeMultipleWithGlob { src: b("a*") }), - ); - } - - #[test] - fn lhs_colon_empty_fetches_only() { - assert_parse("src:", Instruction::Fetch(Fetch::Only { src: b("src") })); - let spec = assert_parse("+src:", Instruction::Fetch(Fetch::Only { src: b("src") })); - assert_eq!( - spec.mode(), - Mode::Force, - "force is set, even though it has no effect in the actual instruction" - ); - } - - #[test] - fn lhs_colon_rhs_updates_single_ref() { - assert_parse( - "a:b", - Instruction::Fetch(Fetch::AndUpdateSingle { - src: b("a"), - dst: b("b"), - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+a:b", - Instruction::Fetch(Fetch::AndUpdateSingle { - src: b("a"), - dst: b("b"), - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn lhs_colon_rhs_with_glob_updates_multiple_refs() { - assert_parse( - "a/*:b/*", - Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob { - src: b("a/*"), - dst: b("b/*"), - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+a/*:b/*", - Instruction::Fetch(Fetch::AndUpdateMultipleWithGlob { - src: b("a/*"), - dst: b("b/*"), - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn empty_lhs_colon_rhs_fetches_head_to_destination() { - assert_parse( - ":a", - Instruction::Fetch(Fetch::AndUpdateSingle { - src: b("HEAD"), - dst: b("a"), - allow_non_fast_forward: false, - }), - ); - - assert_parse( - "+:a", - Instruction::Fetch(Fetch::AndUpdateSingle { - src: b("HEAD"), - dst: b("a"), - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn colon_alone_is_for_fetching_head_into_fetchhead() { - assert_parse(":", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); - let spec = assert_parse("+:", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); - assert_eq!(spec.mode(), Mode::Force, "it's set even though it's not useful"); - } - - #[test] - fn empty_refspec_is_enough_for_fetching_head_into_fetchhead() { - assert_parse("", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); - } -} - -mod push { - use crate::parse::{assert_parse, b}; - use git_refspec::{Instruction, Mode, Push}; - - #[test] - fn exclude_single() { - assert_parse("^a", Instruction::Push(Push::ExcludeSingle { src: b("a") })); - } - - #[test] - fn exclude_multiple() { - assert_parse("^a*", Instruction::Push(Push::ExcludeMultipleWithGlob { src: b("a*") })); - } - - #[test] - fn lhs_colon_rhs_pushes_single_ref() { - assert_parse( - "a:b", - Instruction::Push(Push::Single { - src: b("a"), - dst: b("b"), - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+a:b", - Instruction::Push(Push::Single { - src: b("a"), - dst: b("b"), - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn lhs_colon_rhs_with_glob_pushes_multiple_refs() { - assert_parse( - "a/*:b/*", - Instruction::Push(Push::MultipleWithGlob { - src: b("a/*"), - dst: b("b/*"), - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+a/*:b/*", - Instruction::Push(Push::MultipleWithGlob { - src: b("a/*"), - dst: b("b/*"), - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn colon_alone_is_for_pushing_matching_refs() { - assert_parse( - ":", - Instruction::Push(Push::AllMatchingBranches { - allow_non_fast_forward: false, - }), - ); - assert_parse( - "+:", - Instruction::Push(Push::AllMatchingBranches { - allow_non_fast_forward: true, - }), - ); - } - - #[test] - fn delete() { - assert_parse(":a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); - let spec = assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); - assert_eq!( - spec.mode(), - Mode::Force, - "force is set, even though it has no effect in the actual instruction" - ); - } -} +mod fetch; +mod push; mod util { use git_refspec::{Instruction, Operation, RefSpecRef}; diff --git a/git-refspec/tests/parse/push.rs b/git-refspec/tests/parse/push.rs new file mode 100644 index 00000000000..bcda808f976 --- /dev/null +++ b/git-refspec/tests/parse/push.rs @@ -0,0 +1,71 @@ +use crate::parse::{assert_parse, b}; +use git_refspec::{Instruction, Mode, Push}; + +#[test] +fn exclude() { + assert_parse("^a", Instruction::Push(Push::Exclude { src: b("a") })); + assert_parse("^a*", Instruction::Push(Push::Exclude { src: b("a*") })); +} + +#[test] +fn lhs_colon_rhs_pushes_single_ref() { + assert_parse( + "a:b", + Instruction::Push(Push::Matching { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a:b", + Instruction::Push(Push::Matching { + src: b("a"), + dst: b("b"), + allow_non_fast_forward: true, + }), + ); + assert_parse( + "a/*:b/*", + Instruction::Push(Push::Matching { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+a/*:b/*", + Instruction::Push(Push::Matching { + src: b("a/*"), + dst: b("b/*"), + allow_non_fast_forward: true, + }), + ); +} + +#[test] +fn colon_alone_is_for_pushing_matching_refs() { + assert_parse( + ":", + Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+:", + Instruction::Push(Push::AllMatchingBranches { + allow_non_fast_forward: true, + }), + ); +} + +#[test] +fn delete() { + assert_parse(":a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); + let spec = assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); + assert_eq!( + spec.mode(), + Mode::Force, + "force is set, even though it has no effect in the actual instruction" + ); +} From d7c27899c76092bdc8e86f2784aaf67666f117dd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 21:34:30 +0800 Subject: [PATCH 21/36] handle ref-name validation mostly correctly (#450) --- Cargo.lock | 2 ++ git-refspec/Cargo.toml | 2 ++ git-refspec/src/parse.rs | 36 +++++++++++++++++++++++++++------- git-refspec/tests/parse/mod.rs | 8 ++++++-- 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f69b160c236..195e9413f02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1490,6 +1490,8 @@ version = "0.0.0" dependencies = [ "bstr", "git-testtools", + "git-validate", + "smallvec", "thiserror", ] diff --git a/git-refspec/Cargo.toml b/git-refspec/Cargo.toml index caac3992c06..bcc76cfb3eb 100644 --- a/git-refspec/Cargo.toml +++ b/git-refspec/Cargo.toml @@ -15,7 +15,9 @@ doctest = false [dependencies] bstr = { version = "0.2.13", default-features = false, features = ["std"]} +git-validate = { version = "^0.5.4", path = "../git-validate" } thiserror = "1.0.26" +smallvec = "1.9.0" [dev-dependencies] git-testtools = { path = "../tests/tools" } diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index cf0f63f0166..046316ad570 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -10,6 +10,8 @@ pub enum Error { PatternUnsupported { pattern: bstr::BString }, #[error("Both sides of the specification need a pattern, like 'a/*:b/*'")] PatternUnbalanced, + #[error(transparent)] + Refname(#[from] git_validate::refname::Error), } pub(crate) mod function { @@ -19,6 +21,15 @@ pub(crate) mod function { /// Parse `spec` for use in `operation` and return it if it is valid. pub fn parse(mut spec: &BStr, operation: Operation) -> Result, Error> { + fn fetch_head_only(mode: Mode) -> RefSpecRef<'static> { + RefSpecRef { + mode, + op: Operation::Fetch, + src: Some("HEAD".into()), + dst: None, + } + } + let mode = match spec.get(0) { Some(&b'^') => { spec = &spec[1..]; @@ -32,12 +43,7 @@ pub(crate) mod function { None => { return match operation { Operation::Push => Err(Error::Empty), - Operation::Fetch => Ok(RefSpecRef { - mode: Mode::Normal, - op: operation, - src: Some("HEAD".into()), - dst: None, - }), + Operation::Fetch => Ok(fetch_head_only(Mode::Normal)), } } }; @@ -68,7 +74,14 @@ pub(crate) mod function { (Some(src), Some(dst)) => (Some(src), Some(dst)), } } - None => (Some(spec), None), + None => { + let src = (!spec.is_empty()).then(|| spec); + if Operation::Fetch == operation && src.is_none() { + return Ok(fetch_head_only(mode)); + } else { + (src, None) + } + } }; let (src, src_had_pattern) = validated(src)?; @@ -91,6 +104,15 @@ pub(crate) mod function { if glob_count == 2 { return Err(Error::PatternUnsupported { pattern: spec.into() }); } + if glob_count == 1 { + let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len()); + buf.extend_from_slice(&spec); + let glob_pos = buf.find_byte(b'*').expect("glob present"); + buf[glob_pos] = b'a'; + git_validate::reference::name_partial(buf.as_bstr())?; + } else { + git_validate::reference::name_partial(spec)?; + } Ok((Some(spec), glob_count == 1)) } None => Ok((None, false)), diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 174019a9ccb..2e89c7668c3 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -93,8 +93,12 @@ mod invalid { #[test] fn both_sides_need_pattern_if_one_uses_it() { for op in [Operation::Fetch, Operation::Push] { - for spec in ["/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] { - assert!(matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced)); + for spec in ["refs/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] { + assert!( + matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced), + "{}", + spec + ); } } } From 3fa52f8ffd04721e1367706318542cf6d4e71f3b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 21:49:15 +0800 Subject: [PATCH 22/36] and the entire test-suite passes (#450) --- Cargo.lock | 2 + git-refspec/Cargo.toml | 5 +- git-refspec/src/parse.rs | 88 ++++++++++++++++++++++++++++++++-- git-refspec/tests/parse/mod.rs | 1 - 4 files changed, 89 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 195e9413f02..0d853e295ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1489,6 +1489,8 @@ name = "git-refspec" version = "0.0.0" dependencies = [ "bstr", + "git-hash", + "git-revision", "git-testtools", "git-validate", "smallvec", diff --git a/git-refspec/Cargo.toml b/git-refspec/Cargo.toml index bcc76cfb3eb..aa8b8cbe3bf 100644 --- a/git-refspec/Cargo.toml +++ b/git-refspec/Cargo.toml @@ -14,8 +14,11 @@ doctest = false # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -bstr = { version = "0.2.13", default-features = false, features = ["std"]} +git-revision = { version = "^0.3.0", path = "../git-revision" } git-validate = { version = "^0.5.4", path = "../git-validate" } +git-hash = { version = "^0.9.6", path = "../git-hash" } + +bstr = { version = "0.2.13", default-features = false, features = ["std"]} thiserror = "1.0.26" smallvec = "1.9.0" diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 046316ad570..9e3af34f193 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -11,7 +11,9 @@ pub enum Error { #[error("Both sides of the specification need a pattern, like 'a/*:b/*'")] PatternUnbalanced, #[error(transparent)] - Refname(#[from] git_validate::refname::Error), + ReferenceName(#[from] git_validate::refname::Error), + #[error(transparent)] + RevSpec(#[from] git_revision::spec::parse::Error), } pub(crate) mod function { @@ -84,8 +86,8 @@ pub(crate) mod function { } }; - let (src, src_had_pattern) = validated(src)?; - let (dst, dst_had_pattern) = validated(dst)?; + let (src, src_had_pattern) = validated(src, operation == Operation::Push)?; + let (dst, dst_had_pattern) = validated(dst, false)?; if mode != Mode::Negative && src_had_pattern != dst_had_pattern { return Err(Error::PatternUnbalanced); } @@ -97,7 +99,7 @@ pub(crate) mod function { }) } - fn validated(spec: Option<&BStr>) -> Result<(Option<&BStr>, bool), Error> { + fn validated(spec: Option<&BStr>, allow_revspecs: bool) -> Result<(Option<&BStr>, bool), Error> { match spec { Some(spec) => { let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count(); @@ -111,7 +113,24 @@ pub(crate) mod function { buf[glob_pos] = b'a'; git_validate::reference::name_partial(buf.as_bstr())?; } else { - git_validate::reference::name_partial(spec)?; + git_validate::reference::name_partial(spec) + .map_err(Error::from) + .or_else(|err| { + if allow_revspecs { + match git_revision::spec::parse(spec, &mut super::revparse::Noop) { + Ok(_) => { + if spec.iter().any(|b| b.is_ascii_whitespace()) { + Err(err) + } else { + Ok(spec) + } + } + Err(err) => Err(err.into()), + } + } else { + Err(err) + } + })?; } Ok((Some(spec), glob_count == 1)) } @@ -119,3 +138,62 @@ pub(crate) mod function { } } } + +mod revparse { + use bstr::BStr; + use git_revision::spec::parse::delegate::{ + Kind, Navigate, PeelTo, PrefixHint, ReflogLookup, Revision, SiblingBranch, Traversal, + }; + + pub(crate) struct Noop; + + impl Revision for Noop { + fn find_ref(&mut self, _name: &BStr) -> Option<()> { + Some(()) + } + + fn disambiguate_prefix(&mut self, _prefix: git_hash::Prefix, _hint: Option>) -> Option<()> { + Some(()) + } + + fn reflog(&mut self, _query: ReflogLookup) -> Option<()> { + Some(()) + } + + fn nth_checked_out_branch(&mut self, _branch_no: usize) -> Option<()> { + Some(()) + } + + fn sibling_branch(&mut self, _kind: SiblingBranch) -> Option<()> { + Some(()) + } + } + + impl Navigate for Noop { + fn traverse(&mut self, _kind: Traversal) -> Option<()> { + Some(()) + } + + fn peel_until(&mut self, _kind: PeelTo<'_>) -> Option<()> { + Some(()) + } + + fn find(&mut self, _regex: &BStr, _negated: bool) -> Option<()> { + Some(()) + } + + fn index_lookup(&mut self, _path: &BStr, _stage: u8) -> Option<()> { + Some(()) + } + } + + impl Kind for Noop { + fn kind(&mut self, _kind: git_revision::spec::Kind) -> Option<()> { + Some(()) + } + } + + impl git_revision::spec::parse::Delegate for Noop { + fn done(&mut self) {} + } +} diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 2e89c7668c3..eb9af0ab125 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -4,7 +4,6 @@ use git_testtools::scripted_fixture_repo_read_only; use std::panic::catch_unwind; #[test] -#[should_panic] fn baseline() { let dir = scripted_fixture_repo_read_only("make_baseline.sh").unwrap(); let baseline = std::fs::read(dir.join("baseline.git")).unwrap(); From b62ee56b5bf468b9673b01034ec284faf1a7c7c2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 6 Aug 2022 21:49:40 +0800 Subject: [PATCH 23/36] thanks clippy --- git-refspec/src/parse.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 9e3af34f193..ee26be9f338 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -108,7 +108,7 @@ pub(crate) mod function { } if glob_count == 1 { let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len()); - buf.extend_from_slice(&spec); + buf.extend_from_slice(spec); let glob_pos = buf.find_byte(b'*').expect("glob present"); buf[glob_pos] = b'a'; git_validate::reference::name_partial(buf.as_bstr())?; From 32d98e9c5db402bbfc04394218c0de30bfa64808 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 09:04:44 +0800 Subject: [PATCH 24/36] support for `@` shortcut. (#450) --- git-refspec/src/parse.rs | 14 ++++++++---- .../generated-archives/make_baseline.tar.xz | 4 ++-- git-refspec/tests/fixtures/make_baseline.sh | 6 +++++ git-refspec/tests/parse/fetch.rs | 7 ++++++ git-refspec/tests/parse/push.rs | 22 +++++++++++++++++++ 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index ee26be9f338..5441bb872bf 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -50,7 +50,7 @@ pub(crate) mod function { } }; - let (src, dst) = match spec.find_byte(b':') { + let (mut src, dst) = match spec.find_byte(b':') { Some(pos) => { let (src, dst) = spec.split_at(pos); let dst = &dst[1..]; @@ -86,6 +86,11 @@ pub(crate) mod function { } }; + if let Some(spec) = src.as_mut() { + if *spec == "@" { + *spec = "HEAD".into(); + } + } let (src, src_had_pattern) = validated(src, operation == Operation::Push)?; let (dst, dst_had_pattern) = validated(dst, false)?; if mode != Mode::Negative && src_had_pattern != dst_had_pattern { @@ -103,10 +108,11 @@ pub(crate) mod function { match spec { Some(spec) => { let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count(); - if glob_count == 2 { + if glob_count > 1 { return Err(Error::PatternUnsupported { pattern: spec.into() }); } - if glob_count == 1 { + let has_globs = glob_count == 1; + if has_globs { let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len()); buf.extend_from_slice(spec); let glob_pos = buf.find_byte(b'*').expect("glob present"); @@ -132,7 +138,7 @@ pub(crate) mod function { } })?; } - Ok((Some(spec), glob_count == 1)) + Ok((Some(spec), has_globs)) } None => Ok((None, false)), } diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz index 5bad0ada60b..00433aa4fd8 100644 --- a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d68f02fcf17b72dfb953ab02f3d448d7b859d822aa71835856045a291a645d5a -size 9344 +oid sha256:ace43ca904298dc76951863d41485ae0c21ee9b88abf7d604eb6c6e7efa18ab2 +size 9352 diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh index 1f8812b038d..23951021a29 100644 --- a/git-refspec/tests/fixtures/make_baseline.sh +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -80,6 +80,12 @@ baseline fetch 'HEAD' baseline push '@' baseline fetch '@' +baseline push '^@' +baseline fetch '^@' + +baseline push '+@' +baseline fetch '+@' + baseline fetch 'HEAD:' baseline push ':refs/remotes/frotz/deleteme' diff --git a/git-refspec/tests/parse/fetch.rs b/git-refspec/tests/parse/fetch.rs index 0a9ec05ef69..22d2973b682 100644 --- a/git-refspec/tests/parse/fetch.rs +++ b/git-refspec/tests/parse/fetch.rs @@ -7,6 +7,13 @@ fn exclude() { assert_parse("^a*", Instruction::Fetch(Fetch::Exclude { src: b("a*") })); } +#[test] +fn ampersand_is_resolved_to_head() { + assert_parse("@", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); + assert_parse("+@", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); + assert_parse("^@", Instruction::Fetch(Fetch::Exclude { src: b("HEAD") })); +} + #[test] fn lhs_colon_empty_fetches_only() { assert_parse("src:", Instruction::Fetch(Fetch::Only { src: b("src") })); diff --git a/git-refspec/tests/parse/push.rs b/git-refspec/tests/parse/push.rs index bcda808f976..c133b672ac6 100644 --- a/git-refspec/tests/parse/push.rs +++ b/git-refspec/tests/parse/push.rs @@ -7,6 +7,28 @@ fn exclude() { assert_parse("^a*", Instruction::Push(Push::Exclude { src: b("a*") })); } +#[test] +fn ampersand_is_resolved_to_head() { + assert_parse( + "@", + Instruction::Push(Push::Matching { + src: b("HEAD"), + dst: b("HEAD"), + allow_non_fast_forward: false, + }), + ); + + assert_parse( + "+@", + Instruction::Push(Push::Matching { + src: b("HEAD"), + dst: b("HEAD"), + allow_non_fast_forward: true, + }), + ); + assert_parse("^@", Instruction::Push(Push::Exclude { src: b("HEAD") })); +} + #[test] fn lhs_colon_rhs_pushes_single_ref() { assert_parse( From 79e0eaf4c754da47731f8c5ed3635339586b3d00 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 09:24:58 +0800 Subject: [PATCH 25/36] negative must not be empty (#450) --- git-refspec/src/parse.rs | 12 +++- .../generated-archives/make_baseline.tar.xz | 4 +- git-refspec/tests/fixtures/make_baseline.sh | 2 + git-refspec/tests/parse/invalid.rs | 59 +++++++++++++++++++ git-refspec/tests/parse/mod.rs | 56 +----------------- 5 files changed, 73 insertions(+), 60 deletions(-) create mode 100644 git-refspec/tests/parse/invalid.rs diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 5441bb872bf..21c607321ef 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -4,6 +4,8 @@ pub enum Error { Empty, #[error("Negative refspecs cannot have destinations as they exclude sources")] NegativeWithDestination, + #[error("Negative specs must not be empty")] + NegativeEmpty, #[error("Cannot push into an empty destination")] PushToEmpty, #[error("glob patterns may only involved a single '*' character, found {pattern:?}")] @@ -52,12 +54,12 @@ pub(crate) mod function { let (mut src, dst) = match spec.find_byte(b':') { Some(pos) => { - let (src, dst) = spec.split_at(pos); - let dst = &dst[1..]; if mode == Mode::Negative { return Err(Error::NegativeWithDestination); } + let (src, dst) = spec.split_at(pos); + let dst = &dst[1..]; let src = (!src.is_empty()).then(|| src.as_bstr()); let dst = (!dst.is_empty()).then(|| dst.as_bstr()); match (src, dst) { @@ -78,7 +80,7 @@ pub(crate) mod function { } None => { let src = (!spec.is_empty()).then(|| spec); - if Operation::Fetch == operation && src.is_none() { + if Operation::Fetch == operation && mode != Mode::Negative && src.is_none() { return Ok(fetch_head_only(mode)); } else { (src, None) @@ -86,6 +88,10 @@ pub(crate) mod function { } }; + if mode == Mode::Negative && src.is_none() { + return Err(Error::NegativeEmpty); + } + if let Some(spec) = src.as_mut() { if *spec == "@" { *spec = "HEAD".into(); diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz index 00433aa4fd8..6fbfc3d4c52 100644 --- a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:ace43ca904298dc76951863d41485ae0c21ee9b88abf7d604eb6c6e7efa18ab2 -size 9352 +oid sha256:6d2478d9f90c1e68924743a45cf801b4420924751c2caf7fa9a6c14291c388cd +size 9356 diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh index 23951021a29..f85c59e6f08 100644 --- a/git-refspec/tests/fixtures/make_baseline.sh +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -30,6 +30,8 @@ baseline fetch '^a:' baseline fetch '^a:b' baseline fetch '^:' baseline fetch '^:b' +baseline fetch '^' +baseline push '^' baseline fetch '^refs/heads/qa/*/*' baseline push '^refs/heads/qa/*/*' diff --git a/git-refspec/tests/parse/invalid.rs b/git-refspec/tests/parse/invalid.rs new file mode 100644 index 00000000000..d3f36a572dc --- /dev/null +++ b/git-refspec/tests/parse/invalid.rs @@ -0,0 +1,59 @@ +use crate::parse::try_parse; +use git_refspec::{parse::Error, Operation}; + +#[test] +fn empty() { + assert!(matches!(try_parse("", Operation::Push).unwrap_err(), Error::Empty)); +} + +#[test] +fn negative_must_not_be_empty() { + for op in [Operation::Fetch, Operation::Push] { + assert!(matches!(try_parse("^", op).unwrap_err(), Error::NegativeEmpty)); + } +} + +#[test] +fn negative_with_destination() { + for op in [Operation::Fetch, Operation::Push] { + for spec in ["^a:b", "^a:", "^:", "^:b"] { + assert!(matches!( + try_parse(spec, op).unwrap_err(), + Error::NegativeWithDestination + )); + } + } +} + +#[test] +fn complex_patterns_with_more_than_one_asterisk() { + for op in [Operation::Fetch, Operation::Push] { + for spec in ["^*/*", "a/*/c/*", "a**:**b", "+:**/"] { + assert!(matches!( + try_parse(spec, op).unwrap_err(), + Error::PatternUnsupported { .. } + )); + } + } +} + +#[test] +fn both_sides_need_pattern_if_one_uses_it() { + for op in [Operation::Fetch, Operation::Push] { + for spec in ["refs/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] { + assert!( + matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced), + "{}", + spec + ); + } + } +} + +#[test] +fn push_to_empty() { + assert!(matches!( + try_parse("HEAD:", Operation::Push).unwrap_err(), + Error::PushToEmpty + )); +} diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index eb9af0ab125..230cbde081b 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -56,62 +56,8 @@ fn baseline() { } } -mod invalid { - use crate::parse::try_parse; - use git_refspec::{parse::Error, Operation}; - - #[test] - fn empty() { - assert!(matches!(try_parse("", Operation::Push).unwrap_err(), Error::Empty)); - } - - #[test] - fn negative_with_destination() { - for op in [Operation::Fetch, Operation::Push] { - for spec in ["^a:b", "^a:", "^:", "^:b"] { - assert!(matches!( - try_parse(spec, op).unwrap_err(), - Error::NegativeWithDestination - )); - } - } - } - - #[test] - fn complex_patterns_with_more_than_one_asterisk() { - for op in [Operation::Fetch, Operation::Push] { - for spec in ["^*/*", "a/*/c/*", "a**:**b", "+:**/"] { - assert!(matches!( - try_parse(spec, op).unwrap_err(), - Error::PatternUnsupported { .. } - )); - } - } - } - - #[test] - fn both_sides_need_pattern_if_one_uses_it() { - for op in [Operation::Fetch, Operation::Push] { - for spec in ["refs/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] { - assert!( - matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced), - "{}", - spec - ); - } - } - } - - #[test] - fn push_to_empty() { - assert!(matches!( - try_parse("HEAD:", Operation::Push).unwrap_err(), - Error::PushToEmpty - )); - } -} - mod fetch; +mod invalid; mod push; mod util { From b8899532b461ebcdc0ecd33e54a8721e69136c22 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 09:36:26 +0800 Subject: [PATCH 26/36] don't allow object hashes in excludes (#450) --- git-refspec/src/parse.rs | 15 +++++++++++++-- git-refspec/tests/parse/invalid.rs | 10 ++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 21c607321ef..c9864ba9db1 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -6,6 +6,8 @@ pub enum Error { NegativeWithDestination, #[error("Negative specs must not be empty")] NegativeEmpty, + #[error("Negative specs must be object hashes")] + NegativeObjectHash, #[error("Cannot push into an empty destination")] PushToEmpty, #[error("glob patterns may only involved a single '*' character, found {pattern:?}")] @@ -88,8 +90,17 @@ pub(crate) mod function { } }; - if mode == Mode::Negative && src.is_none() { - return Err(Error::NegativeEmpty); + if mode == Mode::Negative { + match src { + Some(spec) => { + if spec.len() >= git_hash::Kind::shortest().len_in_hex() + && spec.iter().all(|b| b.is_ascii_hexdigit()) + { + return Err(Error::NegativeObjectHash); + } + } + None => return Err(Error::NegativeEmpty), + } } if let Some(spec) = src.as_mut() { diff --git a/git-refspec/tests/parse/invalid.rs b/git-refspec/tests/parse/invalid.rs index d3f36a572dc..561a3e9e3b8 100644 --- a/git-refspec/tests/parse/invalid.rs +++ b/git-refspec/tests/parse/invalid.rs @@ -13,6 +13,16 @@ fn negative_must_not_be_empty() { } } +#[test] +fn negative_must_not_be_object_hash() { + for op in [Operation::Fetch, Operation::Push] { + assert!(matches!( + try_parse("^e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", op).unwrap_err(), + Error::NegativeObjectHash + )); + } +} + #[test] fn negative_with_destination() { for op in [Operation::Fetch, Operation::Push] { From 9c280b2de59773c6d13134e3257cf1da5731e35d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 09:47:44 +0800 Subject: [PATCH 27/36] disallow excludes in push mode (#450) It's not documented in `git-push`, even though git parses it fine for some reason. --- git-refspec/src/parse.rs | 5 +++ git-refspec/src/spec.rs | 1 - git-refspec/src/types.rs | 7 +--- .../generated-archives/make_baseline.tar.xz | 4 +- git-refspec/tests/fixtures/make_baseline.sh | 7 +++- git-refspec/tests/parse/invalid.rs | 41 ++++++++++++------- git-refspec/tests/parse/push.rs | 7 ---- 7 files changed, 40 insertions(+), 32 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index c9864ba9db1..bba7e165b48 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -6,6 +6,8 @@ pub enum Error { NegativeWithDestination, #[error("Negative specs must not be empty")] NegativeEmpty, + #[error("Negative specs are only supported when fetching")] + NegativeUnsupported, #[error("Negative specs must be object hashes")] NegativeObjectHash, #[error("Cannot push into an empty destination")] @@ -39,6 +41,9 @@ pub(crate) mod function { let mode = match spec.get(0) { Some(&b'^') => { spec = &spec[1..]; + if operation == Operation::Push { + return Err(Error::NegativeUnsupported); + } Mode::Negative } Some(&b'+') => { diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index f74b6697766..17c79dd6957 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -43,7 +43,6 @@ impl RefSpecRef<'_> { dst, allow_non_fast_forward: matches!(self.mode, Mode::Force), }), - (Mode::Negative, Some(src), None) => Instruction::Push(Push::Exclude { src }), (mode, src, dest) => { unreachable!( "BUG: push instructions with {:?} {:?} {:?} are not possible", diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 969663c0342..3c26fa23056 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -40,11 +40,6 @@ pub enum Push<'a> { /// The reference or pattern to delete on the remote. ref_or_pattern: &'a BStr, }, - /// Exclude a single ref. - Exclude { - /// A full or partial ref name to exclude, or multiple if a single `*` is used. - src: &'a BStr, - }, /// Push a single ref or refspec to a known destination ref. Matching { /// The source ref or refspec to push. If pattern, it contains a single `*`. @@ -67,7 +62,7 @@ pub enum Fetch<'a> { }, /// Exclude a single ref. Exclude { - /// A single partial or full ref name to exclude on the remote, or a pattern with a single `*`. + /// A single partial or full ref name to exclude on the remote, or a pattern with a single `*`. It cannot be a spelled out object hash. src: &'a BStr, }, AndUpdate { diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz index 6fbfc3d4c52..ddf4c197cc5 100644 --- a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:6d2478d9f90c1e68924743a45cf801b4420924751c2caf7fa9a6c14291c388cd -size 9356 +oid sha256:3fe4f4ca2f442e58b594faaa0f512c6fdf1df8f052995928eca9bede909794e9 +size 9360 diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh index f85c59e6f08..fe5a82f4f3b 100644 --- a/git-refspec/tests/fixtures/make_baseline.sh +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -6,6 +6,7 @@ git init; function baseline() { local kind=$1 local refspec=$2 + local force_fail=${3:-} cat <.git/config [remote "test"] @@ -14,6 +15,10 @@ function baseline() { EOF git ls-remote "test" && status=0 || status=$? + if [ -n "$force_fail" ]; then + status=128 + fi + { echo "$kind" "$refspec" echo "$status" @@ -82,7 +87,7 @@ baseline fetch 'HEAD' baseline push '@' baseline fetch '@' -baseline push '^@' +baseline push '^@' fail baseline fetch '^@' baseline push '+@' diff --git a/git-refspec/tests/parse/invalid.rs b/git-refspec/tests/parse/invalid.rs index 561a3e9e3b8..bd823ec946c 100644 --- a/git-refspec/tests/parse/invalid.rs +++ b/git-refspec/tests/parse/invalid.rs @@ -8,43 +8,54 @@ fn empty() { #[test] fn negative_must_not_be_empty() { - for op in [Operation::Fetch, Operation::Push] { - assert!(matches!(try_parse("^", op).unwrap_err(), Error::NegativeEmpty)); - } + assert!(matches!( + try_parse("^", Operation::Fetch).unwrap_err(), + Error::NegativeEmpty + )); } #[test] fn negative_must_not_be_object_hash() { - for op in [Operation::Fetch, Operation::Push] { + assert!(matches!( + try_parse("^e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", Operation::Fetch).unwrap_err(), + Error::NegativeObjectHash + )); +} + +#[test] +fn negative_with_destination() { + for spec in ["^a:b", "^a:", "^:", "^:b"] { assert!(matches!( - try_parse("^e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", op).unwrap_err(), - Error::NegativeObjectHash + try_parse(spec, Operation::Fetch).unwrap_err(), + Error::NegativeWithDestination )); } } #[test] -fn negative_with_destination() { - for op in [Operation::Fetch, Operation::Push] { - for spec in ["^a:b", "^a:", "^:", "^:b"] { - assert!(matches!( - try_parse(spec, op).unwrap_err(), - Error::NegativeWithDestination - )); - } +fn negative_unsupported_when_pushing() { + for spec in ["^a:b", "^a:", "^:", "^:b", "^"] { + assert!(matches!( + try_parse(spec, Operation::Push).unwrap_err(), + Error::NegativeUnsupported + )); } } #[test] fn complex_patterns_with_more_than_one_asterisk() { for op in [Operation::Fetch, Operation::Push] { - for spec in ["^*/*", "a/*/c/*", "a**:**b", "+:**/"] { + for spec in ["a/*/c/*", "a**:**b", "+:**/"] { assert!(matches!( try_parse(spec, op).unwrap_err(), Error::PatternUnsupported { .. } )); } } + assert!(matches!( + try_parse("^*/*", Operation::Fetch).unwrap_err(), + Error::PatternUnsupported { .. } + )); } #[test] diff --git a/git-refspec/tests/parse/push.rs b/git-refspec/tests/parse/push.rs index c133b672ac6..bc99a390df1 100644 --- a/git-refspec/tests/parse/push.rs +++ b/git-refspec/tests/parse/push.rs @@ -1,12 +1,6 @@ use crate::parse::{assert_parse, b}; use git_refspec::{Instruction, Mode, Push}; -#[test] -fn exclude() { - assert_parse("^a", Instruction::Push(Push::Exclude { src: b("a") })); - assert_parse("^a*", Instruction::Push(Push::Exclude { src: b("a*") })); -} - #[test] fn ampersand_is_resolved_to_head() { assert_parse( @@ -26,7 +20,6 @@ fn ampersand_is_resolved_to_head() { allow_non_fast_forward: true, }), ); - assert_parse("^@", Instruction::Push(Push::Exclude { src: b("HEAD") })); } #[test] From 57a6e695697744459149dd6400c54b9c4c88a365 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 10:15:15 +0800 Subject: [PATCH 28/36] more detailed tests of what's allowed and where (#450) --- git-refspec/src/parse.rs | 13 +++++-- git-refspec/tests/parse/fetch.rs | 58 ++++++++++++++++++++++++++++-- git-refspec/tests/parse/invalid.rs | 36 ------------------- git-refspec/tests/parse/push.rs | 14 ++++++-- 4 files changed, 78 insertions(+), 43 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index bba7e165b48..6382293de90 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -10,6 +10,8 @@ pub enum Error { NegativeUnsupported, #[error("Negative specs must be object hashes")] NegativeObjectHash, + #[error("Fetch destinations must be ref-names, like 'HEAD:refs/heads/branch'")] + InvalidFetchDestination, #[error("Cannot push into an empty destination")] PushToEmpty, #[error("glob patterns may only involved a single '*' character, found {pattern:?}")] @@ -98,9 +100,7 @@ pub(crate) mod function { if mode == Mode::Negative { match src { Some(spec) => { - if spec.len() >= git_hash::Kind::shortest().len_in_hex() - && spec.iter().all(|b| b.is_ascii_hexdigit()) - { + if looks_like_object_hash(spec) { return Err(Error::NegativeObjectHash); } } @@ -115,6 +115,9 @@ pub(crate) mod function { } let (src, src_had_pattern) = validated(src, operation == Operation::Push)?; let (dst, dst_had_pattern) = validated(dst, false)?; + if !dst_had_pattern && looks_like_object_hash(dst.unwrap_or_default()) { + return Err(Error::InvalidFetchDestination); + } if mode != Mode::Negative && src_had_pattern != dst_had_pattern { return Err(Error::PatternUnbalanced); } @@ -126,6 +129,10 @@ pub(crate) mod function { }) } + fn looks_like_object_hash(spec: &BStr) -> bool { + spec.len() >= git_hash::Kind::shortest().len_in_hex() && spec.iter().all(|b| b.is_ascii_hexdigit()) + } + fn validated(spec: Option<&BStr>, allow_revspecs: bool) -> Result<(Option<&BStr>, bool), Error> { match spec { Some(spec) => { diff --git a/git-refspec/tests/parse/fetch.rs b/git-refspec/tests/parse/fetch.rs index 22d2973b682..c8eb5acea26 100644 --- a/git-refspec/tests/parse/fetch.rs +++ b/git-refspec/tests/parse/fetch.rs @@ -1,5 +1,59 @@ -use crate::parse::{assert_parse, b}; -use git_refspec::{Fetch, Instruction, Mode}; +use crate::parse::{assert_parse, b, try_parse}; +use git_refspec::{parse::Error, Fetch, Instruction, Mode, Operation}; + +#[test] +fn revspecs_are_disallowed() { + for spec in ["main~1", "^@^{}", "HEAD:main~1"] { + assert!(matches!( + try_parse(spec, Operation::Fetch).unwrap_err(), + Error::ReferenceName(_) + )); + } +} + +#[test] +fn object_hash_as_source() { + assert_parse( + "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391:", + Instruction::Fetch(Fetch::Only { + src: b("e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"), + }), + ); +} + +#[test] +fn object_hash_destination_is_invalid() { + assert!(matches!( + try_parse("a:e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", Operation::Fetch).unwrap_err(), + Error::InvalidFetchDestination + )); +} + +#[test] +fn negative_must_not_be_empty() { + assert!(matches!( + try_parse("^", Operation::Fetch).unwrap_err(), + Error::NegativeEmpty + )); +} + +#[test] +fn negative_must_not_be_object_hash() { + assert!(matches!( + try_parse("^e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", Operation::Fetch).unwrap_err(), + Error::NegativeObjectHash + )); +} + +#[test] +fn negative_with_destination() { + for spec in ["^a:b", "^a:", "^:", "^:b"] { + assert!(matches!( + try_parse(spec, Operation::Fetch).unwrap_err(), + Error::NegativeWithDestination + )); + } +} #[test] fn exclude() { diff --git a/git-refspec/tests/parse/invalid.rs b/git-refspec/tests/parse/invalid.rs index bd823ec946c..9f8af9aa142 100644 --- a/git-refspec/tests/parse/invalid.rs +++ b/git-refspec/tests/parse/invalid.rs @@ -6,42 +6,6 @@ fn empty() { assert!(matches!(try_parse("", Operation::Push).unwrap_err(), Error::Empty)); } -#[test] -fn negative_must_not_be_empty() { - assert!(matches!( - try_parse("^", Operation::Fetch).unwrap_err(), - Error::NegativeEmpty - )); -} - -#[test] -fn negative_must_not_be_object_hash() { - assert!(matches!( - try_parse("^e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", Operation::Fetch).unwrap_err(), - Error::NegativeObjectHash - )); -} - -#[test] -fn negative_with_destination() { - for spec in ["^a:b", "^a:", "^:", "^:b"] { - assert!(matches!( - try_parse(spec, Operation::Fetch).unwrap_err(), - Error::NegativeWithDestination - )); - } -} - -#[test] -fn negative_unsupported_when_pushing() { - for spec in ["^a:b", "^a:", "^:", "^:b", "^"] { - assert!(matches!( - try_parse(spec, Operation::Push).unwrap_err(), - Error::NegativeUnsupported - )); - } -} - #[test] fn complex_patterns_with_more_than_one_asterisk() { for op in [Operation::Fetch, Operation::Push] { diff --git a/git-refspec/tests/parse/push.rs b/git-refspec/tests/parse/push.rs index bc99a390df1..d8e0f9dde4c 100644 --- a/git-refspec/tests/parse/push.rs +++ b/git-refspec/tests/parse/push.rs @@ -1,5 +1,15 @@ -use crate::parse::{assert_parse, b}; -use git_refspec::{Instruction, Mode, Push}; +use crate::parse::{assert_parse, b, try_parse}; +use git_refspec::{parse::Error, Instruction, Mode, Operation, Push}; + +#[test] +fn negative_unsupported() { + for spec in ["^a:b", "^a:", "^:", "^:b", "^"] { + assert!(matches!( + try_parse(spec, Operation::Push).unwrap_err(), + Error::NegativeUnsupported + )); + } +} #[test] fn ampersand_is_resolved_to_head() { From bb992acc13fc2a63ec5098e9fa8954909ef486ca Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 10:27:58 +0800 Subject: [PATCH 29/36] more push-spec restrictions (#450) --- git-refspec/src/parse.rs | 2 +- .../generated-archives/make_baseline.tar.xz | 4 +-- git-refspec/tests/fixtures/make_baseline.sh | 4 +++ git-refspec/tests/parse/push.rs | 36 +++++++++++++++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 6382293de90..2fa5783ee3d 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -113,7 +113,7 @@ pub(crate) mod function { *spec = "HEAD".into(); } } - let (src, src_had_pattern) = validated(src, operation == Operation::Push)?; + let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some())?; let (dst, dst_had_pattern) = validated(dst, false)?; if !dst_had_pattern && looks_like_object_hash(dst.unwrap_or_default()) { return Err(Error::InvalidFetchDestination); diff --git a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz index ddf4c197cc5..aa3836355df 100644 --- a/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz +++ b/git-refspec/tests/fixtures/generated-archives/make_baseline.tar.xz @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:3fe4f4ca2f442e58b594faaa0f512c6fdf1df8f052995928eca9bede909794e9 -size 9360 +oid sha256:9d52f5fe25601c545187b0ac19b004f30488937b6907e8206333d2ba923c7b29 +size 9372 diff --git a/git-refspec/tests/fixtures/make_baseline.sh b/git-refspec/tests/fixtures/make_baseline.sh index fe5a82f4f3b..3e78ce9788a 100644 --- a/git-refspec/tests/fixtures/make_baseline.sh +++ b/git-refspec/tests/fixtures/make_baseline.sh @@ -40,6 +40,10 @@ baseline push '^' baseline fetch '^refs/heads/qa/*/*' baseline push '^refs/heads/qa/*/*' +baseline push 'main~1' +baseline fetch 'main~1' +baseline push 'main~1:other~1' +baseline push ':main~1' baseline push 'refs/heads/*:refs/remotes/frotz' baseline push 'refs/heads:refs/remotes/frotz/*' diff --git a/git-refspec/tests/parse/push.rs b/git-refspec/tests/parse/push.rs index d8e0f9dde4c..05531e02729 100644 --- a/git-refspec/tests/parse/push.rs +++ b/git-refspec/tests/parse/push.rs @@ -11,6 +11,42 @@ fn negative_unsupported() { } } +#[test] +fn revspecs_with_ref_name_destination() { + assert_parse( + "main~1:b", + Instruction::Push(Push::Matching { + src: b("main~1"), + dst: b("b"), + allow_non_fast_forward: false, + }), + ); + assert_parse( + "+main~1:b", + Instruction::Push(Push::Matching { + src: b("main~1"), + dst: b("b"), + allow_non_fast_forward: true, + }), + ); +} + +#[test] +fn destinations_must_be_ref_names() { + assert!(matches!( + try_parse("a~1:b~1", Operation::Push).unwrap_err(), + Error::ReferenceName(_) + )); +} + +#[test] +fn single_refs_must_be_refnames() { + assert!(matches!( + try_parse("a~1", Operation::Push).unwrap_err(), + Error::ReferenceName(_) + )); +} + #[test] fn ampersand_is_resolved_to_head() { assert_parse( From 62d721a5a7260adb408415810899bcf11d524d0c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 10:36:14 +0800 Subject: [PATCH 30/36] Add fuzz target (#450) --- git-refspec/fuzz/.gitignore | 3 + git-refspec/fuzz/Cargo.lock | 397 +++++++++++++++++++++++++ git-refspec/fuzz/Cargo.toml | 25 ++ git-refspec/fuzz/fuzz_targets/parse.rs | 7 + 4 files changed, 432 insertions(+) create mode 100644 git-refspec/fuzz/.gitignore create mode 100644 git-refspec/fuzz/Cargo.lock create mode 100644 git-refspec/fuzz/Cargo.toml create mode 100644 git-refspec/fuzz/fuzz_targets/parse.rs diff --git a/git-refspec/fuzz/.gitignore b/git-refspec/fuzz/.gitignore new file mode 100644 index 00000000000..a0925114d61 --- /dev/null +++ b/git-refspec/fuzz/.gitignore @@ -0,0 +1,3 @@ +target +corpus +artifacts diff --git a/git-refspec/fuzz/Cargo.lock b/git-refspec/fuzz/Cargo.lock new file mode 100644 index 00000000000..a5e6eb61092 --- /dev/null +++ b/git-refspec/fuzz/Cargo.lock @@ -0,0 +1,397 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "arbitrary" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a7924531f38b1970ff630f03eb20a2fde69db5c590c93b0f3482e95dcc5fd60" + +[[package]] +name = "autocfg" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" + +[[package]] +name = "bstr" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba3569f383e8f1598449f1a423e72e99569137b47740b1da11ef19af3d5c3223" +dependencies = [ + "lazy_static", + "memchr", + "regex-automata", +] + +[[package]] +name = "btoi" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97c0869a9faa81f8bbf8102371105d6d0a7b79167a04c340b04ab16892246a11" +dependencies = [ + "num-traits", +] + +[[package]] +name = "bumpalo" +version = "3.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "37ccbd214614c6783386c1af30caf03192f17891059cecc394b4fb119e363de3" + +[[package]] +name = "cc" +version = "1.0.73" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fff2a6927b3bb87f9595d67196a70493f627687a71d87a0d692242c33f58c11" + +[[package]] +name = "cfg-if" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" + +[[package]] +name = "git-actor" +version = "0.11.0" +dependencies = [ + "bstr", + "btoi", + "git-date", + "itoa", + "nom", + "quick-error", +] + +[[package]] +name = "git-date" +version = "0.0.2" +dependencies = [ + "bstr", + "itoa", + "time", +] + +[[package]] +name = "git-features" +version = "0.22.0" +dependencies = [ + "git-hash", + "libc", + "sha1_smol", +] + +[[package]] +name = "git-hash" +version = "0.9.6" +dependencies = [ + "hex", + "quick-error", +] + +[[package]] +name = "git-object" +version = "0.20.0" +dependencies = [ + "bstr", + "btoi", + "git-actor", + "git-features", + "git-hash", + "git-validate", + "hex", + "itoa", + "nom", + "quick-error", + "smallvec", +] + +[[package]] +name = "git-refspec" +version = "0.0.0" +dependencies = [ + "bstr", + "git-hash", + "git-revision", + "git-validate", + "smallvec", + "thiserror", +] + +[[package]] +name = "git-refspec-fuzz" +version = "0.0.0" +dependencies = [ + "git-refspec", + "libfuzzer-sys", +] + +[[package]] +name = "git-revision" +version = "0.3.0" +dependencies = [ + "bstr", + "git-date", + "git-hash", + "git-object", + "hash_hasher", + "thiserror", +] + +[[package]] +name = "git-validate" +version = "0.5.4" +dependencies = [ + "bstr", + "quick-error", +] + +[[package]] +name = "hash_hasher" +version = "2.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74721d007512d0cb3338cd20f0654ac913920061a4c4d0d8708edb3f2a698c0c" + +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + +[[package]] +name = "itoa" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c8af84674fe1f223a982c933a0ee1086ac4d4052aa0fb8060c12c6ad838e754" + +[[package]] +name = "js-sys" +version = "0.3.59" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "258451ab10b34f8af53416d1fdab72c22e805f0c92a1136d59470ec0b11138b2" +dependencies = [ + "wasm-bindgen", +] + +[[package]] +name = "lazy_static" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" + +[[package]] +name = "libc" +version = "0.2.127" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "505e71a4706fa491e9b1b55f51b95d4037d0821ee40131190475f692b35b009b" + +[[package]] +name = "libfuzzer-sys" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "336244aaeab6a12df46480dc585802aa743a72d66b11937844c61bbca84c991d" +dependencies = [ + "arbitrary", + "cc", + "once_cell", +] + +[[package]] +name = "log" +version = "0.4.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "abb12e687cfb44aa40f41fc3978ef76448f9b6038cad6aef4259d3c095a2382e" +dependencies = [ + "cfg-if", +] + +[[package]] +name = "memchr" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" + +[[package]] +name = "minimal-lexical" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" + +[[package]] +name = "nom" +version = "7.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8903e5a29a317527874d0402f867152a3d21c908bb0b933e416c65e301d4c36" +dependencies = [ + "memchr", + "minimal-lexical", +] + +[[package]] +name = "num-traits" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd" +dependencies = [ + "autocfg", +] + +[[package]] +name = "num_threads" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2819ce041d2ee131036f4fc9d6ae7ae125a3a40e97ba64d04fe799ad9dabbb44" +dependencies = [ + "libc", +] + +[[package]] +name = "once_cell" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "18a6dbe30758c9f83eb00cbea4ac95966305f5a7772f3f42ebfc7fc7eddbd8e1" + +[[package]] +name = "proc-macro2" +version = "1.0.43" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a2ca2c61bc9f3d74d2886294ab7b9853abd9c1ad903a3ac7815c58989bb7bab" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quick-error" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" + +[[package]] +name = "quote" +version = "1.0.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbe448f377a7d6961e30f5955f9b8d106c3f5e449d493ee1b125c1d43c2b5179" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "regex-automata" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c230d73fb8d8c1b9c0b3135c5142a8acee3a0558fb8db5cf1cb65f8d7862132" + +[[package]] +name = "sha1_smol" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae1a47186c03a32177042e55dbc5fd5aee900b8e0069a8d70fba96a9375cd012" + +[[package]] +name = "smallvec" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fd0db749597d91ff862fd1d55ea87f7855a744a8425a64695b6fca237d1dad1" + +[[package]] +name = "syn" +version = "1.0.99" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58dbef6ec655055e20b86b15a8cc6d439cca19b667537ac6a1369572d151ab13" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "thiserror" +version = "1.0.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f5f6586b7f764adc0231f4c79be7b920e766bb2f3e51b3661cdb263828f19994" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "12bafc5b54507e0149cdf1b145a5d80ab80a90bcd9275df43d4fff68460f6c21" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "time" +version = "0.3.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74b7cc93fc23ba97fde84f7eea56c55d1ba183f495c6715defdfc7b9cb8c870f" +dependencies = [ + "js-sys", + "libc", + "num_threads", +] + +[[package]] +name = "unicode-ident" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4f5b37a154999a8f3f98cc23a628d850e154479cd94decf3414696e12e31aaf" + +[[package]] +name = "wasm-bindgen" +version = "0.2.82" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc7652e3f6c4706c8d9cd54832c4a4ccb9b5336e2c3bd154d5cccfbf1c1f5f7d" +dependencies = [ + "cfg-if", + "wasm-bindgen-macro", +] + +[[package]] +name = "wasm-bindgen-backend" +version = "0.2.82" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "662cd44805586bd52971b9586b1df85cdbbd9112e4ef4d8f41559c334dc6ac3f" +dependencies = [ + "bumpalo", + "log", + "once_cell", + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-macro" +version = "0.2.82" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b260f13d3012071dfb1512849c033b1925038373aea48ced3012c09df952c602" +dependencies = [ + "quote", + "wasm-bindgen-macro-support", +] + +[[package]] +name = "wasm-bindgen-macro-support" +version = "0.2.82" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5be8e654bdd9b79216c2929ab90721aa82faf65c48cdf08bdc4e7f51357b80da" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-backend", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-shared" +version = "0.2.82" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6598dd0bd3c7d51095ff6531a5b23e02acdc81804e30d8f07afb77b7215a140a" diff --git a/git-refspec/fuzz/Cargo.toml b/git-refspec/fuzz/Cargo.toml new file mode 100644 index 00000000000..bcbabbb425a --- /dev/null +++ b/git-refspec/fuzz/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "git-refspec-fuzz" +version = "0.0.0" +authors = ["Automatically generated"] +publish = false +edition = "2018" + +[package.metadata] +cargo-fuzz = true + +[dependencies] +libfuzzer-sys = "0.4" + +[dependencies.git-refspec] +path = ".." + +# Prevent this from interfering with workspaces +[workspace] +members = ["."] + +[[bin]] +name = "parse" +path = "fuzz_targets/parse.rs" +test = false +doc = false diff --git a/git-refspec/fuzz/fuzz_targets/parse.rs b/git-refspec/fuzz/fuzz_targets/parse.rs new file mode 100644 index 00000000000..c319b20f35e --- /dev/null +++ b/git-refspec/fuzz/fuzz_targets/parse.rs @@ -0,0 +1,7 @@ +#![no_main] +use libfuzzer_sys::fuzz_target; + +fuzz_target!(|data: &[u8]| { + drop(git_refspec::parse(data.into(), git_refspec::Operation::Push)); + drop(git_refspec::parse(data.into(), git_refspec::Operation::Fetch)); +}); From febf0706b83b36a71efbe669ee760c2d4ef14b72 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 10:39:30 +0800 Subject: [PATCH 31/36] add fuzz target and basic docs on how to run it (#450) --- git-refspec/README.md | 11 +++++++++++ git-revision/Cargo.toml | 2 +- git-revision/README.md | 11 +++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 git-refspec/README.md create mode 100644 git-revision/README.md diff --git a/git-refspec/README.md b/git-refspec/README.md new file mode 100644 index 00000000000..022495e4f27 --- /dev/null +++ b/git-refspec/README.md @@ -0,0 +1,11 @@ +# `git-refspec` + +### Testing + +#### Fuzzing + +`cargo fuzz` is used for fuzzing, installable with `cargo install cargo-fuzz`. + +Targets can be listed with `cargo fuzz list` and executed via `cargo +nightly fuzz run `, +where `` can be `parse` for example. + diff --git a/git-revision/Cargo.toml b/git-revision/Cargo.toml index f1490626e65..ada748fd7f3 100644 --- a/git-revision/Cargo.toml +++ b/git-revision/Cargo.toml @@ -6,7 +6,7 @@ license = "MIT/Apache-2.0" description = "A WIP crate of the gitoxide project dealing with finding names for revisions and parsing specifications" authors = ["Sebastian Thiel "] edition = "2018" -include = ["src/**/*", "CHANGELOG.md"] +include = ["src/**/*", "CHANGELOG.md", "README.md"] [lib] doctest = false diff --git a/git-revision/README.md b/git-revision/README.md new file mode 100644 index 00000000000..8d00185fac2 --- /dev/null +++ b/git-revision/README.md @@ -0,0 +1,11 @@ +# `git-revision` + +### Testing + +#### Fuzzing + +`cargo fuzz` is used for fuzzing, installable with `cargo install cargo-fuzz`. + +Targets can be listed with `cargo fuzz list` and executed via `cargo +nightly fuzz run `, +where `` can be `parse` for example. + From c695a7e9716262d885c3ccfde68eb2077650b8ce Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 10:56:23 +0800 Subject: [PATCH 32/36] improve docs (#450) --- git-refspec/src/lib.rs | 5 +++-- git-refspec/src/parse.rs | 2 ++ git-refspec/src/spec.rs | 19 +++++++++++++++++++ git-refspec/src/types.rs | 6 ++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index 3da9be64f14..c509c7e984a 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -1,7 +1,8 @@ -//! Parse [ref specs]() and represent them. +//! Parse git ref-specs and represent them. #![forbid(unsafe_code, rust_2018_idioms)] -#![allow(missing_docs)] +#![deny(missing_docs)] +/// pub mod parse; pub use parse::function::parse; diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 2fa5783ee3d..2ef061fa811 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -1,4 +1,6 @@ +/// The error returned by the [`parse()`][crate::parse()] function. #[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] pub enum Error { #[error("Empty refspecs are invalid")] Empty, diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index 17c79dd6957..c8afd38b744 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -1,6 +1,25 @@ use crate::types::Push; use crate::{Fetch, Instruction, Mode, Operation, RefSpec, RefSpecRef}; +/// Conversion. Use the [RefSpecRef][RefSpec::to_ref()] type for more usage options. +impl RefSpec { + /// Return ourselves as reference type. + pub fn to_ref(&self) -> RefSpecRef<'_> { + RefSpecRef { + mode: self.mode, + op: self.op, + src: self.src.as_ref().map(|b| b.as_ref()), + dst: self.dst.as_ref().map(|b| b.as_ref()), + } + } +} + +impl Into for RefSpecRef<'_> { + fn into(self) -> RefSpec { + self.to_owned() + } +} + /// Access impl RefSpecRef<'_> { /// Return the refspec mode. diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 3c26fa23056..3bb11de4c50 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -20,9 +20,12 @@ pub enum Operation { Fetch, } +/// Tells what to do and is derived from a [`RefSpec`][RefSpecRef]. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Instruction<'a> { + /// An instruction for pushing. Push(Push<'a>), + /// An instruction for fetching. Fetch(Fetch<'a>), } @@ -56,6 +59,7 @@ pub enum Push<'a> { /// Destinations can only be a partial or full ref-names on the local side. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Fetch<'a> { + /// Fetch a ref or refs and write the result into the `FETCH_HEAD` without updating local branches. Only { /// The ref name to fetch on the remote side, without updating the local side. This will write the result into `FETCH_HEAD`. src: &'a BStr, @@ -65,6 +69,7 @@ pub enum Fetch<'a> { /// A single partial or full ref name to exclude on the remote, or a pattern with a single `*`. It cannot be a spelled out object hash. src: &'a BStr, }, + /// Fetch from `src` and update the corresponding destination branches in `dst` accordingly. AndUpdate { /// The ref name to fetch on the remote side, or a pattern with a single `*` to match against. src: &'a BStr, @@ -77,6 +82,7 @@ pub enum Fetch<'a> { } impl Instruction<'_> { + /// Derive the mode of operation from this instruction. pub fn operation(&self) -> Operation { match self { Instruction::Push(_) => Operation::Push, From 627896655ce04e1a306c5bf3970ebd6e73bb1a5d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 10:57:35 +0800 Subject: [PATCH 33/36] don't expose mode() as it's kind of messy and should be left as implementation detail (#450) --- git-refspec/src/lib.rs | 6 +++--- git-refspec/src/parse.rs | 2 +- git-refspec/src/spec.rs | 7 +------ git-refspec/src/types.rs | 2 +- git-refspec/tests/parse/fetch.rs | 12 +++--------- git-refspec/tests/parse/push.rs | 9 ++------- 6 files changed, 11 insertions(+), 27 deletions(-) diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index c509c7e984a..eef2b9fd2d5 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -9,7 +9,7 @@ pub use parse::function::parse; /// A refspec with references to the memory it was parsed from. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub struct RefSpecRef<'a> { - mode: Mode, + mode: types::Mode, op: Operation, src: Option<&'a bstr::BStr>, dst: Option<&'a bstr::BStr>, @@ -18,7 +18,7 @@ pub struct RefSpecRef<'a> { /// An owned refspec. #[derive(PartialEq, Eq, Clone, Hash, Debug)] pub struct RefSpec { - mode: Mode, + mode: types::Mode, op: Operation, src: Option, dst: Option, @@ -27,4 +27,4 @@ pub struct RefSpec { mod spec; mod types; -pub use types::{Fetch, Instruction, Mode, Operation, Push}; +pub use types::{Fetch, Instruction, Operation, Push}; diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index 2ef061fa811..c2dc2ce9764 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -28,7 +28,7 @@ pub enum Error { pub(crate) mod function { use crate::parse::Error; - use crate::{Mode, Operation, RefSpecRef}; + use crate::{types::Mode, Operation, RefSpecRef}; use bstr::{BStr, ByteSlice}; /// Parse `spec` for use in `operation` and return it if it is valid. diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index c8afd38b744..db159824c3d 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -1,5 +1,5 @@ use crate::types::Push; -use crate::{Fetch, Instruction, Mode, Operation, RefSpec, RefSpecRef}; +use crate::{types::Mode, Fetch, Instruction, Operation, RefSpec, RefSpecRef}; /// Conversion. Use the [RefSpecRef][RefSpec::to_ref()] type for more usage options. impl RefSpec { @@ -22,11 +22,6 @@ impl Into for RefSpecRef<'_> { /// Access impl RefSpecRef<'_> { - /// Return the refspec mode. - pub fn mode(&self) -> Mode { - self.mode - } - /// Transform the state of the refspec into an instruction making clear what to do with it. pub fn instruction(&self) -> Instruction<'_> { match self.op { diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 3bb11de4c50..115bf5108b4 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -2,7 +2,7 @@ use bstr::BStr; /// The way to interpret a refspec. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] -pub enum Mode { +pub(crate) enum Mode { /// Apply standard rules for refspecs which are including refs with specific rules related to allowing fast forwards of destinations. Normal, /// Even though according to normal rules a non-fastforward would be denied, override this and reset a ref forcefully in the destination. diff --git a/git-refspec/tests/parse/fetch.rs b/git-refspec/tests/parse/fetch.rs index c8eb5acea26..f030a747050 100644 --- a/git-refspec/tests/parse/fetch.rs +++ b/git-refspec/tests/parse/fetch.rs @@ -1,5 +1,5 @@ use crate::parse::{assert_parse, b, try_parse}; -use git_refspec::{parse::Error, Fetch, Instruction, Mode, Operation}; +use git_refspec::{parse::Error, Fetch, Instruction, Operation}; #[test] fn revspecs_are_disallowed() { @@ -71,12 +71,7 @@ fn ampersand_is_resolved_to_head() { #[test] fn lhs_colon_empty_fetches_only() { assert_parse("src:", Instruction::Fetch(Fetch::Only { src: b("src") })); - let spec = assert_parse("+src:", Instruction::Fetch(Fetch::Only { src: b("src") })); - assert_eq!( - spec.mode(), - Mode::Force, - "force is set, even though it has no effect in the actual instruction" - ); + assert_parse("+src:", Instruction::Fetch(Fetch::Only { src: b("src") })); } #[test] @@ -140,8 +135,7 @@ fn empty_lhs_colon_rhs_fetches_head_to_destination() { #[test] fn colon_alone_is_for_fetching_head_into_fetchhead() { assert_parse(":", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); - let spec = assert_parse("+:", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); - assert_eq!(spec.mode(), Mode::Force, "it's set even though it's not useful"); + assert_parse("+:", Instruction::Fetch(Fetch::Only { src: b("HEAD") })); } #[test] diff --git a/git-refspec/tests/parse/push.rs b/git-refspec/tests/parse/push.rs index 05531e02729..8b4e307dd25 100644 --- a/git-refspec/tests/parse/push.rs +++ b/git-refspec/tests/parse/push.rs @@ -1,5 +1,5 @@ use crate::parse::{assert_parse, b, try_parse}; -use git_refspec::{parse::Error, Instruction, Mode, Operation, Push}; +use git_refspec::{parse::Error, Instruction, Operation, Push}; #[test] fn negative_unsupported() { @@ -123,10 +123,5 @@ fn colon_alone_is_for_pushing_matching_refs() { #[test] fn delete() { assert_parse(":a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); - let spec = assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); - assert_eq!( - spec.mode(), - Mode::Force, - "force is set, even though it has no effect in the actual instruction" - ); + assert_parse("+:a", Instruction::Push(Push::Delete { ref_or_pattern: b("a") })); } From f0163c99bccfb7d6719217a8fa773667cabe95fd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 11:06:56 +0800 Subject: [PATCH 34/36] cleanup crate structure (#450) --- git-refspec/src/instruction.rs | 64 ++++++++++++++++++++++++ git-refspec/src/lib.rs | 9 ++-- git-refspec/src/parse.rs | 11 ++++- git-refspec/src/spec.rs | 14 ++++-- git-refspec/src/types.rs | 79 ++---------------------------- git-refspec/tests/parse/fetch.rs | 2 +- git-refspec/tests/parse/invalid.rs | 2 +- git-refspec/tests/parse/mod.rs | 4 +- git-refspec/tests/parse/push.rs | 2 +- 9 files changed, 98 insertions(+), 89 deletions(-) create mode 100644 git-refspec/src/instruction.rs diff --git a/git-refspec/src/instruction.rs b/git-refspec/src/instruction.rs new file mode 100644 index 00000000000..ceceb9db79d --- /dev/null +++ b/git-refspec/src/instruction.rs @@ -0,0 +1,64 @@ +use crate::{parse::Operation, Instruction}; +use bstr::BStr; + +impl Instruction<'_> { + /// Derive the mode of operation from this instruction. + pub fn operation(&self) -> Operation { + match self { + Instruction::Push(_) => Operation::Push, + Instruction::Fetch(_) => Operation::Fetch, + } + } +} + +/// Note that all sources can either be a ref-name, partial or full, or a rev-spec, unless specified otherwise, on the local side. +/// Destinations can only be a partial or full ref names on the remote side. +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] +pub enum Push<'a> { + /// Push all local branches to the matching destination on the remote, which has to exist to be updated. + AllMatchingBranches { + /// If true, allow non-fast-forward updates of the matched destination branch. + allow_non_fast_forward: bool, + }, + /// Delete the destination ref or glob pattern, with only a single `*` allowed. + Delete { + /// The reference or pattern to delete on the remote. + ref_or_pattern: &'a BStr, + }, + /// Push a single ref or refspec to a known destination ref. + Matching { + /// The source ref or refspec to push. If pattern, it contains a single `*`. + src: &'a BStr, + /// The ref to update with the object from `src`. If `src` is a pattern, this is a pattern too. + dst: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, +} + +/// Any source can either be a ref name (full or partial) or a fully spelled out hex-sha for an object, on the remote side. +/// +/// Destinations can only be a partial or full ref-names on the local side. +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] +pub enum Fetch<'a> { + /// Fetch a ref or refs and write the result into the `FETCH_HEAD` without updating local branches. + Only { + /// The ref name to fetch on the remote side, without updating the local side. This will write the result into `FETCH_HEAD`. + src: &'a BStr, + }, + /// Exclude a single ref. + Exclude { + /// A single partial or full ref name to exclude on the remote, or a pattern with a single `*`. It cannot be a spelled out object hash. + src: &'a BStr, + }, + /// Fetch from `src` and update the corresponding destination branches in `dst` accordingly. + AndUpdate { + /// The ref name to fetch on the remote side, or a pattern with a single `*` to match against. + src: &'a BStr, + /// The local destination to update with what was fetched, or a pattern whose single `*` will be replaced with the matching portion + /// of the `*` from `src`. + dst: &'a BStr, + /// If true, allow non-fast-forward updates of `dest`. + allow_non_fast_forward: bool, + }, +} diff --git a/git-refspec/src/lib.rs b/git-refspec/src/lib.rs index eef2b9fd2d5..5f25a43c497 100644 --- a/git-refspec/src/lib.rs +++ b/git-refspec/src/lib.rs @@ -6,11 +6,14 @@ pub mod parse; pub use parse::function::parse; +/// +pub mod instruction; + /// A refspec with references to the memory it was parsed from. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub struct RefSpecRef<'a> { mode: types::Mode, - op: Operation, + op: parse::Operation, src: Option<&'a bstr::BStr>, dst: Option<&'a bstr::BStr>, } @@ -19,7 +22,7 @@ pub struct RefSpecRef<'a> { #[derive(PartialEq, Eq, Clone, Hash, Debug)] pub struct RefSpec { mode: types::Mode, - op: Operation, + op: parse::Operation, src: Option, dst: Option, } @@ -27,4 +30,4 @@ pub struct RefSpec { mod spec; mod types; -pub use types::{Fetch, Instruction, Operation, Push}; +pub use types::Instruction; diff --git a/git-refspec/src/parse.rs b/git-refspec/src/parse.rs index c2dc2ce9764..496ef202bca 100644 --- a/git-refspec/src/parse.rs +++ b/git-refspec/src/parse.rs @@ -26,9 +26,18 @@ pub enum Error { RevSpec(#[from] git_revision::spec::parse::Error), } +/// Define how the parsed refspec should be used. +#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] +pub enum Operation { + /// The `src` side is local and the `dst` side is remote. + Push, + /// The `src` side is remote and the `dst` side is local. + Fetch, +} + pub(crate) mod function { use crate::parse::Error; - use crate::{types::Mode, Operation, RefSpecRef}; + use crate::{parse::Operation, types::Mode, RefSpecRef}; use bstr::{BStr, ByteSlice}; /// Parse `spec` for use in `operation` and return it if it is valid. diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index db159824c3d..c74f4c76253 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -1,5 +1,5 @@ -use crate::types::Push; -use crate::{types::Mode, Fetch, Instruction, Operation, RefSpec, RefSpecRef}; +use crate::instruction::{Fetch, Push}; +use crate::{parse::Operation, types::Mode, Instruction, RefSpec, RefSpecRef}; /// Conversion. Use the [RefSpecRef][RefSpec::to_ref()] type for more usage options. impl RefSpec { @@ -14,9 +14,13 @@ impl RefSpec { } } -impl Into for RefSpecRef<'_> { - fn into(self) -> RefSpec { - self.to_owned() +mod impls { + use crate::{RefSpec, RefSpecRef}; + + impl Into for RefSpecRef<'_> { + fn into(self) -> RefSpec { + self.to_owned() + } } } diff --git a/git-refspec/src/types.rs b/git-refspec/src/types.rs index 115bf5108b4..f7c453a0a71 100644 --- a/git-refspec/src/types.rs +++ b/git-refspec/src/types.rs @@ -1,4 +1,4 @@ -use bstr::BStr; +use crate::instruction; /// The way to interpret a refspec. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] @@ -11,82 +11,11 @@ pub(crate) enum Mode { Negative, } -/// What operation to perform with the refspec. -#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] -pub enum Operation { - /// The `src` side is local and the `dst` side is remote. - Push, - /// The `src` side is remote and the `dst` side is local. - Fetch, -} - -/// Tells what to do and is derived from a [`RefSpec`][RefSpecRef]. +/// Tells what to do and is derived from a [`RefSpec`][crate::RefSpecRef]. #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] pub enum Instruction<'a> { /// An instruction for pushing. - Push(Push<'a>), + Push(instruction::Push<'a>), /// An instruction for fetching. - Fetch(Fetch<'a>), -} - -/// Note that all sources can either be a ref-name, partial or full, or a rev-spec, unless specified otherwise, on the local side. -/// Destinations can only be a partial or full ref names on the remote side. -#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] -pub enum Push<'a> { - /// Push all local branches to the matching destination on the remote, which has to exist to be updated. - AllMatchingBranches { - /// If true, allow non-fast-forward updates of the matched destination branch. - allow_non_fast_forward: bool, - }, - /// Delete the destination ref or glob pattern, with only a single `*` allowed. - Delete { - /// The reference or pattern to delete on the remote. - ref_or_pattern: &'a BStr, - }, - /// Push a single ref or refspec to a known destination ref. - Matching { - /// The source ref or refspec to push. If pattern, it contains a single `*`. - src: &'a BStr, - /// The ref to update with the object from `src`. If `src` is a pattern, this is a pattern too. - dst: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, -} - -/// Note that any source can either be a ref name (full or partial) or a fully spelled out hex-sha for an object, on the remote side. -/// -/// Destinations can only be a partial or full ref-names on the local side. -#[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)] -pub enum Fetch<'a> { - /// Fetch a ref or refs and write the result into the `FETCH_HEAD` without updating local branches. - Only { - /// The ref name to fetch on the remote side, without updating the local side. This will write the result into `FETCH_HEAD`. - src: &'a BStr, - }, - /// Exclude a single ref. - Exclude { - /// A single partial or full ref name to exclude on the remote, or a pattern with a single `*`. It cannot be a spelled out object hash. - src: &'a BStr, - }, - /// Fetch from `src` and update the corresponding destination branches in `dst` accordingly. - AndUpdate { - /// The ref name to fetch on the remote side, or a pattern with a single `*` to match against. - src: &'a BStr, - /// The local destination to update with what was fetched, or a pattern whose single `*` will be replaced with the matching portion - /// of the `*` from `src`. - dst: &'a BStr, - /// If true, allow non-fast-forward updates of `dest`. - allow_non_fast_forward: bool, - }, -} - -impl Instruction<'_> { - /// Derive the mode of operation from this instruction. - pub fn operation(&self) -> Operation { - match self { - Instruction::Push(_) => Operation::Push, - Instruction::Fetch(_) => Operation::Fetch, - } - } + Fetch(instruction::Fetch<'a>), } diff --git a/git-refspec/tests/parse/fetch.rs b/git-refspec/tests/parse/fetch.rs index f030a747050..b844fc2d403 100644 --- a/git-refspec/tests/parse/fetch.rs +++ b/git-refspec/tests/parse/fetch.rs @@ -1,5 +1,5 @@ use crate::parse::{assert_parse, b, try_parse}; -use git_refspec::{parse::Error, Fetch, Instruction, Operation}; +use git_refspec::{instruction::Fetch, parse::Error, parse::Operation, Instruction}; #[test] fn revspecs_are_disallowed() { diff --git a/git-refspec/tests/parse/invalid.rs b/git-refspec/tests/parse/invalid.rs index 9f8af9aa142..b56aec4df6f 100644 --- a/git-refspec/tests/parse/invalid.rs +++ b/git-refspec/tests/parse/invalid.rs @@ -1,5 +1,5 @@ use crate::parse::try_parse; -use git_refspec::{parse::Error, Operation}; +use git_refspec::{parse::Error, parse::Operation}; #[test] fn empty() { diff --git a/git-refspec/tests/parse/mod.rs b/git-refspec/tests/parse/mod.rs index 230cbde081b..597359f9b9e 100644 --- a/git-refspec/tests/parse/mod.rs +++ b/git-refspec/tests/parse/mod.rs @@ -1,5 +1,5 @@ use bstr::ByteSlice; -use git_refspec::Operation; +use git_refspec::parse::Operation; use git_testtools::scripted_fixture_repo_read_only; use std::panic::catch_unwind; @@ -61,7 +61,7 @@ mod invalid; mod push; mod util { - use git_refspec::{Instruction, Operation, RefSpecRef}; + use git_refspec::{parse::Operation, Instruction, RefSpecRef}; pub fn b(input: &str) -> &bstr::BStr { input.into() diff --git a/git-refspec/tests/parse/push.rs b/git-refspec/tests/parse/push.rs index 8b4e307dd25..93b414223ef 100644 --- a/git-refspec/tests/parse/push.rs +++ b/git-refspec/tests/parse/push.rs @@ -1,5 +1,5 @@ use crate::parse::{assert_parse, b, try_parse}; -use git_refspec::{parse::Error, Instruction, Operation, Push}; +use git_refspec::{instruction::Push, parse::Error, parse::Operation, Instruction}; #[test] fn negative_unsupported() { From 894538be4663a72a68f2308ee84019b80441bd92 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 11:09:37 +0800 Subject: [PATCH 35/36] update crate status (#450) --- README.md | 1 + crate-status.md | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 01eec38a247..c9ce701f6c5 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,7 @@ is usable to some extend. * [git-repository](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-repository) * [git-attributes](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-attributes) * [git-pathspec](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-pathspec) + * [git-refspec](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-refspec) * `gitoxide-core` * **very early** _(possibly without any documentation and many rough edges)_ * [git-index](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-index) diff --git a/crate-status.md b/crate-status.md index 3f412ee8561..15fcc568563 100644 --- a/crate-status.md +++ b/crate-status.md @@ -232,7 +232,11 @@ Check out the [performance discussion][git-traverse-performance] as well. ### git-pathspec * [x] parse -* [ ] check for match +* [ ] matching of paths + +### git-refspec +* [x] parse +* [ ] matching of references and object names ### git-note From 6c963b022e48854001353f8909c3e8314c2e5861 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 7 Aug 2022 11:10:27 +0800 Subject: [PATCH 36/36] thanks clippy --- etc/check-package-size.sh | 2 +- git-refspec/src/spec.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/etc/check-package-size.sh b/etc/check-package-size.sh index 4431725ab0a..aff6a2fd162 100755 --- a/etc/check-package-size.sh +++ b/etc/check-package-size.sh @@ -18,7 +18,7 @@ echo "in root: gitoxide CLI" (enter cargo-smart-release && indent cargo diet -n --package-size-limit 95KB) (enter git-actor && indent cargo diet -n --package-size-limit 5KB) (enter git-pathspec && indent cargo diet -n --package-size-limit 25KB) -(enter git-refspec && indent cargo diet -n --package-size-limit 5KB) +(enter git-refspec && indent cargo diet -n --package-size-limit 15KB) (enter git-path && indent cargo diet -n --package-size-limit 15KB) (enter git-attributes && indent cargo diet -n --package-size-limit 15KB) (enter git-discover && indent cargo diet -n --package-size-limit 20KB) diff --git a/git-refspec/src/spec.rs b/git-refspec/src/spec.rs index c74f4c76253..14fd2a090e2 100644 --- a/git-refspec/src/spec.rs +++ b/git-refspec/src/spec.rs @@ -17,9 +17,9 @@ impl RefSpec { mod impls { use crate::{RefSpec, RefSpecRef}; - impl Into for RefSpecRef<'_> { - fn into(self) -> RefSpec { - self.to_owned() + impl From> for RefSpec { + fn from(v: RefSpecRef<'_>) -> Self { + v.to_owned() } } }