Subject: Re: [PATCH 05/10] submodule--helper: add is_active command

From: Brandon Williams <[email-masked]>
Date: Mon, 27 Feb 2017 10:35:36 -0800

On 02/23, Stefan Beller wrote:
> On Thu, Feb 23, 2017 at 3:47 PM, Brandon Williams <[email-masked]> wrote:
> > There are a lot of places where an explicit check for
> > submodule."<name>".url is done to see if a submodule exists.  In order
> > to more easily facilitate the use of the submodule.active config option
> > to indicate active submodules, add a helper which can be used to query
> > if a submodule is active or not.
> >
> > Signed-off-by: Brandon Williams <[email-masked]>
> > ---
> >  builtin/submodule--helper.c    | 11 ++++++++
> >  t/t7413-submodule-is-active.sh | 63 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 74 insertions(+)
> >  create mode 100755 t/t7413-submodule-is-active.sh
> >
> > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> > index df0d9c166..dac02604d 100644
> > --- a/builtin/submodule--helper.c
> > +++ b/builtin/submodule--helper.c
> > @@ -1128,6 +1128,16 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
> >         return 0;
> >  }
> >
> > +static int is_active(int argc, const char **argv, const char *prefix)
> > +{
> > +       if (argc != 2)
> > +               die("submodule--helper is-active takes exactly 1 arguments");
> > +
> > +       gitmodules_config();
> > +
> > +       return !is_submodule_initialized(argv[1]);
> > +}
> > +
> >  #define SUPPORT_SUPER_PREFIX (1<<0)
> >
> >  struct cmd_struct {
> > @@ -1147,6 +1157,7 @@ static struct cmd_struct commands[] = {
> >         {"init", module_init, 0},
> >         {"remote-branch", resolve_remote_submodule_branch, 0},
> >         {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
> > +       {"is-active", is_active, 0},
> >  };
> >
> >  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> > diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh
> > new file mode 100755
> > index 000000000..683487020
> > --- /dev/null
> > +++ b/t/t7413-submodule-is-active.sh
> > @@ -0,0 +1,63 @@
> > +#!/bin/sh
> > +
> > +test_description='Test submodule--helper is-active
> > +
> > +This test verifies that `git submodue--helper is-active` correclty identifies
> > +submodules which are "active" and interesting to the user.
> > +'
> > +
> > +. ./test-lib.sh
> > +
> > +test_expect_success 'setup' '
> > +       git init sub &&
> > +       test_commit -C sub initial &&
> > +       git init super &&
> > +       test_commit -C super initial &&
> > +       git -C super submodule add ../sub sub1 &&
> > +       git -C super submodule add ../sub sub2 &&
> > +       git -C super commit -a -m "add 2 submodules at sub{1,2}"
> > +'
> > +
> > +test_expect_success 'is-active works with urls' '
> > +       git -C super submodule--helper is-active sub1 &&
> > +       git -C super submodule--helper is-active sub2 &&
> > +
> > +       git -C super config --unset submodule.sub1.URL &&
> > +       test_must_fail git -C super submodule--helper is-active sub1 &&
> > +       git -C super config submodule.sub1.URL ../sub &&
> > +       git -C super submodule--helper is-active sub1
> > +'
> > +
> > +test_expect_success 'is-active works with basic submodule.active config' '
> > +       git -C super config --add submodule.active "." &&
> > +       git -C super config --unset submodule.sub1.URL &&
> > +       git -C super config --unset submodule.sub2.URL &&
> 
> I think we'd want to unset only one of them here
> 
> > +
> > +       git -C super submodule--helper is-active sub1 &&
> > +       git -C super submodule--helper is-active sub2 &&
> 
> to test 2 different cases of one being active by config setting only and
> the other having both.

Will do.

> 
> I could not spot test for having the URL set but the config setting set, not
> including the submodule, e.g.
> 
>     git -C super config  submodule.sub1.URL ../sub &&
>     git -C super submodule.active  ":(exclude)sub1" &&
> 
> which would be expected to not be active, as once the configuration
> is there it takes precedence over any (no)URL setting?

The last test, tests this functionality as the URL settings for both
sub1 and sub2 are in the config.  You'll notice in the tests where I
unset the URL config, that they get added back at the end of the test so
that future tests have a clean state to work with.

-- 
Brandon Williams