Skip to content

attributes matching#400

Merged
Sebastian Thiel (Byron) merged 14 commits into
mainfrom
worktree-stack
Apr 14, 2023
Merged

attributes matching#400
Sebastian Thiel (Byron) merged 14 commits into
mainfrom
worktree-stack

Conversation

@Byron

@Byron Sebastian Thiel (Byron) commented Apr 30, 2022

Copy link
Copy Markdown
Member

A way to obtain a set of attributes given a path, efficiently.

Each path we handle needs one lookup there during checkout, thus it must be lazy and per thread.

git seems to implement it as stack which is efficient if similar paths are handled together, and then drops information as it is no longer needed. This seems like the way to go as we never have to optimize for random access.

Tasks

  • an Outcome type to handle searches across groups and can deal with macros.
  • implement attribute stack lookups in user-defined order
  • a mechanism to reject macros in non-root gitattribute files (should be persistent, with leniency) - it's always lenient but logs issues.
  • test for and implement differences (compared to excludes) on how paths match there
  • fix gix-ignore test failures due to case-sensitivity
  • assure we can handle macro attributes
  • assure user-defined order is maintained across multiple Search instances
  • a test to see what happens if macros are redefined
  • leniency to only log on invalid pattern (similar to git, log errors)
  • proper gix gitattribute setup (see git implementation)

Next PRs

  • gix-worktree integration (fs::Cache) and support for various sources. Needs to ignore macros in dynamically loaded attr files.
  • gix repo attribute query/verify with support for overrides
  • A test tool to collect gitattribute and maybe gitignore information from real-world git repos along with the replies by git check-attr to have baselines against which to test our implementation.

Differences to .gitignore

  • negative patterns are forbidden
  • patterns that match a directory do not recursively match paths
    inside that directory (so using the trailing-slash path/ syntax is
    pointless in an attributes file; use path/** instead)
  • built-in macro attributes binary which is -text -diff -merge.
  • auto-attributes: text/binary (doesn't seem to be a thing - diff might do its own binary check before diffing blobs if the attribute isn't set)
  • macro-resolution: what about cycles?

Really just an excuse to start a new PR for additional attribute work
without investing much time.
… path generation.

This will make system wide configuration available on windows at least when using the migw builds.
…t copy parsed input.

This kind of processing was only done for ignore patterns, which
are quite different from how things are done in attribute files.
@Byron
Sebastian Thiel (Byron) force-pushed the worktree-stack branch 2 times, most recently from 6b66e1b to 3541e20 Compare April 12, 2023 17:22
@Byron Sebastian Thiel (Byron) changed the title worktree cache with attributes support attributes matching Apr 13, 2023
An implementation for `Search<Attributes>` along with various
breaking modifications of the original module and type layout.
This is done to fully understand how attribute initialization works
with the current architecture, while taking care of its particular
difference compared to the handling of exclude patterns.
… are loaded from.

That way it's possible to, for example, isolate all operations that rely on the `gitattribute`
system, like checkouts or additions to the index.
This is an on-demand operation anyway, but now we turn on the loading of
git binary-specific gitattributes which should help to emulate gits
behaviour perfectly.
@Byron
Sebastian Thiel (Byron) merged commit 3d47919 into main Apr 14, 2023
@Byron
Sebastian Thiel (Byron) deleted the worktree-stack branch April 15, 2023 07:13
Eliah Kagan (EliahKagan) added a commit to EliahKagan/advisory-db that referenced this pull request Jul 24, 2024
gix-attributes was found by @ssbr to be unsound, as reported in
GitoxideLabs/gitoxide#1460. This adds an
informational notice for that, as discussed in comments there.

It looks like the affected code, having been introduced in
GitoxideLabs/gitoxide#400, was present in all
versions of the crate prior to the fix in 0.22.3 (which was one of
the bugs fixed in GitoxideLabs/gitoxide#1462).

Co-authored-by: Devin Jeanpierre <[email protected]>
Sergey "Shnatsel" Davidoff (Shnatsel) pushed a commit to rustsec/advisory-db that referenced this pull request Jul 24, 2024
* Unsoundness notice for gix-attributes (kstring integration)

gix-attributes was found by @ssbr to be unsound, as reported in
GitoxideLabs/gitoxide#1460. This adds an
informational notice for that, as discussed in comments there.

It looks like the affected code, having been introduced in
GitoxideLabs/gitoxide#400, was present in all
versions of the crate prior to the fix in 0.22.3 (which was one of
the bugs fixed in GitoxideLabs/gitoxide#1462).

Co-authored-by: Devin Jeanpierre <[email protected]>

* Small adjustments for advisory

This makes some minor changes to the advisory description to adapt
the text from GitoxideLabs/gitoxide#1460 to be
an advisory. For the most part it has remained the same. Changes:

* Express the claim of unsoundness with more confidence, since it
  has been reviewed by the maintainer.

* Modify the link to the affected code to point to the latest tag
  for gix-attributes that has that code. The original link was to
  a branch, so it was broken when the fix was applied.

* Apply inline code formatting in a few more places, where doing
  so improves stylistic consistency.

---------

Co-authored-by: Devin Jeanpierre <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant