Skip to content

various fixes #1462

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion gix-attributes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
doc = ::document_features::document_features!()
)]
#![cfg_attr(all(doc, feature = "document-features"), feature(doc_cfg, doc_auto_cfg))]
#![deny(missing_docs, rust_2018_idioms, unsafe_code)]
#![deny(missing_docs, rust_2018_idioms)]
#![forbid(unsafe_code)]

pub use gix_glob as glob;
use kstring::{KString, KStringRef};
Expand Down
3 changes: 1 addition & 2 deletions gix-attributes/src/name.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::{Name, NameRef};
use bstr::{BStr, BString, ByteSlice};
use kstring::KStringRef;

use crate::{Name, NameRef};

impl<'a> NameRef<'a> {
/// Turn this ref into its owned counterpart.
pub fn to_owned(self) -> Name {
Expand Down
3 changes: 1 addition & 2 deletions gix-attributes/src/parse.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use std::borrow::Cow;

use crate::{name, AssignmentRef, Name, NameRef, StateRef};
use bstr::{BStr, ByteSlice};
use kstring::KStringRef;

use crate::{name, AssignmentRef, Name, NameRef, StateRef};

/// The kind of attribute that was parsed.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand Down
3 changes: 1 addition & 2 deletions gix-attributes/src/search/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::collections::HashMap;

use kstring::KString;
use smallvec::SmallVec;
use std::collections::HashMap;

use crate::{Assignment, AssignmentRef};

Expand Down
23 changes: 9 additions & 14 deletions gix-attributes/src/state.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,24 @@
use bstr::{BStr, ByteSlice};
use kstring::{KString, KStringRef};

use crate::{State, StateRef};
use bstr::{BStr, BString, ByteSlice};

/// A container to encapsulate a tightly packed and typically unallocated byte value that isn't necessarily UTF8 encoded.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct Value(KString);
// TODO: This should be some sort of 'smallbstring' - but can't use `kstring` here due to UTF8 requirement. 5% performance boost possible.
// What's really needed here is a representation that displays as string when serialized which helps with JSON.
// Maybe `smallvec` with display and serialization wrapper would do the trick?
pub struct Value(BString);

/// A reference container to encapsulate a tightly packed and typically unallocated byte value that isn't necessarily UTF8 encoded.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct ValueRef<'a>(#[cfg_attr(feature = "serde", serde(borrow))] KStringRef<'a>);
pub struct ValueRef<'a>(#[cfg_attr(feature = "serde", serde(borrow))] &'a [u8]);

/// Lifecycle
impl<'a> ValueRef<'a> {
/// Keep `input` as our value.
pub fn from_bytes(input: &'a [u8]) -> Self {
Self(KStringRef::from_ref(
// SAFETY: our API makes accessing that value as `str` impossible, so illformed UTF8 is never exposed as such.
#[allow(unsafe_code)]
unsafe {
std::str::from_utf8_unchecked(input)
},
))
Self(input)
}
}

Expand All @@ -42,7 +37,7 @@ impl ValueRef<'_> {

impl<'a> From<&'a str> for ValueRef<'a> {
fn from(v: &'a str) -> Self {
ValueRef(v.into())
ValueRef(v.as_bytes())
}
}

Expand All @@ -54,7 +49,7 @@ impl<'a> From<ValueRef<'a>> for Value {

impl From<&str> for Value {
fn from(v: &str) -> Self {
Value(KString::from_ref(v))
Value(v.as_bytes().into())
}
}

Expand Down
29 changes: 29 additions & 0 deletions gix-attributes/tests/parse/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use bstr::BString;
use gix_attributes::state::ValueRef;
use gix_attributes::{parse, StateRef};
use gix_glob::pattern::Mode;
use gix_testtools::fixture_bytes;
Expand Down Expand Up @@ -275,6 +276,19 @@ fn attributes_can_have_values() {
);
}

#[test]
fn attributes_can_have_illformed_utf8() {
assert_eq!(
byte_line(b"p a=one b=\xC3\x28\x41 c=d "),
(
pattern("p", Mode::NO_SUB_DIR, None),
vec![value("a", "one"), byte_value("b", b"\xC3\x28\x41"), value("c", "d")],
1
),
"illformed UTF8 is fully supported"
);
}

#[test]
fn attributes_see_state_adjustments_over_value_assignments() {
assert_eq!(
Expand Down Expand Up @@ -325,6 +339,10 @@ fn value<'b>(attr: &str, value: &'b str) -> (BString, StateRef<'b>) {
(attr.into(), StateRef::Value(value.into()))
}

fn byte_value<'b>(attr: &str, value: &'b [u8]) -> (BString, StateRef<'b>) {
(attr.into(), StateRef::Value(ValueRef::from_bytes(value)))
}

