Skip to content

Change IndexMatchedPath callback to take an Option #1268

Draft
ehuss wants to merge 1 commit into
rust-lang:mainfrom
ehuss:empty-pathspec-add-all-api-change
Draft

Change IndexMatchedPath callback to take an Option #1268
ehuss wants to merge 1 commit into
rust-lang:mainfrom
ehuss:empty-pathspec-add-all-api-change

Conversation

@ehuss
Copy link
Copy Markdown
Contributor

@ehuss ehuss commented May 29, 2026

The matched pathspec can sometimes be None (if there was no match). This was crashing because the code was panicking in the C callback without being within the panic wrapper. The fix here is to properly handle the case where the matched_pathspec is NULL.

Fixes #585

Opening as a draft since this is a semver-breaking change. We'll need to decide when to slot this in.

@ehuss ehuss added the semver-breaking-change A SemVer breaking change that will require a new SemVer-incompatible version number bump. label May 29, 2026
@rustbot

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-author Status: Waiting on PR author label May 29, 2026
The matched pathspec can sometimes be None (if there was no match). This
was crashing because the code was panicking in the C callback without
being within the panic wrapper. The fix here is to properly handle the
case where the matched_pathspec is NULL.

Fixes rust-lang#585
@ehuss ehuss force-pushed the empty-pathspec-add-all-api-change branch from 1a7e6fb to 44eea16 Compare May 29, 2026 16:44
@DanielEScherzer
Copy link
Copy Markdown
Contributor

The matched pathspec can sometimes be None (if there was no match).

Is there a reason to not just use an empty slice for the missing pathspec, instead of converting to an option?

@ehuss
Copy link
Copy Markdown
Contributor Author

ehuss commented May 29, 2026

Hm, that's an interesting question. libgit2 treats an empty set of pathspecs ([]) the same as an empty pathspec ([""] or any element that is an empty string) (which means to match everything).

git has forbidden empty pathspecs since 2.16 (from 2018), but it looks like libgit2 hasn't followed that change.

One could argue that getting an empty pathspec isn't quite the same as an empty set of pathspecs, but since they are treated the same (and you can't distinguish between them), it seems reasonable to me.

The main risk is if libgit2 ever decides that the empty string is no longer allowed, but an empty set is. I'm not sure if that would ever happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: Waiting on PR author semver-breaking-change A SemVer breaking change that will require a new SemVer-incompatible version number bump.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segfault in Index::add_all() with empty pathspecs slice and Some(cb)

3 participants