NULL (#f) not allowed for `git_diff_options`

[kengruven]

The libgit2 docs for git_diff_options say "passing NULL for the options structure will give the defaults", and the libgit2 sample code shows this in practice.

With these Racket bindings, though, several functions (like git_diff_tree_to_tree) use _git_diff_opts-pointer rather than _git_diff_opts-pointer/null, so passing #f here results in an error:

git_diff_opts->C: argument is not non-null `git_diff_opts' pointer

The scrbl docs even say that this parameter has type [opts (or/c git_diff_opts? #f)].

[kengruven]

I tried working around this from my program, and got stuck at every attempt:

• There's a C macro GIT_DIFF_OPTIONS_INIT which has a value for git_diff_options (but not the defaults, it seems?), but this constant isn't wrapped by libgit2-racket so you can't use that
• There's a C function git_diff_options_init which creates a default git_diff_options for you, but it requires a starting C struct that you wish to initialize
• The libgit2-racket bindings has a define-cstruct _git_diff_opts but it misses the oid_type field from the middle of the struct, so you can't create your own from scratch, either

Based on the upstream docs, I think the most low-level way to create a non-null "default" value would be something like:

(make-git_diff_opts GIT_DIFF_OPTS_VERSION 'GIT_DIFF_NORMAL 'GIT_SUBMODULE_IGNORE_UNSPECIFIED
                    (make-git_strarray '()) #f #f #f 3 0 0 7 (* 512 1024 1024) "a" "b")

but this fails with "the expected number of arguments does not match the given number" because oid_type is missing from the mapping.

[LiberalArtist]

Thanks for the report!

With these Racket bindings, though, several functions (like git_diff_tree_to_tree) use _git_diff_opts-pointer rather than _git_diff_opts-pointer/null, so passing #f here results in an error:

There's a C function git_diff_options_init which creates a default git_diff_options for you, but it requires a starting C struct that you wish to initialize

These are definitely our bug and should be fixed.

• There's a C macro GIT_DIFF_OPTIONS_INIT which has a value for git_diff_options (but not the defaults, it seems?), but this constant isn't wrapped by libgit2-racket so you can't use that

• There's a C function git_diff_options_init which creates a default git_diff_options for you, but it requires a starting C struct that you wish to initialize

We can't really use C macros like GIT_DIFF_OPTIONS_INIT because they don't exist at runtime for dlsym: IIRC, that's why upstream added functions like git_diff_options_init. But they also make these very friendly guarantees about NULL being the default, so we don't really need all of those functions in Racket. In other words, something very much like your:

(make-git_diff_opts GIT_DIFF_OPTS_VERSION 'GIT_DIFF_NORMAL 'GIT_SUBMODULE_IGNORE_UNSPECIFIED
                    (make-git_strarray '()) #f #f #f 3 0 0 7 (* 512 1024 1024) "a" "b")

ought to work, but for bugs (though maybe we shouldn't expose GIT_DIFF_OPTS_VERSION to clients, since the only thing they could do with it is cause errors).

This is also crying out for a convenience function with optional keyword arguments.

[kengruven]

We can't really use C macros like GIT_DIFF_OPTIONS_INIT because they don't exist at runtime

That's true, but the Racket bindings already say things like (define GIT_DIFF_HUNK_HEADER_SIZE 128) to mimic other C macro constants. That big ( make-git_diff_opts ...) expression took me 15 minutes to figure out, so (at least in the case where one needed a custom git_diff_opts value) I think it could be useful to have available in Racket.

That said, I don't think I need any non-default diff options for my program. Allowing null (and letting libgit2 use its default value) would work just fine for me.

[kengruven]

I've never used Racket FFI before, so I have no idea what I'm doing. Does this change make sense?

Still left to do, to completely support git_diff_options:

• add oid_type field to define-cstruct _git_diff_opts
• add git_oid_t enum (how does #ifdef work here?)
• maybe define an GIT_DIFF_OPTIONS_INIT with default values

[kengruven]

• The libgit2-racket bindings has a define-cstruct _git_diff_opts but it misses the oid_type field from the middle of the struct

On further inspection, this enum was added only about 2 years ago, so as long as libgit2-racket sticks with the old 1.4.3 version, this isn't actually a problem.