git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 06/10] submodule: decouple url and submodule interest

From
Stefan Beller <sbeller@google.com>
Date
Mar 16, 2017, 23:45 UTC
Message-ID
<CAGZ79kbMB3e8AXsK5EYjDBe=7+AccF56+kFDY5PQ9i9=R+2G0w@mail.gmail.com>
In-Reply-To
<20170316222952.53801-7-bmwill@google.com>
On Thu, Mar 16, 2017 at 3:29 PM, Brandon Williams <bmwill@google.com> wrote:
Show 13 quoted lines
> Currently the submodule.<name>.url config option is used to determine if
> a given submodule is of interest to the user.  This ends up being
> cumbersome in a world where we want to have different submodules checked
> out in different worktrees or a more generalized mechanism to select
> which submodules are of interest.
>
> In a future with worktree support for submodules, there will be multiple
> working trees, each of which may only need a subset of the submodules
> checked out.  The URL (which is where the submodule repository can be
> obtained) should not differ between different working trees.
>
> It may also be convenient for users to more easily specify groups of
> submodules they are interested in as apposed to running "git submodule
Did you mean opposed?
Show 164 quoted lines
> init <path>" on each submodule they want checked out in their working
> tree.
>
> To this end two config options are introduced, submodule.active and
> submodule.<name>.active.  The submodule.active config holds a pathspec
> that specifies which submodules should exist in the working tree.  The
> submodule.<name>.active config is a boolean flag used to indicate if
> that particular submodule should exist in the working tree.
>
> Its important to note that submodule.active functions differently than
> the other configuration options since it takes a pathspec.  This allows
> users to adopt at least two new workflows:
>
>   1. Submodules can be grouped with a leading directory, such that a
>      pathspec e.g. 'lib/' would cover all library-ish modules to allow
>      those who are interested in library-ish modules to set
>      "submodule.active = lib/" just once to say any and all modules in
>      'lib/' are interesting.
>
>   2. Once the pathspec-attribute feature is invented, users can label
>      submodules with attributes to group them, so that a broad pathspec
>      with attribute requirements, e.g. ':(attr:lib)', can be used to say
>      any and all modules with the 'lib' attribute are interesting.
>      Since the .gitattributes file, just like the .gitmodules file, is
>      tracked by the superproject, when a submodule moves in the
>      superproject tree, the project can adjust which path gets the
>      attribute in .gitattributes, just like it can adjust which path has
>      the submodule in .gitmodules.
>
> Neither of these two additional configuration options solve the problem
> of wanting different submodules checked out in different worktrees
> because multiple worktrees share .git/config.  Only once per-worktree
> configurations become a reality can this be solved, but this is a
> necessary preparatory step for that future.
>
> Given these multiple ways to check if a submodule is of interest, the
> more fine-grained submodule.<name>.active option has the highest order
> of precedence followed by the pathspec check against submodule.active.
> To ensure backwards compatibility, if neither of these options are set,
> git falls back to checking the submodule.<name>.url option to determine
> if a submodule is interesting.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  Documentation/config.txt       | 15 ++++++++++--
>  submodule.c                    | 50 ++++++++++++++++++++++++++++++++------
>  t/t7413-submodule-is-active.sh | 55 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 110 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 5e5c2ae5f..d2d79b9d4 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2920,8 +2920,9 @@ submodule.<name>.url::
>         The URL for a submodule. This variable is copied from the .gitmodules
>         file to the git config via 'git submodule init'. The user can change
>         the configured URL before obtaining the submodule via 'git submodule
> -       update'. After obtaining the submodule, the presence of this variable
> -       is used as a sign whether the submodule is of interest to git commands.
> +       update'. If neither submodule.<name>.active or submodule.active are
> +       set, the presence of this variable is used as a fallback to indicate
> +       whether the submodule is of interest to git commands.
>         See linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
>
>  submodule.<name>.update::
> @@ -2959,6 +2960,16 @@ submodule.<name>.ignore::
>         "--ignore-submodules" option. The 'git submodule' commands are not
>         affected by this setting.
>
> +submodule.<name>.active::
> +       Boolean value indicating if the submodule is of interest to git
> +       commands.  This config option takes precedence over the
> +       submodule.active config option.
> +
> +submodule.active::
> +       A repeated field which contains a pathspec used to match against a
> +       submodule's path to determine if the submodule is of interest to git
> +       commands.
> +
>  submodule.fetchJobs::
>         Specifies how many submodules are fetched/cloned at the same time.
>         A positive integer allows up to that number of submodules fetched
> diff --git a/submodule.c b/submodule.c
> index 0a2831d84..ad2779ee7 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -212,25 +212,59 @@ void gitmodules_config_sha1(const unsigned char *commit_sha1)
>  }
>
>  /*
> + * NEEDSWORK: With the addition of different configuration options to determine
> + * if a submodule is of interests, the validity of this function's name comes
> + * into question.  Once the dust has settled and more concrete terminology is
> + * decided upon, come up with a more proper name for this function.  One
> + * potential candidate could be 'is_submodule_active()'.
> + *
>   * Determine if a submodule has been initialized at a given 'path'
>   */
>  int is_submodule_initialized(const char *path)
>  {
>         int ret = 0;
> -       const struct submodule *module = NULL;
> +       char *key = NULL;
> +       char *value = NULL;
> +       const struct string_list *sl;
> +       const struct submodule *module = submodule_from_path(null_sha1, path);
>
> -       module = submodule_from_path(null_sha1, path);
> +       /* early return if there isn't a path->module mapping */
> +       if (!module)
> +               return 0;
>
> -       if (module) {
> -               char *key = xstrfmt("submodule.%s.url", module->name);
> -               char *value = NULL;
> +       /* submodule.<name>.active is set */
> +       key = xstrfmt("submodule.%s.active", module->name);
> +       if (!git_config_get_bool(key, &ret)) {
> +               free(key);
> +               return ret;
> +       }
> +       free(key);
>
> -               ret = !git_config_get_string(key, &value);
> +       /* submodule.active is set */
> +       sl = git_config_get_value_multi("submodule.active");
> +       if (sl) {
> +               struct pathspec ps;
> +               struct argv_array args = ARGV_ARRAY_INIT;
> +               const struct string_list_item *item;
>
> -               free(value);
> -               free(key);
> +               for_each_string_list_item(item, sl) {
> +                       argv_array_push(&args, item->string);
> +               }
> +
> +               parse_pathspec(&ps, 0, 0, NULL, args.argv);
> +               ret = match_pathspec(&ps, path, strlen(path), 0, NULL, 1);
> +
> +               argv_array_clear(&args);
> +               clear_pathspec(&ps);
> +               return ret;
>         }
>
> +       /* fallback to checking if the URL is set */
> +       key = xstrfmt("submodule.%s.url", module->name);
> +       ret = !git_config_get_string(key, &value);
> +
> +       free(value);
> +       free(key);
>         return ret;
>  }
>
> diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh
> index f18e0c925..ea1644b58 100755
> --- a/t/t7413-submodule-is-active.sh
> +++ b/t/t7413-submodule-is-active.sh
> @@ -28,4 +28,59 @@ test_expect_success 'is-active works with urls' '
>         git -C super submodule--helper is-active sub1
>  '
>
> +test_expect_success 'is-active works with submodule.<name>.active config' '
> +       test_when_finished "git -C super config --unset submodule.sub1.active" &&

Sorry for late notice: This can be solved more elegantly with test_config[_global]. If this is the only nit, we can fix it later, though.

> +       test_when_finished "git -C super config submodule.sub1.URL ../sub" &&

All tests look like they are structured very similarly; we also want to test all combinations, so maybe it is easier to make a function

test_active_case()
{
    on_submodule=$1
    expectation=$2
    with_URL=$3
    with_active_flag=$4
    with_active_pathspec=$5
    test_expect_success 'is_active for $1 $2 $3 $4 $5' '
      # do the actual test
    '
}
# here put all combinations for a submodule + the non-submodule case

I am not sure though, it would be a more disciplined approach, but not necessarily be covering the corner cases just as nice, i.e. having multiple pathspecs, including 'exclude's.

Just food for thought.

Thanks, Stefan

Previous: Brandon WilliamsNext: Brandon Williams
Message 88 of 123 in “decoupling a submodule's existence and its url”
  1. 00/10 decoupling a submodule's existence and its urlBrandon Williams, Feb 23, 2017
  2. 06/10 submodule add: respect submodule.activeBrandon Williams, Feb 23, 2017
  3. 09/10 submodule sync: use submodule--helper is-activeBrandon Williams, Feb 23, 2017
  4. 04/10 completion: clone can initialize specific submodulesBrandon Williams, Feb 23, 2017
  5. 01/10 submodule: decouple url and submodule existenceBrandon Williams, Feb 23, 2017
  6. Junio C HamanoFeb 24, 2017
  7. Brandon WilliamsMar 1, 2017
  8. Stefan BellerMar 1, 2017
  9. Brandon WilliamsMar 6, 2017
  10. Junio C HamanoMar 2, 2017
  11. 03/10 clone: add --submodule-spec=<pathspec> switchBrandon Williams, Feb 23, 2017
  12. 07/10 submodule status: use submodule--helper is-activeBrandon Williams, Feb 23, 2017
  13. 08/10 submodule deinit: use most reliable urlBrandon Williams, Feb 23, 2017
  14. 10/10 submodule--helper clone: check for configured submodules using helperBrandon Williams, Feb 23, 2017
  15. Stefan BellerFeb 24, 2017
  16. Brandon WilliamsFeb 27, 2017
  17. 02/10 submodule update: add `--init-active` switchBrandon Williams, Feb 23, 2017
  18. 05/10 submodule--helper: add is_active commandBrandon Williams, Feb 23, 2017
  19. Stefan BellerFeb 24, 2017
  20. Brandon WilliamsFeb 27, 2017
  21. Stefan BellerFeb 23, 2017
  22. 00/11 decoupling a submodule's existence and its urlBrandon Williams, Mar 9, 2017
  23. 05/11 submodule--helper clone: check for configured submodules using helperBrandon Williams, Mar 9, 2017
  24. 02/11 submodule status: use submodule--helper is-activeBrandon Williams, Mar 9, 2017
  25. 04/11 submodule sync: use submodule--helper is-activeBrandon Williams, Mar 9, 2017
  26. 01/11 submodule--helper: add is_active commandBrandon Williams, Mar 9, 2017
  27. 11/11 submodule add: respect submodule.active and submodule.<name>.activeBrandon Williams, Mar 9, 2017
  28. 06/11 submodule: decouple url and submodule existenceBrandon Williams, Mar 9, 2017
  29. 09/11 completion: clone can initialize specific submodulesBrandon Williams, Mar 9, 2017
  30. 07/11 submodule update: add `--init-active` switchBrandon Williams, Mar 9, 2017
  31. Stefan BellerMar 9, 2017
  32. Brandon WilliamsMar 9, 2017
  33. 08/11 clone: add --submodule-spec=<pathspec> switchBrandon Williams, Mar 9, 2017
  34. 10/11 submodule--helper init: set submodule.<name>.activeBrandon Williams, Mar 9, 2017
  35. Stefan BellerMar 9, 2017
  36. Brandon WilliamsMar 9, 2017
  37. 03/11 submodule deinit: use most reliable urlBrandon Williams, Mar 9, 2017
  38. Stefan BellerMar 9, 2017
  39. Brandon WilliamsMar 9, 2017
  40. 00/10 decoupling a submodule's existence and its urlBrandon Williams, Mar 13, 2017
  41. 01/10 submodule--helper: add is_active commandBrandon Williams, Mar 13, 2017
  42. Stefan BellerMar 13, 2017
  43. Junio C HamanoMar 14, 2017
  44. Brandon WilliamsMar 14, 2017
  45. 02/10 submodule status: use submodule--helper is-activeBrandon Williams, Mar 13, 2017
  46. Junio C HamanoMar 14, 2017
  47. Stefan BellerMar 14, 2017
  48. Junio C HamanoMar 14, 2017
  49. Junio C HamanoMar 14, 2017
  50. Brandon WilliamsMar 14, 2017
  51. Brandon WilliamsMar 14, 2017
  52. 03/10 submodule sync: use submodule--helper is-activeBrandon Williams, Mar 13, 2017
  53. Junio C HamanoMar 14, 2017
  54. Brandon WilliamsMar 14, 2017
  55. 05/10 submodule: decouple url and submodule existenceBrandon Williams, Mar 13, 2017
  56. Stefan BellerMar 13, 2017
  57. Brandon WilliamsMar 15, 2017
  58. Junio C HamanoMar 14, 2017
  59. Junio C HamanoMar 14, 2017
  60. Brandon WilliamsMar 15, 2017
  61. 07/10 clone: add --submodule-spec=<pathspec> switchBrandon Williams, Mar 13, 2017
  62. Junio C HamanoMar 14, 2017
  63. Brandon WilliamsMar 15, 2017
  64. Stefan BellerMar 15, 2017
  65. 08/10 completion: clone can initialize specific submodulesBrandon Williams, Mar 13, 2017
  66. 09/10 submodule--helper init: set submodule.<name>.activeBrandon Williams, Mar 13, 2017
  67. Junio C HamanoMar 14, 2017
  68. Brandon WilliamsMar 15, 2017
  69. Junio C HamanoMar 16, 2017
  70. 04/10 submodule--helper clone: check for configured submodules using helperBrandon Williams, Mar 13, 2017
  71. Junio C HamanoMar 14, 2017
  72. Stefan BellerMar 14, 2017
  73. Junio C HamanoMar 14, 2017
  74. Brandon WilliamsMar 15, 2017
  75. 10/10 submodule add: respect submodule.active and submodule.<name>.activeBrandon Williams, Mar 13, 2017
  76. Junio C HamanoMar 14, 2017
  77. Brandon WilliamsMar 15, 2017
  78. 06/10 submodule update: add `--init-active` switchBrandon Williams, Mar 13, 2017
  79. Junio C HamanoMar 14, 2017
  80. Brandon WilliamsMar 15, 2017
  81. Junio C HamanoMar 14, 2017
  82. Brandon WilliamsMar 15, 2017
  83. Stefan BellerMar 13, 2017
  84. Junio C HamanoMar 14, 2017
  85. Brandon WilliamsMar 14, 2017
  86. 00/10 decoupling url and submodule interestBrandon Williams, Mar 16, 2017
  87. 06/10 submodule: decouple url and submodule interestBrandon Williams, Mar 16, 2017
  88. Stefan BellerMar 16, 2017
  89. 03/10 submodule sync: skip work for inactive submodulesBrandon Williams, Mar 16, 2017
  90. Stefan BellerMar 16, 2017
  91. Brandon WilliamsMar 16, 2017
  92. Stefan BellerMar 16, 2017
  93. Junio C HamanoMar 17, 2017
  94. Brandon WilliamsMar 17, 2017
  95. Junio C HamanoMar 17, 2017
  96. 05/10 submodule--helper clone: check for configured submodules using helperBrandon Williams, Mar 16, 2017
  97. 02/10 submodule status: use submodule--helper is-activeBrandon Williams, Mar 16, 2017
  98. 08/10 clone: teach --recurse-submodules to optionally take a pathspecBrandon Williams, Mar 16, 2017
  99. Stefan BellerMar 17, 2017
  100. Brandon WilliamsMar 17, 2017
  101. 04/10 submodule sync: use submodule--helper is-activeBrandon Williams, Mar 16, 2017
  102. 07/10 submodule init: initialize active submodulesBrandon Williams, Mar 16, 2017
  103. Stefan BellerMar 17, 2017
  104. Brandon WilliamsMar 17, 2017
  105. Stefan BellerMar 17, 2017
  106. 01/10 submodule--helper: add is-active subcommandBrandon Williams, Mar 16, 2017
  107. 10/10 submodule add: respect submodule.active and submodule.<name>.activeBrandon Williams, Mar 16, 2017
  108. 09/10 submodule--helper init: set submodule.<name>.activeBrandon Williams, Mar 16, 2017
  109. Stefan BellerMar 17, 2017
  110. Brandon WilliamsMar 17, 2017
  111. 00/10 decoupling url and submodule interestBrandon Williams, Mar 17, 2017
  112. 06/10 submodule: decouple url and submodule interestBrandon Williams, Mar 17, 2017
  113. 09/10 submodule--helper init: set submodule.<name>.activeBrandon Williams, Mar 17, 2017
  114. 05/10 submodule--helper clone: check for configured submodules using helperBrandon Williams, Mar 17, 2017
  115. 03/10 submodule sync: skip work for inactive submodulesBrandon Williams, Mar 17, 2017
  116. 02/10 submodule status: use submodule--helper is-activeBrandon Williams, Mar 17, 2017
  117. Stefan BellerMar 17, 2017
  118. 08/10 clone: teach --recurse-submodules to optionally take a pathspecBrandon Williams, Mar 17, 2017
  119. 01/10 submodule--helper: add is-active subcommandBrandon Williams, Mar 17, 2017
  120. 07/10 submodule init: initialize active submodulesBrandon Williams, Mar 17, 2017
  121. 04/10 submodule sync: use submodule--helper is-activeBrandon Williams, Mar 17, 2017
  122. 10/10 submodule add: respect submodule.active and submodule.<name>.activeBrandon Williams, Mar 17, 2017
  123. Junio C HamanoMar 18, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.