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

Re: [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path

From
Antonio Ospite <ao2@ao2.it>
Date
Apr 16, 2018, 16:37 UTC
Message-ID
<20180416183742.032530a4d44faccd44189ea6@ao2.it>
In-Reply-To
<CAGZ79kZV+MLaVp4AM77bJkT=MeBvDSHTxZCni8dkqq4VLWZ0AA@mail.gmail.com>

On Thu, 12 Apr 2018 16:50:03 -0700 Stefan Beller <sbeller@google.com> wrote:

Show 14 quoted lines
> Hi Antonio,
> 
> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> > When multiple repositories with detached work-trees take turns using the
> > same directory as their work-tree, and more than one of them want to use
> > submodules, there will be conflicts about the '.gitmodules' file.
> 
> unlike other files which would not conflict?
> There might be file names such as LICENSE, Readme.md etc,
> which are common enough that they would produce conflicts as well?
> I find this argument on its own rather weak. ("Just delete everything in
> the working dir before using it with another repository"). I might be
> missing a crucial bit here?
>

All the vcsh repositories _share_ the same work-tree; they may control it taking turns but, in general, all files are meant to be checked out at all times as the basic use case is: *distinct* sets of config files.

Maybe saying that the repositories "take turns" is confusing. It's an unnecessary information, so I will omit that part form the commit message.

After your question I've done some research and I've seen other vcsh users managing conflicting LICENSE and README files using git sparse-checkouts, to have these files in the single repositories but not checked out in the shared work-tree: https://github.com/RichiH/vcsh/issues/120#issuecomment-42639619 https://github.com/jwhitley/vcsh-root/commit/30b0d495c2cbe47ae9617ace9c2c14720d961d78

However I guess that my point here is that the gitmodules file is something that influences git behavior so it should not be on the user's shoulder to manage conflicts for it, and most importantly it needs to be checked out for git to access it, doesn't it?

Show 29 quoted lines
> > git hardcodes this path so it's not possible to override its location on
> > a per-repository basis to allow such repositories to coexists
> > peacefully.
> >
> > Make the path of the "gitmodules file" customizable exposing
> > a 'core.submodulesFile' configuration setting.
> >
> > The default value will still be '.gitmodules' when 'core.submodulesFile'
> > is not set.
> 
> ok.
> 
> 
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1774,6 +1774,7 @@ extern void prepare_pager_args(struct child_process *, const char *pager);
> >  extern const char *editor_program;
> >  extern const char *askpass_program;
> >  extern const char *excludes_file;
> > +extern const char *submodules_file;
> 
> Could you place this variable in repository.h in struct repository?
> (Some developers currently try to move any global state to that place,
> as that makes working with e.g. nested submodules easier in-process
> and you would not need to spawn processes for submodules)
> 
> Once migrated to the repository struct mentioned above, you'd access
> it via the_repository->submodules_file for the main repository.
>

OK, thanks, I didn't like the global variable either, I was just copying from excludes_file.

Show 19 quoted lines
> 
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 24914963c..610fd0dc5 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -71,7 +71,9 @@ get_submodule_config () {
> >         value=$(git config submodule."$name"."$option")
> >         if test -z "$value"
> >         then
> > -               value=$(git config -f .gitmodules submodule."$name"."$option")
> > +               gitmodules_file=$(git config core.submodulesfile)
> > +               : ${gitmodules_file:=.gitmodules}
> > +               value=$(git config -f "$gitmodules_file" submodule."$name"."$option")
> 
> I wonder if it would be cheaper to write a special config lookup now, e.g.
> in builtin/submodule--helper.c we could have a "config-from-gitmodules"
> subcommand that is looking up the modules file and then running the config
> on that file.
>

Can you give an example from the user point of view of such a "config-from-gitmodules" command?

I might look into it, but that can also be a followup change.
Show 41 quoted lines
> I am surprised how little access of the .gitmodules is left in git-submodule.sh
> (which is partially ported to the builtin/submodule--helper.c)
> 
> > diff --git a/submodule-config.c b/submodule-config.c
> > index 3f2075764..8a3396ade 100644
> > --- a/submodule-config.c
> > +++ b/submodule-config.c
> > @@ -468,7 +468,7 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name,
> >                 return 1;
> >         }
> >
> > -       strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name));
> > +       strbuf_addf(rev, "%s:%s", oid_to_hex(treeish_name), submodules_file);
> >         if (get_oid(rev->buf, gitmodules_oid) >= 0)
> >                 ret = 1;
> >
> > @@ -583,7 +583,7 @@ void repo_read_gitmodules(struct repository *repo)
> >                 if (repo_read_index(repo) < 0)
> >                         return;
> >
> > -               gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
> > +               gitmodules = repo_worktree_path(repo, submodules_file);
> >
> >                 if (!is_gitmodules_unmerged(repo->index))
> >                         git_config_from_file(gitmodules_cb, gitmodules, repo);
> > diff --git a/submodule.c b/submodule.c
> > index 9a50168b2..2afbdb644 100644
> > --- a/submodule.c
> > +++ b/submodule.c
> > @@ -36,13 +36,13 @@ static struct oid_array ref_tips_after_fetch;
> >   */
> >  int is_gitmodules_unmerged(const struct index_state *istate)
> >  {
> > -       int pos = index_name_pos(istate, GITMODULES_FILE, strlen(GITMODULES_FILE));
> > +       int pos = index_name_pos(istate, submodules_file, strlen(submodules_file));
> 
> Ah, regarding the coverletter: This clearly assumes the modules
> file is in the tree. So at least here we would make an exception
> for files outside the tree to either not check for un-merged-ness or
> disallow that case entirely.
>

Sorry I am not sure I follow what you are saying here, keep in mind that I am new to git internals.

Do you mean that, even if we ensure (in config.c::git_default_core_config) that only paths relative to the work-tree are allowed, we still have to check here that the constraint is respected? And is so, why?

Show 6 quoted lines
> There are quite a few functions in submodule.c which access the new global. :/
> So moving them to the_repository should be fine, but eventually (not
> in this series)
> all these functions would would want to take a repository argument as well
> such that they work on more than the_repository.
>

I will surely get rid of the global variable, but about changing the functions signatures I don't feel like promising anything just yet.

Thanks,
   Antonio
-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
Previous: Stefan BellerNext: Stefan Beller
Message 10 of 28 in “[RFC 00/10] Make .the gitmodules file path configurable”
  1. Antonio OspiteApr 12, 2018
  2. 03/10 submodule: use the 'submodules_file' variable in output messagesAntonio Ospite, Apr 12, 2018
  3. 02/10 submodule: fix getting custom gitmodule file in fetch commandAntonio Ospite, Apr 12, 2018
  4. Stefan BellerApr 12, 2018
  5. Antonio OspiteApr 16, 2018
  6. Stefan BellerApr 16, 2018
  7. Antonio OspiteApr 16, 2018
  8. 01/10 submodule: add 'core.submodulesFile' to override the '.gitmodules' pathAntonio Ospite, Apr 12, 2018
  9. Stefan BellerApr 12, 2018
  10. Antonio OspiteApr 16, 2018
  11. Stefan BellerApr 16, 2018
  12. Antonio OspiteApr 18, 2018
  13. Stefan BellerApr 18, 2018
  14. 04/10 submodule: document 'core.submodulesFile' and fix references to '.gitmodules'Antonio Ospite, Apr 12, 2018
  15. 05/10 submodule: adjust references to '.gitmodules' in commentsAntonio Ospite, Apr 12, 2018
  16. 06/10 completion: add 'core.submodulesfile' to the git-completion.bash fileAntonio Ospite, Apr 12, 2018
  17. Stefan BellerApr 12, 2018
  18. Antonio OspiteApr 16, 2018
  19. Stefan BellerApr 16, 2018
  20. Antonio OspiteApr 13, 2018
  21. 07/10 FIXME: wrap-for-bin.sh: set 'core.submodulesFile' for each git invocationAntonio Ospite, Apr 13, 2018
  22. 09/10 FIXME: submodule: pass custom gitmodules file to 'test-tool submodule-config'Antonio Ospite, Apr 13, 2018
  23. 10/10 FIXME: add a hacky script to test the changes with a patched test suiteAntonio Ospite, Apr 13, 2018
  24. 08/10 FIXME: submodule: fix t1300-repo-config.sh to take into account the new configAntonio Ospite, Apr 13, 2018
  25. Stefan BellerApr 13, 2018
  26. Antonio OspiteApr 16, 2018
  27. Jonathan NiederApr 23, 2018
  28. Antonio OspiteApr 30, 2018

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.