{"thread":{"id":"36557","subject":"Problem: staging of an alternative repository","startedAt":"2014-04-30T21:22:44Z","lastAt":"2014-05-19T10:05:44Z","messageCount":7,"participants":["Pasha Bolokhov","Jonathan Nieder","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"240372","messageId":"CAKpPgveXqraM4bXb499mJm5Ls+EinihfEJ6VgOFikC0_Qp8iTA@mail.gmail.com","threadId":"36557","inReplyTo":null,"subject":"Problem: staging of an alternative repository","fromName":"Pasha Bolokhov","fromEmail":"pasha.bolokhov@gmail.com","sentAt":"2014-04-30T21:22:44Z","receivedAt":"2014-04-30T21:22:44Z","isPatch":false,"sender":{"key":"pasha.bolokhov@gmail.com","avatar":null},"body":" Hi\n\n It turns out Git treats the directory '.git' differently enough\nfrom everything else. That may be ok, but here's one place where I\nencountered an unpleasant (and imho unexpected) behaviour:\n\n if you supply a different repository base name, say, '.git_new',\nby either setting GIT_DIR or using the '--git-dir' option, Git 'add'\nwill not make any exception for it and think of it as a new (weird)\ndirectory. In particular, 'git add -A' with a consequent commit will\nadd this repository into itself with all its guts.\n\n Now I know, the '--git-dir' option may usually be meant to use\nwhen the repository is somewhere outside of the work tree, and such a\nproblem would not arise. And even if it is inside, sure enough, you\ncan add this '.git_new' to the ignores or excludes. But is this really\nwhat you expect?\n\n I come forward to offer my own will to fix this behaviour (which\nis rooted in 'dir.c'). However there are uncertainties, and I'm asking\nfor an opinion.\n\n Apparently, the assumption that the repository is in '.git' has\npropagated far enough. In particular, every '.git' within the working\ntree seems to be ignored for the purpose of staging. Is this a\nconsistent behaviour? And, perhaps there are a million more places\nwhere the name '.git' is hard-coded, and it might be reasonable to\nquestion the legitimacy for that. Or, in contrast, to what degree or\ndepth (in the source code) does one *expect* Git to rename all its\nhard-coded '.git's into '.gut's when a \"GIT_DIR=.gut\" is supplied?\n\n cheers\nPavel\n"},{"id":"240374","messageId":"20140430213545.GV9218@google.com","threadId":"36557","inReplyTo":"CAKpPgveXqraM4bXb499mJm5Ls+EinihfEJ6VgOFikC0_Qp8iTA@mail.gmail.com","subject":"Re: Problem: staging of an alternative repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-04-30T21:35:45Z","receivedAt":"2014-04-30T21:35:45Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Pavel,\n\nPasha Bolokhov wrote:\n\n> It turns out Git treats the directory '.git' differently enough\n> from everything else. That may be ok,\n\nYeah, it's intended.\n\n[...]\n> if you supply a different repository base name, say, '.git_new',\n> by either setting GIT_DIR or using the '--git-dir' option, Git 'add'\n> will not make any exception for it and think of it as a new (weird)\n> directory.\n\nYep, a git repository metadata directory named .git_new is not special\nin any way and you can use \"git add\" to track it if you want (for\nexample to add a testcase).\n\n[...]\n> Now I know, the '--git-dir' option may usually be meant to use\n> when the repository is somewhere outside of the work tree, and such a\n> problem would not arise. And even if it is inside, sure enough, you\n> can add this '.git_new' to the ignores or excludes. But is this really\n> what you expect?\n\nI think it's more that it never came up. Excluding the current\n$GIT_DIR from what \"git add\" can add (on top of the current rule of\nexcluding all instances of \".git\") seems like a sensible change,\nassuming it can be done without hurting the code too much. ;-)\n\nBut as you note, you are not using $GIT_DIR the way it was intended to\nbe used.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"240515","messageId":"CAKpPgvcbYf4nJPqYECOe3Kkgk-UnYrWePYkz76NU+-ok_6AqUQ@mail.gmail.com","threadId":"36557","inReplyTo":"20140430213545.GV9218@google.com","subject":"Re: Problem: staging of an alternative repository","fromName":"Pasha Bolokhov","fromEmail":"pasha.bolokhov@gmail.com","sentAt":"2014-05-02T05:23:25Z","receivedAt":"2014-05-02T05:23:25Z","isPatch":false,"sender":{"key":"pasha.bolokhov@gmail.com","avatar":null},"body":" Hi Jonathan\n\nThanks for the answers\n\n> I think it's more that it never came up. Excluding the current\n> $GIT_DIR from what \"git add\" can add (on top of the current rule of\n> excluding all instances of \".git\") seems like a sensible change,\n> assuming it can be done without hurting the code too much. ;-)\n\nI did notice it to have come up in the forums in some related but\nnon-transparent ways.\n\nAnyway, this can hopefully be easily fixed and I can look into it. My\nunderstanding is, that unlike the special treatment of \".git\", the\nalternative repository (call it '.gut') should only be \"ignored\" at\nthe *top* of the work tree and not anywhere deeper inside. And of\ncourse, the special treatment (that is, the ignoring) of \".git\" should\nbe kept as it is. Am I right?\n\n regards\nPavel\n"},{"id":"240516","messageId":"CACsJy8C4bO_ZB-DMOkbJc9cJd_LmR6z0RZ1UZB3wKPEX005xmA@mail.gmail.com","threadId":"36557","inReplyTo":"20140430213545.GV9218@google.com","subject":"Re: Problem: staging of an alternative repository","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-02T06:20:55Z","receivedAt":"2014-05-02T06:20:55Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, May 1, 2014 at 4:35 AM, Jonathan Nieder wrote:\n>> Now I know, the '--git-dir' option may usually be meant to use\n>> when the repository is somewhere outside of the work tree, and such a\n>> problem would not arise. And even if it is inside, sure enough, you\n>> can add this '.git_new' to the ignores or excludes. But is this really\n>> what you expect?\n>\n> I think it's more that it never came up. Excluding the current\n> $GIT_DIR from what \"git add\" can add (on top of the current rule of\n> excluding all instances of \".git\") seems like a sensible change,\n> assuming it can be done without hurting the code too much. ;-)\n\nI think it came up before. Changes could be very messy (but I did not\ncheck carefully) because right now we just compare $(basename $path)\nwith \".git\", one path component, simple and easy. Checking against\n$GIT_DIR means all path components. You also have to deal with\nrelative and absolute paths and symlinks in some path components. You\nmay also need to think if submodule detection code (checking \".git\"\nagain) is impacted. On top of that, read_directory() code is already\nmessy (or at least scary to me) with all kinds of shortcuts we have\nadded over the years. A simpler solution may be ignoring all\ndirectories whose last component is \"$GIT_DIR_NAME\" (e.g.\nGIT_DIR_NAME=.git_new).\n-- \nDuy\n"},{"id":"240959","messageId":"CAKpPgvcvOeneqpidNAbp-8gkS3dpLfMEdTjb-L7o-VSadKHP9A@mail.gmail.com","threadId":"36557","inReplyTo":"CACsJy8C4bO_ZB-DMOkbJc9cJd_LmR6z0RZ1UZB3wKPEX005xmA@mail.gmail.com","subject":"Re: Problem: staging of an alternative repository","fromName":"Pasha Bolokhov","fromEmail":"pasha.bolokhov@gmail.com","sentAt":"2014-05-07T20:51:57Z","receivedAt":"2014-05-07T20:51:57Z","isPatch":false,"sender":{"key":"pasha.bolokhov@gmail.com","avatar":null},"body":" Hi,\n\nI've looked more attentively, here are my observations and the\nresulting suggestions:\n\n- Suggest only to check *string-wise* the given \"path\" against\n$GIT_DIR. Both or one of them may be relative paths (but comparison\nbest be performed when converted to absolute paths). That's the only\nsolution which will give predictable results, but does not handle\nsymlinks: if you have symlinks (so that both differing paths actually\npoint to the same location) the comparison will return failure. Only\nif you have an exact string-wise match do we ignore the \"path\"\n\n- With this way of comparison, only the root \".git_new\" will be\nignored. Submodules will likely contain the usual \".git\". Since the\ncode normally ignores \".git\" anyway, and I do not intend to change\nthat, I don't see how submodule detection can be affected\n\n- The problem of resolving symlinks is in a very general case\ninsolvable (e.g. imagine one of the symlinks points to another\nfilesystem which may be up or down depending on the day of week - it's\neasy to plot a scenario where symlinks will resolve (or even fail to\nresolve) differently at different runs)\n\n- Even if it was solvable, the current implementation of handling\n\".git\" certainly does not check any symlinks:\n $ mv -i .git .metadata\n $ ln -s .metadata .git\n Then certainly \"git add -A\" will grab all \".metadata\" and store into itself\n\n Please let me know what you think. Again, I can try to carefully do\nthis suggestion\n\nPavel\n\n\nOn Thu, May 1, 2014 at 11:20 PM, Duy Nguyen wrote:\n> On Thu, May 1, 2014 at 4:35 AM, Jonathan Nieder wrote:\n>>> Now I know, the '--git-dir' option may usually be meant to use\n>>> when the repository is somewhere outside of the work tree, and such a\n>>> problem would not arise. And even if it is inside, sure enough, you\n>>> can add this '.git_new' to the ignores or excludes. But is this really\n>>> what you expect?\n>>\n>> I think it's more that it never came up. Excluding the current\n>> $GIT_DIR from what \"git add\" can add (on top of the current rule of\n>> excluding all instances of \".git\") seems like a sensible change,\n>> assuming it can be done without hurting the code too much. ;-)\n>\n> I think it came up before. Changes could be very messy (but I did not\n> check carefully) because right now we just compare $(basename $path)\n> with \".git\", one path component, simple and easy. Checking against\n> $GIT_DIR means all path components. You also have to deal with\n> relative and absolute paths and symlinks in some path components. You\n> may also need to think if submodule detection code (checking \".git\"\n> again) is impacted. On top of that, read_directory() code is already\n> messy (or at least scary to me) with all kinds of shortcuts we have\n> added over the years. A simpler solution may be ignoring all\n> directories whose last component is \"$GIT_DIR_NAME\" (e.g.\n> GIT_DIR_NAME=.git_new).\n> --\n> Duy\n"},{"id":"242069","messageId":"CAKpPgvdC3VEHoW4eqmjoPtr4Duh8hkaeHL-JehaVMVUAdGhLAw@mail.gmail.com","threadId":"36557","inReplyTo":"CACsJy8C4bO_ZB-DMOkbJc9cJd_LmR6z0RZ1UZB3wKPEX005xmA@mail.gmail.com","subject":"Re: Problem: staging of an alternative repository","fromName":"Pasha Bolokhov","fromEmail":"pasha.bolokhov@gmail.com","sentAt":"2014-05-17T16:31:33Z","receivedAt":"2014-05-17T16:31:33Z","isPatch":false,"sender":{"key":"pasha.bolokhov@gmail.com","avatar":null},"body":" Hi again\n\n I've come up with a fix for this. It's just two and a half lines,\nand required more studying the code than typing.\nA lot of path-processing work has been implemented in \"abspath.c\" and\n\"dir.c\", including the symlinks and checking whether one path is a\nsubdirectory of another. I just added an \"exclude\" for GITDIR without\ntouching anything else.\n\n Now the best place to add that exclude would probably be \"git.c\",\nright after the option \"--git-dir\" is processed. But this is not\nactually the place where excludes are initialized or used any how.\nSince initialization of excludes is done more or less individually by\neach command concerned about them, the most \"centralized\" place\nhappens to be dir:setup_standard_excludes(), and that's where I did\nit. One of the (side?) effects is that the excludes work in such a way\nthat any directory named \".metadata\" in the directory tree will be\nignored once \"-git-dir=.metadata\" has been given\n\n Now if you guys don't see anything against this, I would shoot out a patch?\n\n regards\nPasha\n\n\nOn Thu, May 1, 2014 at 11:20 PM, Duy Nguyen wrote:\n> On Thu, May 1, 2014 at 4:35 AM, Jonathan Nieder wrote:\n>>> Now I know, the '--git-dir' option may usually be meant to use\n>>> when the repository is somewhere outside of the work tree, and such a\n>>> problem would not arise. And even if it is inside, sure enough, you\n>>> can add this '.git_new' to the ignores or excludes. But is this really\n>>> what you expect?\n>>\n>> I think it's more that it never came up. Excluding the current\n>> $GIT_DIR from what \"git add\" can add (on top of the current rule of\n>> excluding all instances of \".git\") seems like a sensible change,\n>> assuming it can be done without hurting the code too much. ;-)\n>\n> I think it came up before. Changes could be very messy (but I did not\n> check carefully) because right now we just compare $(basename $path)\n> with \".git\", one path component, simple and easy. Checking against\n> $GIT_DIR means all path components. You also have to deal with\n> relative and absolute paths and symlinks in some path components. You\n> may also need to think if submodule detection code (checking \".git\"\n> again) is impacted. On top of that, read_directory() code is already\n> messy (or at least scary to me) with all kinds of shortcuts we have\n> added over the years. A simpler solution may be ignoring all\n> directories whose last component is \"$GIT_DIR_NAME\" (e.g.\n> GIT_DIR_NAME=.git_new).\n> --\n> Duy\n"},{"id":"242152","messageId":"CACsJy8BAYgjeKzy8Rssz2tNzeCRSAg1UO+XK1JxuXyJh+uZMoQ@mail.gmail.com","threadId":"36557","inReplyTo":"CAKpPgvdC3VEHoW4eqmjoPtr4Duh8hkaeHL-JehaVMVUAdGhLAw@mail.gmail.com","subject":"Re: Problem: staging of an alternative repository","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-19T10:05:44Z","receivedAt":"2014-05-19T10:05:44Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, May 17, 2014 at 11:31 PM, Pasha Bolokhov\n wrote:\n> Now if you guys don't see anything against this, I would shoot out a patch?\n>\n\nIf you have written the patch already, I see no harm in sending it\nhere. I'm concerned about the perfomance impact on this code, which is\nalready slow when the repo is large. But we can benchmark it later.\n-- \nDuy\n"}]}