Subject: Re: [PATCH v3 3/6] libgit-sys: add repo initialization and config access

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 10 Sep 2024 08:42:02 +0200

On Fri, Sep 06, 2024 at 10:21:13PM +0000, Calvin Wan wrote:
> diff --git a/contrib/libgit-rs/libgit-sys/public_symbol_export.c b/contrib/libgit-rs/libgit-sys/public_symbol_export.c
> index 39c27d9c1a..65d1620d28 100644
> --- a/contrib/libgit-rs/libgit-sys/public_symbol_export.c
> +++ b/contrib/libgit-rs/libgit-sys/public_symbol_export.c
> @@ -2,11 +2,37 @@
>  // original symbols can be hidden. Renaming these with a "libgit_" prefix also
>  // avoid conflicts with other libraries such as libgit2.
>  
> +#include "git-compat-util.h"
>  #include "contrib/libgit-rs/libgit-sys/public_symbol_export.h"
> +#include "common-init.h"
> +#include "config.h"
> +#include "setup.h"
>  #include "version.h"
>  
> +extern struct repository *the_repository;
> +
>  #pragma GCC visibility push(default)
>  
> +const char *libgit_setup_git_directory(void)
> +{
> +	return setup_git_directory();
> +}
> +
> +int libgit_config_get_int(const char *key, int *dest)
> +{
> +	return repo_config_get_int(the_repository, key, dest);
> +}
> +
> +void libgit_init_git(const char **argv)
> +{
> +	init_git(argv);
> +}
> +
> +int libgit_parse_maybe_bool(const char *val)
> +{
> +	return git_parse_maybe_bool(val);
> +}
> +

I don't quite get why we expose functionality that is inherently not
libified. Exposing the current state to library users certainly does not
feel right to me, doubly so because `the_repository` is deprecated and
will eventually go away. So we already know that we'll have to break the
API here once that has happened. I'd rather want to see that introducing
the library makes us double down on providing properly encapsulated
interfaces from hereon.

Also, we already have ways to initialize a repository and read their
config without relying on `the_repository`. So shouldn't we expose that
instead?

Patrick