Skip to content

Commit

Permalink
disallow excludes in push mode (#450)
Browse files Browse the repository at this point in the history
It's not documented in `git-push`, even though git parses it fine for
some reason.
  • Loading branch information
Byron committed Aug 7, 2022
1 parent b889953 commit 9c280b2
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 32 deletions.
5 changes: 5 additions & 0 deletions git-refspec/src/parse.rs
Expand Up @@ -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")]
Expand Down Expand Up @@ -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'+') => {
Expand Down
1 change: 0 additions & 1 deletion git-refspec/src/spec.rs
Expand Up @@ -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",
Expand Down
7 changes: 1 addition & 6 deletions git-refspec/src/types.rs
Expand Up @@ -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 `*`.
Expand All @@ -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 {
Expand Down
Git LFS file not shown
7 changes: 6 additions & 1 deletion git-refspec/tests/fixtures/make_baseline.sh
Expand Up @@ -6,6 +6,7 @@ git init;
function baseline() {
local kind=$1
local refspec=$2
local force_fail=${3:-}

cat <<EOF >.git/config
[remote "test"]
Expand All @@ -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"
Expand Down Expand Up @@ -82,7 +87,7 @@ baseline fetch 'HEAD'
baseline push '@'
baseline fetch '@'

baseline push '^@'
baseline push '^@' fail
baseline fetch '^@'

baseline push '+@'
Expand Down
41 changes: 26 additions & 15 deletions git-refspec/tests/parse/invalid.rs
Expand Up @@ -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]
Expand Down
7 changes: 0 additions & 7 deletions 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(
Expand All @@ -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]
Expand Down

0 comments on commit 9c280b2

Please sign in to comment.