* Bug with "git mv" and submodules, and with "git submodule add something --force" @ 2018-10-19 12:33 Juergen Vogl 2018-10-19 20:40 ` Stefan Beller 0 siblings, 1 reply; 6+ messages in thread From: Juergen Vogl @ 2018-10-19 12:33 UTC (permalink / raw) To: git Hi there, tested on both, git 2.18 and git 2.19.1: moving a file with `git mv` from a project to a submodule results in an **undefined state** of the local repository. It breaks up the submodule (it's still in .gitmodules, but not accessable via `git submodule`), and is not reversible on local repository. Either `git mv submodule/file .` nor deleting the folder works. For the locale repo the submodule is gone. But: trying to add it with `git submodule add` also do not work and results in an error message (with and without `--force` flag): $ git submodule add git@github.com:-----------/wiki-public.git public --force A git directory for 'public' is found locally with remote(s): origin git@github.com:-----------/wiki-public.git If you want to reuse this local git directory instead of cloning again from git@github.com:-----------/wiki-public.git use the '--force' option. If the local git directory is not the correct repo or you are unsure what this means choose another name with the '--name' option. Therefore, it's in a undefined, broken state. Another bug I've got by testing upper line: * --force will be used as folder name * when used in `git submodule add git@github.com:someone/some.git --force`: $ git submodule add git@github.com:---/wiki-public.git --force Cloning into '/home/---/---/---/---/wiki-internal.wiki/--force'... remote: Enumerating objects: 29, done. remote: Counting objects: 100% (29/29), done. remote: Compressing objects: 100% (25/25), done. remote: Total 29 (delta 5), reused 20 (delta 2), pack-reused 0 Receiving objects: 100% (29/29), 37.03 KiB | 421.00 KiB/s, done. Resolving deltas: 100% (5/5), done. /usr/libexec/git-core/git-submodule: line 273: cd: --: invalid option cd: usage: cd [-L|-P] [dir] Unable to checkout submodule '--force' but it creates the `--force` folder: $ tree . ├── --force Best, Jürgen Vogl ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug with "git mv" and submodules, and with "git submodule add something --force" 2018-10-19 12:33 Bug with "git mv" and submodules, and with "git submodule add something --force" Juergen Vogl @ 2018-10-19 20:40 ` Stefan Beller 2018-10-19 20:58 ` Jonathan Nieder 0 siblings, 1 reply; 6+ messages in thread From: Stefan Beller @ 2018-10-19 20:40 UTC (permalink / raw) To: juergen.vogl; +Cc: git On Fri, Oct 19, 2018 at 5:43 AM Juergen Vogl <juergen.vogl@jku.at> wrote: > > Hi there, > > tested on both, git 2.18 and git 2.19.1: > > moving a file with `git mv` from a project to a submodule results in an > **undefined state** of the local repository. Luckily we do have a submodule in git.git itself, so we can try it out here as well (I'll use a separate worktree to not hose my main repo): git worktree add ../testgit && cd ../testgit git checkout v2.19.1 && make install git --version git version 2.19.1 git submodule update --init Cloning into '/home/sbeller/testgit/sha1collisiondetection'... Submodule path 'sha1collisiondetection': checked out '232357eb2ea0397388254a4b188333a227bf5b10' git mv cache.h sha1collisiondetection/ git status HEAD detached at v2.19.1 Changes to be committed: (use "git reset HEAD <file>..." to unstage) deleted: sha1collisiondetection renamed: cache.h -> sha1collisiondetection/cache.h Untracked files: (use "git add <file>..." to include in what will be committed) sha1collisiondetection/.gitignore sha1collisiondetection/.travis.yml sha1collisiondetection/LICENSE.txt sha1collisiondetection/Makefile sha1collisiondetection/README.md sha1collisiondetection/lib/ sha1collisiondetection/src/ sha1collisiondetection/test/ sha1collisiondetection/vs2015/ > It breaks up the submodule (it's still in .gitmodules, but not > accessable via `git submodule`), and is not reversible on local repository. This seems like what I just did. However reversing can be done via: git checkout -f git status HEAD detached at v2.19.1 nothing to commit, working tree clean git submodule status 232357eb2ea0397388254a4b188333a227bf5b10 sha1collisiondetection (stable-v1.0.3-31-g232357e) So I think it's just "git-mv" that doesn't respect submodule boundaries, which we would want to address. The man page of git mv (https://git-scm.com/docs/git-mv) actually has a short note about submodules, but that is about moving *a* submodule not about moving things in and out. Maybe for now we can do with just an update of the documentation/bugs section and say we cannot move files in and out of submodules? > > Either `git mv submodule/file .` which is just running the command in reverse, (i.e. swapping destination and target), git mv cache.h sha1collisiondetection/ git mv sha1collisiondetection/cache.h cache.h git status HEAD detached at v2.19.1 Changes to be committed: (use "git reset HEAD <file>..." to unstage) deleted: sha1collisiondetection Untracked files: (use "git add <file>..." to include in what will be committed) sha1collisiondetection/ which seems like the submodule is gone, > nor deleting the folder works. For the > locale repo the submodule is gone. Yup, that seems to be the case. > But: trying to add it with `git > submodule add` also do not work and results in an error message (with > and without `--force` flag): Would checkout out a state where the submodule still exists (such as HEAD) and then running "git submodule update --init" fix it instead? > > $ git submodule add git@github.com:-----------/wiki-public.git public > --force > A git directory for 'public' is found locally with remote(s): > origin git@github.com:-----------/wiki-public.git > If you want to reuse this local git directory instead of cloning again from > git@github.com:-----------/wiki-public.git > use the '--force' option. If the local git directory is not the correct repo > or you are unsure what this means choose another name with the '--name' > option. > > Therefore, it's in a undefined, broken state. > > > Another bug I've got by testing upper line: > * --force will be used as folder name * when used in `git submodule add > git@github.com:someone/some.git --force`: Yes, the order of arguments is important, the options (such as --force) goes before the URL and path. > /usr/libexec/git-core/git-submodule: line 273: cd: --: invalid option > cd: usage: cd [-L|-P] [dir] > Unable to checkout submodule '--force' Gah! We'd need to escape the path after the options, i.e. cd -- --force would cd into that directory, but I am unsure if that is accepted by all shells. Thanks, Stefan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug with "git mv" and submodules, and with "git submodule add something --force" 2018-10-19 20:40 ` Stefan Beller @ 2018-10-19 20:58 ` Jonathan Nieder 2018-10-22 2:52 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Nieder @ 2018-10-19 20:58 UTC (permalink / raw) To: Stefan Beller; +Cc: juergen.vogl, git Stefan Beller wrote: > Maybe for now we can do with just an update of the documentation/bugs > section and say we cannot move files in and out of submodules? I think we have some existing logic to prevent "git add"-ing a file within a submodule to the superproject, for example. So "git mv" should learn the same trick. And perhaps the trick needs to be moved down a layer (e.g. into the index API). Hints? Thanks, Jonathan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug with "git mv" and submodules, and with "git submodule add something --force" 2018-10-19 20:58 ` Jonathan Nieder @ 2018-10-22 2:52 ` Junio C Hamano 2018-10-22 21:52 ` Stefan Beller 0 siblings, 1 reply; 6+ messages in thread From: Junio C Hamano @ 2018-10-22 2:52 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Stefan Beller, juergen.vogl, git Jonathan Nieder <jrnieder@gmail.com> writes: > Stefan Beller wrote: > >> Maybe for now we can do with just an update of the documentation/bugs >> section and say we cannot move files in and out of submodules? > > I think we have some existing logic to prevent "git add"-ing a file > within a submodule to the superproject, for example. There is die_path_inside_submodule() that sanity-checks the pathspec and rejects. But I think that is done primarily to give an error message and not strictly necesary for correctness. The real safety of "git add" is its call to dir.c::fill_directory(); it collects untracked paths that match the pathspec so that they can be added as new paths, but because it won't cross the module boundary, you won't get such a path in the index to begin with. > So "git mv" should learn the same trick. And perhaps the trick needs > to be moved down a layer (e.g. into the index API). Hints? You would want to be able to remove a submodule and replace it with a directory, but you can probably do it in two steps, i.e. git reset --hard git rm --cached sha1collisiondetection echo Now a regular dir >sha1collisiondetection/READ.ME find sha1collisiondetection ! -type d -print0 | git update-index --add --stdin -z So from that point of view, forbidding (starting from the same state of our project) this sequence: git reset --hard echo Now a regular dir >sha1collisiondetection/READ.ME find sha1collisiondetection ! -type d -print0 | git update-index --add --remove --stdin -z that would nuke the submodule and replace it with a directory within which there are files would be OK. Making the latter's default rejection overridable with ADD_CACHE_OK_TO_REPLACE would also be fine. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Bug with "git mv" and submodules, and with "git submodule add something --force" 2018-10-22 2:52 ` Junio C Hamano @ 2018-10-22 21:52 ` Stefan Beller 2018-10-24 7:18 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Stefan Beller @ 2018-10-22 21:52 UTC (permalink / raw) To: gitster; +Cc: git, jrnieder, juergen.vogl, sbeller On Sun, Oct 21, 2018 at 7:52 PM Junio C Hamano <gitster@pobox.com> wrote: > > Jonathan Nieder <jrnieder@gmail.com> writes: > > > Stefan Beller wrote: > > > >> Maybe for now we can do with just an update of the documentation/bugs > >> section and say we cannot move files in and out of submodules? > > > > I think we have some existing logic to prevent "git add"-ing a file > > within a submodule to the superproject, for example. > > There is die_path_inside_submodule() that sanity-checks the pathspec > and rejects. But I think that is done primarily to give an error > message and not strictly necesary for correctness. c08397e3aa (pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag, 2017-05-11) seems to be relevant here, as that factors out the warning. > The real safety of "git add" is its call to dir.c::fill_directory(); > it collects untracked paths that match the pathspec so that they can > be added as new paths, but because it won't cross the module > boundary, you won't get such a path in the index to begin with. It would not cross the boundary and fail silently as it would treat a path into the submodule as a no-op. > > So "git mv" should learn the same trick. And perhaps the trick needs > > to be moved down a layer (e.g. into the index API). Hints? Yeah, I think we'd want to teach git-mv about that trick. Unfortunately git-mv is one of the last remainders of not properly using pathspecs, and die_path_inside_submodule expects a pathspec. :-/ > You would want to be able to remove a submodule and replace it with > a directory, but you can probably do it in two steps, i.e. > > git reset --hard > git rm --cached sha1collisiondetection > echo Now a regular dir >sha1collisiondetection/READ.ME > find sha1collisiondetection ! -type d -print0 | > git update-index --add --stdin -z "Ignoring path sha1collisiondetection/.git" Nice! > > So from that point of view, forbidding (starting from the same state > of our project) this sequence: > > git reset --hard > echo Now a regular dir >sha1collisiondetection/READ.ME > find sha1collisiondetection ! -type d -print0 | > git update-index --add --remove --stdin -z > > that would nuke the submodule and replace it with a directory within > which there are files would be OK. Making the latter's default > rejection overridable with ADD_CACHE_OK_TO_REPLACE would also be > fine. I am having trouble of relating these commands to the original git-mv across submodule boundaries. Moving files from the submodule out to the superproject, seems to fail properly, though having a less-than-optimal error message: $ git mv sha1collisiondetection/LICENSE.txt . fatal: not under version control, source=sha1collisiondetection/LICENSE.txt, destination=LICENSE.txt and moving things inside was the original report, below is a proof of concept that would yield ./git mv -v cache.h sha1collisiondetection/ fatal: moving across submodule boundaries not supported, source=cache.h, destination=sha1collisiondetection/cache.h --8<-- Subject: [WIP/PATCH] builtin/mv.c: disallow moving across submodule boundaries Signed-off-by: Stefan Beller <sbeller@google.com> --- builtin/mv.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/builtin/mv.c b/builtin/mv.c index 80bb967a63..9ec4b2f0a3 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -172,7 +172,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) /* Checking */ for (i = 0; i < argc; i++) { const char *src = source[i], *dst = destination[i]; - int length, src_is_dir; + int length, src_is_dir, pos; const char *bad = NULL; if (show_only) @@ -243,6 +243,13 @@ int cmd_mv(int argc, const char **argv, const char *prefix) else string_list_insert(&src_for_dst, dst); + pos = cache_name_pos(dst, strlen(dst)); + if (pos < 0) { + pos = -(pos + 1); + if (!S_ISGITLINK(active_cache[pos]->ce_mode)) + bad = _("moving across submodule boundaries not supported"); + } + if (!bad) continue; if (!ignore_errors) -- 2.19.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Bug with "git mv" and submodules, and with "git submodule add something --force" 2018-10-22 21:52 ` Stefan Beller @ 2018-10-24 7:18 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2018-10-24 7:18 UTC (permalink / raw) To: Stefan Beller; +Cc: git, jrnieder, juergen.vogl Stefan Beller <sbeller@google.com> writes: >> You would want to be able to remove a submodule and replace it with >> a directory, but you can probably do it in two steps, i.e. >> >> git reset --hard >> git rm --cached sha1collisiondetection >> echo Now a regular dir >sha1collisiondetection/READ.ME >> find sha1collisiondetection ! -type d -print0 | >> git update-index --add --stdin -z > > "Ignoring path sha1collisiondetection/.git" > > Nice! There actually is another possible outcome that anybody following along must be aware of and be careful about: not even .git directory exist there, i.e. it is possible that the submodule has never been init'ed. So, it is not that nice X-<. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-24 7:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-19 12:33 Bug with "git mv" and submodules, and with "git submodule add something --force" Juergen Vogl 2018-10-19 20:40 ` Stefan Beller 2018-10-19 20:58 ` Jonathan Nieder 2018-10-22 2:52 ` Junio C Hamano 2018-10-22 21:52 ` Stefan Beller 2018-10-24 7:18 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).