Re: [PATCHv2 4/4] submodule: add embed-git-dir function
- From
Stefan Beller <sbeller@google.com>
- Date
- Nov 30, 2016, 18:04 UTC
- Message-ID
- <CAGZ79kbvey_f8+R16yYT_qsF0RErOh8own8n-RRApTM0dS-+ag@mail.gmail.com>
- In-Reply-To
- <CACsJy8Ce3Oa-xJ4BwgRRy6neM=Jxkfqq7yboHZDXLDG2tu9GzQ@mail.gmail.com>
On Wed, Nov 30, 2016 at 3:14 AM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 10 quoted lines
> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller <sbeller@google.com> wrote: >> +/* >> + * Migrate the given submodule (and all its submodules recursively) from >> + * having its git directory within the working tree to the git dir nested >> + * in its superprojects git dir under modules/. >> + */ >> +void migrate_submodule_gitdir(const char *prefix, const char *path, >> + int recursive) > > Submodules and worktrees seem to have many things in common.
Yes. :)
> The first > one is this. "git worktree move" on a worktree that contains > submodules .git also benefits from something like this [1].
That patch is a sensible approach. :) (By checking all files to not be submodules a worktree would not run into problems like a127331cd81, mv: allow moving nested submodules)
> I suggest > you move this function to some neutral place and maybe rename it to > relocate_gitdir() or something.
ok tell me where this neutral place is found? (I'd prefer to not clobber it into cache.h *the* most neutral place in git) Maybe dir.{c,h} ?
Show 6 quoted lines
> > It probably should take a bit flag instead of "recursive" here. One > thing I would need is the ability to tell this function "I have moved > all these .git dirs already (because I move whole worktree in one > operation), here are the old and new locations of them, fix them up!". > In other words, no rename() could be optionally skipped.
In the non-main working trees you'd also have a .git file linking to the actual git dir and you'd only have to fix them up instead of moving.
Show 23 quoted lines
>
> [1] https://public-inbox.org/git/20161128094319.16176-11-pclouds@gmail.com/T/#u
>
>> +{
>> + char *old_git_dir;
>> + const char *new_git_dir;
>> + const struct submodule *sub;
>> +
>> + old_git_dir = xstrfmt("%s/.git", path);
>> + if (read_gitfile(old_git_dir))
>> + /* If it is an actual gitfile, it doesn't need migration. */
>> + goto out;
>> +
>> + sub = submodule_from_path(null_sha1, path);
>> + if (!sub)
>> + die(_("Could not lookup name for submodule '%s'"),
>> + path);
>> +
>> + new_git_dir = git_common_path("modules/%s", sub->name);
>
> Why doesn't git_path() work here? This would make "modules" shared
> between worktrees, even though it's not normally. That inconsistency
> could cause trouble.I thought that was a long term goal? (I actually think about reviving the series you sent out a few weeks ago to make worktree and submodules work well together)
So for that we'd want to have at least the object store shared across all worktrees.
Show 20 quoted lines
>
>> + if (safe_create_leading_directories_const(new_git_dir) < 0)
>> + die(_("could not create directory '%s'"), new_git_dir);
>> +
>> + if (!prefix)
>> + prefix = get_super_prefix();
>> + printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
>> + prefix ? prefix : "", path,
>> + real_path(old_git_dir), new_git_dir);
>> +
>> + if (rename(old_git_dir, new_git_dir) < 0)
>> + die_errno(_("Could not migrate git directory from '%s' to '%s'"),
>> + old_git_dir, new_git_dir);
>> +
>> + connect_work_tree_and_git_dir(path, new_git_dir);
>
> Another thing in common is, both submodules and worktrees use some
> form of textual symlinks. You need to fix up some here. But if this
> submodule has multiple worktreee, there may be some "symlinks" in
> .git/worktrees which would need fixing up as well.We could signal that via one of the flag bits? (e.g. FIXUP_WORKTREE_SYMLINKS )
Show 6 quoted lines
> > You don't have to do the fix up thing right away, but I think we > should at least make sure we leave no dangling links behind (by > die()ing early if we find a .git dir we can't handle yet) > -- > Duy