fn pattern(name: &str, flags: gix_glob::pattern::Mode, first_wildcard_pos: Option<usize>) -> parse::Kind {
parse::Kind::Pattern(gix_glob::Pattern {
text: name.into(),
Expand All @@ -344,6 +362,17 @@ fn line(input: &str) -> ExpandedAttribute {
try_line(input).unwrap()
}

fn byte_line(input: &[u8]) -> ExpandedAttribute {
try_byte_line(input).unwrap()
}

fn try_byte_line(input: &[u8]) -> Result<ExpandedAttribute, parse::Error> {
let mut lines = gix_attributes::parse(input);
let res = expand(lines.next().unwrap())?;
assert!(lines.next().is_none(), "expected only one line");
Ok(res)
}

fn lenient_lines(input: &str) -> Vec<ExpandedAttribute> {
gix_attributes::parse(input.as_bytes())
.map(expand)
Expand Down
2 changes: 1 addition & 1 deletion gix-attributes/tests/search/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ fn given_attributes_are_made_available_in_given_order() -> crate::Result {
fn size_of_outcome() {
assert_eq!(
std::mem::size_of::<Outcome>(),
904,
840,
"it's quite big, shouldn't change without us noticing"
)
}
Expand Down
2 changes: 2 additions & 0 deletions gix-dir/src/walk/classify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ pub fn path(
.map(|platform| platform.excluded_kind())
})
.map_err(Error::ExcludesAccess)?
.filter(|_| filename_start_idx > 0)
Copy link
Member Author

Choose a reason for hiding this comment

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

This line and the line below constitute the fix, it's quite neat.

The idea is that previously, the classifier didn't distinguish between the worktree root (and the contained repository) and sub-repositories. For the latter, the current classification is very much intentional, but the root should never ever be counted as ignored, and thus removable.

The commit with the fix also shows how the classification changes due to the fix, which is all it takes to prevent gix clean to go off the rail.

When looking for weaknesses, I also recommend using the pathspec, as it is quite integrated to the algorithm to have some (possibly unexpected) influence.

{
out.status = entry::Status::Ignored(excluded);
}
Expand Down Expand Up @@ -256,6 +257,7 @@ pub fn path(
if let Some(excluded) = ctx
.excludes
.as_mut()
.filter(|_| !rela_path.is_empty())
.map_or(Ok(None), |stack| {
stack
.at_entry(rela_path.as_bstr(), is_dir, ctx.objects)
Expand Down
80 changes: 80 additions & 0 deletions gix-dir/tests/fixtures/many.sh
Original file line number Diff line number Diff line change
Expand Up @@ -351,3 +351,83 @@ git clone submodule multiple-submodules
git submodule add ../submodule a/b
git commit -m "add modules"
)

git clone submodule one-ignored-submodule
(cd one-ignored-submodule
git submodule add ../submodule submodule
echo '/submodule/' > .gitignore
echo '*' > submodule/.gitignore
git commit -m "add seemingly ignored submodule"
)

git init slash-in-root-and-negated
(cd slash-in-root-and-negated
cat <<'EOF' >.gitignore
/
!file
!*.md
!.github
!.github/**
EOF
touch file readme.md
mkdir .github
touch .github/workflow.yml
git add .github readme.md .gitignore
git commit -m "init"
)

git init star-in-root-and-negated
(cd star-in-root-and-negated
cat <<'EOF' >.gitignore
*
!file
!.gitignore
!*.md
!.github
!.github/**
EOF
touch file readme.md
mkdir .github
touch .github/workflow.yml
git add .github readme.md .gitignore
git commit -m "init"
)

git init slash-in-subdir-and-negated
(cd slash-in-subdir-and-negated
mkdir sub
(cd sub
cat <<'EOF' >.gitignore
/
!file
!*.md
!.github
!.github/**
EOF
touch file readme.md
mkdir .github
touch .github/workflow.yml
git add .github readme.md .gitignore
git commit -m "init"
)
)

git init star-in-subdir-and-negated
(cd star-in-subdir-and-negated
mkdir sub
(cd sub
cat <<'EOF' >.gitignore
*
!file
!.gitignore
!*.md
!.github
!.github/**
EOF
touch file readme.md
mkdir .github
touch .github/workflow.yml
git add .github readme.md .gitignore
git commit -m "init"
)
)
Loading
Loading