Re: [PATCH v3 4/7] submodule--helper: add a new 'config' subcommand

From: Brandon Williams <bmwill@google.com>
Date: Aug 14, 2018, 17:10 UTC
Message-ID: <20180814171058.GF240194@google.com>
In-Reply-To: <20180814110525.17801-5-ao2@ao2.it>

On 08/14, Antonio Ospite wrote:

Show 7 quoted lines

> Add a new 'config' subcommand to 'submodule--helper', this extra level
> of indirection makes it possible to add some flexibility to how the
> submodules configuration is handled.
> 
> Signed-off-by: Antonio Ospite <ao2@ao2.it>
> ---
>  builtin/submodule--helper.c | 14 ++++++++++++++

>  new                         |  0

Looks like you may have accidentally left in an empty file "new" it should probably be removed from this commit before it gets merged.

Aside from that this patch looks good. I've recently run into issues where we don't have a good enough abstraction layer around how we interact with submodules so I'm glad we're moving towards better abstractions :)

Show 77 quoted lines

>  t/t7411-submodule-config.sh | 26 ++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+)
>  create mode 100644 new
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index a3c4564c6c..7481d03b63 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2029,6 +2029,19 @@ static int connect_gitdir_workingtree(int argc, const char **argv, const char *p
>  	return 0;
>  }
>  
> +static int module_config(int argc, const char **argv, const char *prefix)
> +{
> +	/* Equivalent to ACTION_GET in builtin/config.c */
> +	if (argc == 2)
> +		return print_config_from_gitmodules(argv[1]);
> +
> +	/* Equivalent to ACTION_SET in builtin/config.c */
> +	if (argc == 3)
> +		return config_set_in_gitmodules_file_gently(argv[1], argv[2]);
> +
> +	die("submodule--helper config takes 1 or 2 arguments: name [value]");
> +}
> +
>  #define SUPPORT_SUPER_PREFIX (1<<0)
>  
>  struct cmd_struct {
> @@ -2057,6 +2070,7 @@ static struct cmd_struct commands[] = {
>  	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
>  	{"is-active", is_active, 0},
>  	{"check-name", check_name, 0},
> +	{"config", module_config, 0},
>  };
>  
>  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> diff --git a/new b/new
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
> index c6b6cf6fae..4afb6f152e 100755
> --- a/t/t7411-submodule-config.sh
> +++ b/t/t7411-submodule-config.sh
> @@ -142,4 +142,30 @@ test_expect_success 'error in history in fetchrecursesubmodule lets continue' '
>  	)
>  '
>  
> +test_expect_success 'reading submodules config with "submodule--helper config"' '
> +	(cd super &&
> +		echo "../submodule" >expected &&
> +		git submodule--helper config submodule.submodule.url >actual &&
> +		test_cmp expected actual
> +	)
> +'
> +
> +test_expect_success 'writing submodules config with "submodule--helper config"' '
> +	(cd super &&
> +		echo "new_url" >expected &&
> +		git submodule--helper config submodule.submodule.url "new_url" &&
> +		git submodule--helper config submodule.submodule.url >actual &&
> +		test_cmp expected actual
> +	)
> +'
> +
> +test_expect_success 'overwriting unstaged submodules config with "submodule--helper config"' '
> +	(cd super &&
> +		echo "newer_url" >expected &&
> +		git submodule--helper config submodule.submodule.url "newer_url" &&
> +		git submodule--helper config submodule.submodule.url >actual &&
> +		test_cmp expected actual
> +	)
> +'
> +
>  test_done
> -- 
> 2.18.0
>

-- 
Brandon Williams