Could git_repository_hashfile be changed to use GIT_FILTER_OPT_ALLOW_UNSAFE ?

[muhqu]

I like to use git_repository_hashfile has a substitute to invoking git hash-object command.

But there is a difference in behaviour: When the repository has configured core.safecrlf: true and a file has invalid line endings, git_repository_hashfile returns an error instead of a hash. git hash-object still returns a hash in this case.

In 5269008 the following note was added to git_repository_hashfile API docs:

Note: if the repository has core.safecrlf set to fail and the filtering triggers that failure, then this function will return an error and not calculate the hash of the file.

However, the same commit also added GIT_FILTER_ALLOW_UNSAFE which is documented as…

Don't error for safecrlf violations, allow them to continue.

Would it be possible to change git_repository_hashfile to pass GIT_FILTER_ALLOW_UNSAFE instead of GIT_FILTER_DEFAULT to the call to git_filter_list_load?

libgit2/src/libgit2/repository.c

Lines 3125 to 3127 in bf2f44d

	error = git_filter_list_load(
	&fl, repo, NULL, as_path,
	GIT_FILTER_TO_ODB, GIT_FILTER_DEFAULT);

I think it would be great to match the behaviour of git hash-object command. Opinions?

^{…tagging some people here which were involved with that part of the code… /cc @arrbee @ethomson @csware}

[ethomson]

Hmm. I wonder if there's a middle ground here where we return a meaningful error and compute the oid. This would be the least-breaking way to change this function.

There's an opportunity here to do a bunch of unnecessary work hashing files where we should have exited early. You would expect safecrlf to affect binary files in particular, which may be large. (And they exit quickly in the current implementation.)

But ... that feels like an edge case. Is this a reasonable compromise?

[muhqu]

@ethomson I just created #6483 as an example. There seems to be no test coverage for the current behaviour.

It would be great to find a way to return the error along with the computed hash, if that is possible! …not sure how would look like?

What do you mean with "they exit quickly in the current implementation" ?