Skip to content
New issue

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

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

Already on GitHub? # to your account

Pass byte slice as root to tree walk callback #1034

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bsidhom
Copy link

@bsidhom bsidhom commented Mar 3, 2024

Fixes #1033.

Here's an example run of the example from that issue, but with the fix:

> cargo run --release
Creating repo at "/var/folders/0h/1lcyrkv11ynfjq5w7bt0sl_00000gn/T/git-repo-_YIz7w.git"
Populating repo
Repo populated

Walking via git2:
9376e30e1b6e8f7aef8aa51d19dfc0726f1835a0: Commit to hold reference
  a.txt:
  > Toplevel file. This is walkable because it's traversed _before_ the broken subtree.
  b�.txt:
  > Toplevel file. This is walkable as above, but it also has a filename which is not valid utf-8.
  c�: <tree>
  c�/nested.txt:
  > Not walkable due to the parent directory name being non-utf-8 bytes. This will abort the walk.
  d.txt:
  > Not walkable due to the broken tree above ending the walk.

Listing committed files using git command:
100644 blob 596c677536cbdd42ed208abb4135afcb95bd967c    a.txt
100644 blob 330e18bbaa623998f17ce703cdcf7158915c6bc5    "b\300.txt"
100644 blob 5fd5bb9cddb4076e8786e90670ba46e621174482    "c\300/nested.txt"
100644 blob 07bd6651bc96c9554dab80f1f15b867d4091116a    d.txt

As noted in that issue, I'm not sure if a breaking change is acceptable in this case (version is below 1 and it seems to be trying to match libgit2 as closely as possible, so I assume this is fine).

@bsidhom
Copy link
Author

bsidhom commented Mar 3, 2024

Since this was a breaking change, I had to add let filename = String::from_utf8_lossy(entry.name_bytes()); to the callback to get the example working.

@ehuss
Copy link
Contributor

ehuss commented May 25, 2024

Thanks! Can the callback take a CStr instead? AFAIK, the value isn't an arbitrary slice of bytes since it cannot contain a null.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tree.walk() broken for subtrees with non-utf-8 names
2 participants