{"thread":{"id":"49615","subject":"Bug with \"git mv\" and submodules, and with \"git submodule add something --force\"","startedAt":"2018-10-19T12:43:17Z","lastAt":"2018-10-24T07:18:29Z","messageCount":6,"participants":["Juergen Vogl","Stefan Beller","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"360912","messageId":"c4792c95-9a19-fd0e-9171-fded095b78a6@jku.at","threadId":"49615","inReplyTo":null,"subject":"Bug with \"git mv\" and submodules, and with \"git submodule add something --force\"","fromName":"Juergen Vogl","fromEmail":"juergen.vogl@jku.at","sentAt":"2018-10-19T12:33:54Z","receivedAt":"2018-10-19T12:43:17Z","isPatch":false,"sender":{"key":"juergen.vogl@jku.at","avatar":null},"body":"Hi there,\n\ntested on both, git 2.18 and git 2.19.1:\n\nmoving a file with `git mv` from a project to a submodule results in an\n**undefined state** of the local repository.\n\nIt breaks up the submodule (it's still in .gitmodules, but not\naccessable via `git submodule`), and is not reversible on local repository.\n\nEither `git mv submodule/file .` nor deleting the folder works. For the\nlocale repo the submodule is gone. But: trying to add it with `git\nsubmodule add` also do not work and results in an error message (with\nand without `--force` flag):\n\n$ git submodule add git@github.com:-----------/wiki-public.git public\n--force\nA git directory for 'public' is found locally with remote(s):\n  origin        git@github.com:-----------/wiki-public.git\nIf you want to reuse this local git directory instead of cloning again from\n  git@github.com:-----------/wiki-public.git\nuse the '--force' option. If the local git directory is not the correct repo\nor you are unsure what this means choose another name with the '--name'\noption.\n\nTherefore, it's in a undefined, broken state.\n\n\nAnother bug I've got by testing upper line:\n* --force will be used as folder name * when used in `git submodule add\ngit@github.com:someone/some.git --force`:\n\n$ git submodule add git@github.com:---/wiki-public.git --force\nCloning into '/home/---/---/---/---/wiki-internal.wiki/--force'...\nremote: Enumerating objects: 29, done.\nremote: Counting objects: 100% (29/29), done.\nremote: Compressing objects: 100% (25/25), done.\nremote: Total 29 (delta 5), reused 20 (delta 2), pack-reused 0\nReceiving objects: 100% (29/29), 37.03 KiB | 421.00 KiB/s, done.\nResolving deltas: 100% (5/5), done.\n/usr/libexec/git-core/git-submodule: line 273: cd: --: invalid option\ncd: usage: cd [-L|-P] [dir]\nUnable to checkout submodule '--force'\n\nbut it creates the `--force` folder:\n\n$ tree\n.\n├── --force\n\nBest,\n\nJürgen Vogl\n\n"},{"id":"360981","messageId":"CAGZ79kZ4NSCknQTkpurdj_CzD+XvFzdaz1bdxPTFizjwV8=rpA@mail.gmail.com","threadId":"49615","inReplyTo":"c4792c95-9a19-fd0e-9171-fded095b78a6@jku.at","subject":"Re: Bug with \"git mv\" and submodules, and with \"git submodule add something --force\"","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-19T20:40:13Z","receivedAt":"2018-10-19T20:40:28Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Oct 19, 2018 at 5:43 AM Juergen Vogl wrote:\n>\n> Hi there,\n>\n> tested on both, git 2.18 and git 2.19.1:\n>\n> moving a file with `git mv` from a project to a submodule results in an\n> **undefined state** of the local repository.\n\nLuckily we do have a submodule in git.git itself, so we can\ntry it out here as well (I'll use a separate worktree to not\nhose my main repo):\n\n git worktree add ../testgit && cd ../testgit\n git checkout v2.19.1 && make install\n git --version\n git version 2.19.1\n git submodule update --init\n Cloning into '/home/sbeller/testgit/sha1collisiondetection'...\n Submodule path 'sha1collisiondetection': checked out\n'232357eb2ea0397388254a4b188333a227bf5b10'\n git mv cache.h sha1collisiondetection/\n git status\nHEAD detached at v2.19.1\nChanges to be committed:\n (use \"git reset HEAD ...\" to unstage)\n\ndeleted: sha1collisiondetection\nrenamed: cache.h -> sha1collisiondetection/cache.h\n\nUntracked files:\n (use \"git add ...\" to include in what will be committed)\n\nsha1collisiondetection/.gitignore\nsha1collisiondetection/.travis.yml\nsha1collisiondetection/LICENSE.txt\nsha1collisiondetection/Makefile\nsha1collisiondetection/README.md\nsha1collisiondetection/lib/\nsha1collisiondetection/src/\nsha1collisiondetection/test/\nsha1collisiondetection/vs2015/\n\n> It breaks up the submodule (it's still in .gitmodules, but not\n> accessable via `git submodule`), and is not reversible on local repository.\n\nThis seems like what I just did. However reversing can be done via:\n\n git checkout -f\n git status\nHEAD detached at v2.19.1\nnothing to commit, working tree clean\n git submodule status\n232357eb2ea0397388254a4b188333a227bf5b10 sha1collisiondetection\n(stable-v1.0.3-31-g232357e)\n\nSo I think it's just \"git-mv\" that doesn't respect submodule\nboundaries, which we would want to address.\n\nThe man page of git mv\n(https://git-scm.com/docs/git-mv)\nactually has a short note about submodules, but that is about\nmoving *a* submodule not about moving things in and out.\n\nMaybe for now we can do with just an update of the documentation/bugs\nsection and say we cannot move files in and out of submodules?\n\n\n\n>\n> Either `git mv submodule/file .`\n\nwhich is just running the command in reverse, (i.e. swapping\ndestination and target),\n git mv cache.h sha1collisiondetection/\n git mv sha1collisiondetection/cache.h cache.h\n git status\nHEAD detached at v2.19.1\nChanges to be committed:\n (use \"git reset HEAD ...\" to unstage)\n\ndeleted: sha1collisiondetection\n\nUntracked files:\n (use \"git add ...\" to include in what will be committed)\n\nsha1collisiondetection/\n\nwhich seems like the submodule is gone,\n\n> nor deleting the folder works. For the\n> locale repo the submodule is gone.\n\nYup, that seems to be the case.\n\n> But: trying to add it with `git\n> submodule add` also do not work and results in an error message (with\n> and without `--force` flag):\n\nWould checkout out a state where the submodule still exists\n(such as HEAD) and then running \"git submodule update --init\"\nfix it instead?\n\n>\n> $ git submodule add git@github.com:-----------/wiki-public.git public\n> --force\n> A git directory for 'public' is found locally with remote(s):\n> origin git@github.com:-----------/wiki-public.git\n> If you want to reuse this local git directory instead of cloning again from\n> git@github.com:-----------/wiki-public.git\n> use the '--force' option. If the local git directory is not the correct repo\n> or you are unsure what this means choose another name with the '--name'\n> option.\n>\n> Therefore, it's in a undefined, broken state.\n>\n>\n> Another bug I've got by testing upper line:\n> * --force will be used as folder name * when used in `git submodule add\n> git@github.com:someone/some.git --force`:\n\nYes, the order of arguments is important, the options\n(such as --force) goes before the URL and path.\n\n> /usr/libexec/git-core/git-submodule: line 273: cd: --: invalid option\n> cd: usage: cd [-L|-P] [dir]\n> Unable to checkout submodule '--force'\n\nGah! We'd need to escape the path after the options,\ni.e. cd -- --force\nwould cd into that directory, but I am unsure if that\nis accepted by all shells.\n\nThanks,\nStefan\n"},{"id":"360982","messageId":"20181019205837.GA84932@aiede.svl.corp.google.com","threadId":"49615","inReplyTo":"CAGZ79kZ4NSCknQTkpurdj_CzD+XvFzdaz1bdxPTFizjwV8=rpA@mail.gmail.com","subject":"Re: Bug with \"git mv\" and submodules, and with \"git submodule add something --force\"","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-10-19T20:58:37Z","receivedAt":"2018-10-19T20:58:41Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n> Maybe for now we can do with just an update of the documentation/bugs\n> section and say we cannot move files in and out of submodules?\n\nI think we have some existing logic to prevent \"git add\"-ing a file\nwithin a submodule to the superproject, for example.\n\nSo \"git mv\" should learn the same trick. And perhaps the trick needs\nto be moved down a layer (e.g. into the index API). Hints?\n\nThanks,\nJonathan\n"},{"id":"361125","messageId":"xmqqd0s2y9o9.fsf@gitster-ct.c.googlers.com","threadId":"49615","inReplyTo":"20181019205837.GA84932@aiede.svl.corp.google.com","subject":"Re: Bug with \"git mv\" and submodules, and with \"git submodule add something --force\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-22T02:52:54Z","receivedAt":"2018-10-22T02:53:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Stefan Beller wrote:\n>\n>> Maybe for now we can do with just an update of the documentation/bugs\n>> section and say we cannot move files in and out of submodules?\n>\n> I think we have some existing logic to prevent \"git add\"-ing a file\n> within a submodule to the superproject, for example.\n\nThere is die_path_inside_submodule() that sanity-checks the pathspec\nand rejects. But I think that is done primarily to give an error\nmessage and not strictly necesary for correctness.\n\nThe real safety of \"git add\" is its call to dir.c::fill_directory();\nit collects untracked paths that match the pathspec so that they can\nbe added as new paths, but because it won't cross the module\nboundary, you won't get such a path in the index to begin with.\n\n> So \"git mv\" should learn the same trick. And perhaps the trick needs\n> to be moved down a layer (e.g. into the index API). Hints?\n\nYou would want to be able to remove a submodule and replace it with\na directory, but you can probably do it in two steps, i.e.\n\n\tgit reset --hard\n\tgit rm --cached sha1collisiondetection\n\techo Now a regular dir >sha1collisiondetection/READ.ME\n\tfind sha1collisiondetection ! -type d -print0 | \n\tgit update-index --add --stdin -z\n\nSo from that point of view, forbidding (starting from the same state\nof our project) this sequence:\n\n\tgit reset --hard\n\techo Now a regular dir >sha1collisiondetection/READ.ME\n\tfind sha1collisiondetection ! -type d -print0 | \n\tgit update-index --add --remove --stdin -z\n\nthat would nuke the submodule and replace it with a directory within\nwhich there are files would be OK. Making the latter's default\nrejection overridable with ADD_CACHE_OK_TO_REPLACE would also be\nfine.\n\n"},{"id":"361210","messageId":"20181022215236.72238-1-sbeller@google.com","threadId":"49615","inReplyTo":"xmqqd0s2y9o9.fsf@gitster-ct.c.googlers.com","subject":"Bug with \"git mv\" and submodules, and with \"git submodule add something --force\"","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-22T21:52:36Z","receivedAt":"2018-10-22T21:53:08Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Oct 21, 2018 at 7:52 PM Junio C Hamano wrote:\n>\n> Jonathan Nieder writes:\n>\n> > Stefan Beller wrote:\n> >\n> >> Maybe for now we can do with just an update of the documentation/bugs\n> >> section and say we cannot move files in and out of submodules?\n> >\n> > I think we have some existing logic to prevent \"git add\"-ing a file\n> > within a submodule to the superproject, for example.\n>\n> There is die_path_inside_submodule() that sanity-checks the pathspec\n> and rejects. But I think that is done primarily to give an error\n> message and not strictly necesary for correctness.\n\nc08397e3aa (pathspec: remove PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE\nflag, 2017-05-11) seems to be relevant here, as that factors out the\nwarning.\n\n> The real safety of \"git add\" is its call to dir.c::fill_directory();\n> it collects untracked paths that match the pathspec so that they can\n> be added as new paths, but because it won't cross the module\n> boundary, you won't get such a path in the index to begin with.\n\nIt would not cross the boundary and fail silently as it would\ntreat a path into the submodule as a no-op.\n\n> > So \"git mv\" should learn the same trick. And perhaps the trick needs\n> > to be moved down a layer (e.g. into the index API). Hints?\n\nYeah, I think we'd want to teach git-mv about that trick.\n\nUnfortunately git-mv is one of the last remainders of not\nproperly using pathspecs, and die_path_inside_submodule\nexpects a pathspec. :-/\n\n> You would want to be able to remove a submodule and replace it with\n> a directory, but you can probably do it in two steps, i.e.\n>\n> git reset --hard\n> git rm --cached sha1collisiondetection\n> echo Now a regular dir >sha1collisiondetection/READ.ME\n> find sha1collisiondetection ! -type d -print0 |\n> git update-index --add --stdin -z\n\n \"Ignoring path sha1collisiondetection/.git\"\n\nNice!\n\n>\n> So from that point of view, forbidding (starting from the same state\n> of our project) this sequence:\n>\n> git reset --hard\n> echo Now a regular dir >sha1collisiondetection/READ.ME\n> find sha1collisiondetection ! -type d -print0 |\n> git update-index --add --remove --stdin -z\n>\n> that would nuke the submodule and replace it with a directory within\n> which there are files would be OK. Making the latter's default\n> rejection overridable with ADD_CACHE_OK_TO_REPLACE would also be\n> fine.\n\nI am having trouble of relating these commands to the original git-mv\nacross submodule boundaries.\n\nMoving files from the submodule out to the superproject, seems\nto fail properly, though having a less-than-optimal error message:\n\n$ git mv sha1collisiondetection/LICENSE.txt .\nfatal: not under version control, source=sha1collisiondetection/LICENSE.txt, destination=LICENSE.txt\n\nand moving things inside was the original report, below is a proof of concept\nthat would yield\n\n./git mv -v cache.h sha1collisiondetection/\nfatal: moving across submodule boundaries not supported, source=cache.h, destination=sha1collisiondetection/cache.h\n\n--8<--\nSubject: [WIP/PATCH] builtin/mv.c: disallow moving across submodule boundaries\n\nSigned-off-by: Stefan Beller \n---\n builtin/mv.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 80bb967a63..9ec4b2f0a3 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -172,7 +172,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t/* Checking */\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *src = source[i], *dst = destination[i];\n-\t\tint length, src_is_dir;\n+\t\tint length, src_is_dir, pos;\n \t\tconst char *bad = NULL;\n \n \t\tif (show_only)\n@@ -243,6 +243,13 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\telse\n \t\t\tstring_list_insert(&src_for_dst, dst);\n \n+\t\tpos = cache_name_pos(dst, strlen(dst));\n+\t\tif (pos < 0) {\n+\t\t\tpos = -(pos + 1);\n+\t\t\tif (!S_ISGITLINK(active_cache[pos]->ce_mode))\n+\t\t\t\tbad = _(\"moving across submodule boundaries not supported\");\n+\t\t}\n+\n \t\tif (!bad)\n \t\t\tcontinue;\n \t\tif (!ignore_errors)\n-- \n2.19.0\n\n"},{"id":"361373","messageId":"xmqq36svolrz.fsf@gitster-ct.c.googlers.com","threadId":"49615","inReplyTo":"20181022215236.72238-1-sbeller@google.com","subject":"Re: Bug with \"git mv\" and submodules, and with \"git submodule add something --force\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-24T07:18:24Z","receivedAt":"2018-10-24T07:18:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> You would want to be able to remove a submodule and replace it with\n>> a directory, but you can probably do it in two steps, i.e.\n>>\n>> git reset --hard\n>> git rm --cached sha1collisiondetection\n>> echo Now a regular dir >sha1collisiondetection/READ.ME\n>> find sha1collisiondetection ! -type d -print0 |\n>> git update-index --add --stdin -z\n>\n> \"Ignoring path sha1collisiondetection/.git\"\n>\n> Nice!\n\nThere actually is another possible outcome that anybody following\nalong must be aware of and be careful about: not even .git directory\nexist there, i.e. it is possible that the submodule has never been\ninit'ed.\n\nSo, it is not that nice X-<.\n"}]}