From: Stefan Sperling <stsp@stsp.name>
Subject: Re: replace got_repo_get_gitconfig_extensions with has_extension
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Sun, 5 Feb 2023 16:52:02 +0100

On Sun, Feb 05, 2023 at 04:45:24PM +0100, Omar Polo wrote:
> I'd like to change how extensions are stored in the struct repository
> and got_repo_get_gitconfig_extensions gets in the way.  It's only used
> in gotadmin to check if the "preciousObjects" extension is active, so
> I'm replacing it with got_repo_has_extension, which makes its use even
> nicer IMHO.
> 
> The change to how extensions are stored is because we currently assume
> that extensions are either enabled or disabled (on/off), while they
> could also be strings (objectformat=sha256 for example ;-)  That's a
> follow-up diff tho.
> 
> ok?

Yes, ok. Makes sense to me.

> diff 30bd0f8ea103b4839e685df105cbdf5a033b3ebd 7c3a2af1f17dda354dd8ae31d233458143646797
> commit - 30bd0f8ea103b4839e685df105cbdf5a033b3ebd
> commit + 7c3a2af1f17dda354dd8ae31d233458143646797
> blob - b36b50b4e34e52be3dca38a6cc051e5fab489e0c
> blob + e67fbcf1e5c4b6bc2399bde36796bf77749c9bed
> --- gotadmin/gotadmin.c
> +++ gotadmin/gotadmin.c
> @@ -1218,8 +1218,6 @@ cmd_cleanup(int argc, char *argv[])
>  	char scaled_before[FMT_SCALED_STRSIZE];
>  	char scaled_after[FMT_SCALED_STRSIZE];
>  	char scaled_diff[FMT_SCALED_STRSIZE];
> -	char **extensions;
> -	int nextensions, i;
>  	int *pack_fds = NULL;
>  
>  	while ((ch = getopt(argc, argv, "anpqr:")) != -1) {
> @@ -1273,15 +1271,11 @@ cmd_cleanup(int argc, char *argv[])
>  	if (error)
>  		goto done;
>  
> -	got_repo_get_gitconfig_extensions(&extensions, &nextensions,
> -	    repo);
> -	for (i = 0; i < nextensions; i++) {
> -		if (strcasecmp(extensions[i], "preciousObjects") == 0) {
> -			error = got_error_msg(GOT_ERR_GIT_REPO_EXT,
> -			    "the preciousObjects Git extension is enabled; "
> -			    "this implies that objects must not be deleted");
> -			goto done;
> -		}
> +	if (got_repo_has_extension(repo, "preciousObjects")) {
> +		error = got_error_msg(GOT_ERR_GIT_REPO_EXT,
> +		    "the preciousObjects Git extension is enabled; "
> +		    "this implies that objects must not be deleted");
> +		goto done;
>  	}
>  
>  	if (remove_lonely_packidx) {
> blob - b507b207a3c163a0492722f3e42ea689dacbf7e6
> blob + 53e99801ab01ad8804d048c5f59a94930e5ef10a
> --- include/got_repository.h
> +++ include/got_repository.h
> @@ -50,9 +50,8 @@ const char *got_repo_get_gitconfig_owner(struct got_re
>  /* Obtain repository owner name if parsed from gitconfig, else NULL. */
>  const char *got_repo_get_gitconfig_owner(struct got_repository *);
>  
> -/* Obtain the list of enabled Git extensions parsed from gitconfig. */
> -void got_repo_get_gitconfig_extensions(char ***, int *,
> -    struct got_repository *);
> +/* Query if a given Git extension is enabled in gitconfig. */
> +int got_repo_has_extension(struct got_repository *, const char *);
>  
>  /* Information about one remote repository. */
>  struct got_remote_repo {
> blob - e754fef75fcc2d66013a8f85b64547acc0aea1c4
> blob + 724a22a36f25053e940871eed29828527c813f87
> --- lib/repository.c
> +++ lib/repository.c
> @@ -122,12 +122,17 @@ void
>  	return repo->gitconfig_owner;
>  }
>  
> -void
> -got_repo_get_gitconfig_extensions(char ***extensions, int *nextensions,
> -    struct got_repository *repo)
> +int
> +got_repo_has_extension(struct got_repository *repo, const char *ext)
>  {
> -	*extensions = repo->extensions;
> -	*nextensions = repo->nextensions;
> +	int i;
> +
> +	for (i = 0; i < repo->nextensions; ++i) {
> +		if (!strcasecmp(ext, repo->extensions[i]))
> +			return 1;
> +	}
> +
> +	return 0;
>  }
>  
>  int
> 
> 
>