{"thread":{"id":"7720","subject":"[BUG] git-new-workdir doesn't understand packed refs","startedAt":"2007-04-17T16:17:20Z","lastAt":"2007-04-21T20:05:50Z","messageCount":22,"participants":["Peter Baumann","Julian Phillips","Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"39670","messageId":"20070417161720.GA3930@xp.machine.xx","threadId":"7720","inReplyTo":null,"subject":"[BUG] git-new-workdir doesn't understand packed refs","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-17T16:17:20Z","receivedAt":"2007-04-17T16:17:20Z","isPatch":false,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"running git-gc or git-gc --prune isn't save because e.g. all the tags\nare packed and .git/packed-refs isn't shared on the several workdirs.\n\nThis has caused me to lose some tags, but being lucky, I could find those in\nthe backup.\n\nGreetings,\n Peter\n"},{"id":"39713","messageId":"Pine.LNX.4.64.0704172253140.14155@beast.quantumfyre.co.uk","threadId":"7720","inReplyTo":"20070417161720.GA3930@xp.machine.xx","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-17T21:55:17Z","receivedAt":"2007-04-17T21:55:17Z","isPatch":false,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Tue, 17 Apr 2007, Peter Baumann wrote:\n\n> running git-gc or git-gc --prune isn't save because e.g. all the tags\n> are packed and .git/packed-refs isn't shared on the several workdirs.\n\nDo you mean that the link wasn't created? Or that the link was removed \nand replaced with a file when you ran gc from a workdir?\n\n-- \nJulian\n\n ---\nMy mother is a fish.\n- William Faulkner\n"},{"id":"39741","messageId":"20070418055215.GA32634@xp.machine.xx","threadId":"7720","inReplyTo":"Pine.LNX.4.64.0704172253140.14155@beast.quantumfyre.co.uk","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-18T05:52:15Z","receivedAt":"2007-04-18T05:52:15Z","isPatch":false,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Tue, Apr 17, 2007 at 10:55:17PM +0100, Julian Phillips wrote:\n> On Tue, 17 Apr 2007, Peter Baumann wrote:\n> \n> > running git-gc or git-gc --prune isn't save because e.g. all the tags\n> > are packed and .git/packed-refs isn't shared on the several workdirs.\n> \n> Do you mean that the link wasn't created? Or that the link was removed and\n> replaced with a file when you ran gc from a workdir?\n> \n\nThe problem is, when I created the new workdir, I don't have a file\n.git/packed-refs, so a new workdir was created with a dangling symlink,\ne.g. workdir/.git/packed-refs -> repo/.git/packed-refs (but the last one\ndoesn't exist). As it seems, git gc removes the dangling symlink and\nreplaces it with a file.\n\nSteps to reproduce (written in this mail; after /usr/bin/script gave me an\noutput whith color coded text *GRR* in ASCII squences):\n\n\tmkdir a && cd a && git init\n\techo 1 > file.txt\n\tgit add file.txt\n\tgit commit -m \"file added\"\n\tgit tag v0\n\tcd ..\n\n\tgit-new-workdir a b\n\tcd b && git-gc\n\n\nOh. Wait. Just forget that theorie about dangling symlink. git-gc replaces\nthe symlink in a new workdir with a file. Just confirmed that.\n\nSo it isn't save to run git-gc in a workdir.\n\n-Peter\n"},{"id":"39744","messageId":"Pine.LNX.4.64.0704180822270.4684@beast.quantumfyre.co.uk","threadId":"7720","inReplyTo":"20070418055215.GA32634@xp.machine.xx","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-18T07:26:11Z","receivedAt":"2007-04-18T07:26:11Z","isPatch":false,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Wed, 18 Apr 2007, Peter Baumann wrote:\n\n> On Tue, Apr 17, 2007 at 10:55:17PM +0100, Julian Phillips wrote:\n>> On Tue, 17 Apr 2007, Peter Baumann wrote:\n>>\n>>> running git-gc or git-gc --prune isn't save because e.g. all the tags\n>>> are packed and .git/packed-refs isn't shared on the several workdirs.\n>>\n>> Do you mean that the link wasn't created? Or that the link was removed and\n>> replaced with a file when you ran gc from a workdir?\n>>\n>\n> The problem is, when I created the new workdir, I don't have a file\n> .git/packed-refs, so a new workdir was created with a dangling symlink,\n> e.g. workdir/.git/packed-refs -> repo/.git/packed-refs (but the last one\n> doesn't exist). As it seems, git gc removes the dangling symlink and\n> replaces it with a file.\n>\n> Steps to reproduce (written in this mail; after /usr/bin/script gave me an\n> output whith color coded text *GRR* in ASCII squences):\n>\n> \tmkdir a && cd a && git init\n> \techo 1 > file.txt\n> \tgit add file.txt\n> \tgit commit -m \"file added\"\n> \tgit tag v0\n> \tcd ..\n>\n> \tgit-new-workdir a b\n> \tcd b && git-gc\n>\n>\n> Oh. Wait. Just forget that theorie about dangling symlink. git-gc replaces\n> the symlink in a new workdir with a file. Just confirmed that.\n>\n> So it isn't save to run git-gc in a workdir.\n\nTrue. I don't think that it would be a good idea to run any purely \nrepository type commands in a workdir.\n\n-- \nJulian\n\n ---\nYou know you're using the computer too much when:\nyou call a doctor a \"virus scanner\"\n \t-- Lews_Therin\n"},{"id":"39745","messageId":"7v7isajfl1.fsf@assigned-by-dhcp.cox.net","threadId":"7720","inReplyTo":"20070418055215.GA32634@xp.machine.xx","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T07:40:10Z","receivedAt":"2007-04-18T07:40:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Baumann writes:\n\n> The problem is, when I created the new workdir, I don't have a file\n> .git/packed-refs, so a new workdir was created with a dangling symlink,\n> e.g. workdir/.git/packed-refs -> repo/.git/packed-refs (but the last one\n> doesn't exist). As it seems, git gc removes the dangling symlink and\n> replaces it with a file.\n\nYes, packed-refs file is creat-to-temp-and-then-rename, and we\nwill lose the sharing if it is run in the symlink-shared work\ntree.\n\nWe can do one of two things. I am not sure which one is better.\n\n (0) The effect of 'git gc' by definition in the symlink-shared\n work tree should be the same as in the original repository\n as the former is to share all the refspace and object\n database. So we _could_ declare that running 'git gc' in\n symlink-shared work tree is insane and educate people to\n run that in the original repository. This is _not_ doing\n anything.\n\n (1) We could by convention declare a worktree whose .git/refs\n is a symlink, and have git-gc and friends check for it, and\n either refuse to run or automatically chdir and run there.\n\n If we were to do this, we probably should check more than\n just .git/refs but some other symlinks under .git/ as well.\n\n (2) We could dereference .git/packed-refs, when it is a\n symlink, by hand, just like we dereference a symlink HEAD\n by hand (see resolve_ref() in refs.c), and run the\n creat-to-temp-and-then-rename sequence to update the real\n file that is pointed at by it.\n\n\n \n"},{"id":"39749","messageId":"20070418081122.GB32634@xp.machine.xx","threadId":"7720","inReplyTo":"7v7isajfl1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-18T08:11:22Z","receivedAt":"2007-04-18T08:11:22Z","isPatch":false,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Wed, Apr 18, 2007 at 12:40:10AM -0700, Junio C Hamano wrote:\n> Peter Baumann writes:\n> \n> > The problem is, when I created the new workdir, I don't have a file\n> > .git/packed-refs, so a new workdir was created with a dangling symlink,\n> > e.g. workdir/.git/packed-refs -> repo/.git/packed-refs (but the last one\n> > doesn't exist). As it seems, git gc removes the dangling symlink and\n> > replaces it with a file.\n> \n> Yes, packed-refs file is creat-to-temp-and-then-rename, and we\n> will lose the sharing if it is run in the symlink-shared work\n> tree.\n> \n> We can do one of two things. I am not sure which one is better.\n> \n> (0) The effect of 'git gc' by definition in the symlink-shared\n> work tree should be the same as in the original repository\n> as the former is to share all the refspace and object\n> database. So we _could_ declare that running 'git gc' in\n> symlink-shared work tree is insane and educate people to\n> run that in the original repository. This is _not_ doing\n> anything.\n> \n> (1) We could by convention declare a worktree whose .git/refs\n> is a symlink, and have git-gc and friends check for it, and\n> either refuse to run or automatically chdir and run there.\n> \n> If we were to do this, we probably should check more than\n> just .git/refs but some other symlinks under .git/ as well.\n> \n> (2) We could dereference .git/packed-refs, when it is a\n> symlink, by hand, just like we dereference a symlink HEAD\n> by hand (see resolve_ref() in refs.c), and run the\n> creat-to-temp-and-then-rename sequence to update the real\n> file that is pointed at by it.\n>\n\nIts not all the clear which one is the best, but (2) sounds as the most\npromosing aproach. Hopefully, I'll have time to cook up a patch this\nevening.\n\n-Peter\n"},{"id":"39759","messageId":"20070418102823.GA5586@xp.machine.xx","threadId":"7720","inReplyTo":"7v7isajfl1.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] pack-refs: dereference .git/packed-refs if it is a symlink","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-18T10:28:23Z","receivedAt":"2007-04-18T10:28:23Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"git-new-workdir creates a new working directory where everything\nnecessary, including .git/packed-refs, is symlinked to your master repo.\nBut git-pack-refs breaks the symlink, so you could accidentally loose some\nrefs. This fixes it to first dereference .git/packed-refs if it is a\nsymlink.\n\nSigned-off-by: Peter Baumann \n---\n builtin-pack-refs.c | 15 ++++++++++++++-\n 1 files changed, 14 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-pack-refs.c b/builtin-pack-refs.c\nindex d080e30..afa9b5a 100644\n--- a/builtin-pack-refs.c\n+++ b/builtin-pack-refs.c\n@@ -89,6 +89,8 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n {\n \tint fd, i;\n \tstruct pack_refs_cb_data cbdata;\n+\tstruct stat st;\n+\tchar *ref_file_name;\n \n \tmemset(&cbdata, 0, sizeof(cbdata));\n \n@@ -113,7 +115,18 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n \tif (i != argc)\n \t\tusage(builtin_pack_refs_usage);\n \n-\tfd = hold_lock_file_for_update(&packed, git_path(\"packed-refs\"), 1);\n+\tref_file_name = git_path(\"packed-refs\");\n+\tif (!lstat(ref_file_name, &st) && S_ISLNK(st.st_mode)) {\n+\t\tchar *buf = xmalloc(st.st_size + 1);\n+\t\tif (readlink(ref_file_name, buf, st.st_size + 1) != st.st_size) {\n+\t\t\tfree(buf);\n+\t\t\tdie(\"readlink failed\\n\");\n+\t\t}\n+\t\tbuf[st.st_size] = '\\0';\n+\t\tref_file_name = buf;\n+\t}\n+\n+\tfd = hold_lock_file_for_update(&packed, ref_file_name, 1);\n \tcbdata.refs_file = fdopen(fd, \"w\");\n \tif (!cbdata.refs_file)\n \t\tdie(\"unable to create ref-pack file structure (%s)\",\n-- \n1.5.1\n"},{"id":"39770","messageId":"Pine.LNX.4.64.0704181251040.19261@reaper.quantumfyre.co.uk","threadId":"7720","inReplyTo":"20070418081122.GB32634@xp.machine.xx","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-18T11:55:15Z","receivedAt":"2007-04-18T11:55:15Z","isPatch":false,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Wed, 18 Apr 2007, Peter Baumann wrote:\n\n> On Wed, Apr 18, 2007 at 12:40:10AM -0700, Junio C Hamano wrote:\n>>\n>> We can do one of two things. I am not sure which one is better.\n>>\n>> (0) The effect of 'git gc' by definition in the symlink-shared\n>> work tree should be the same as in the original repository\n>> as the former is to share all the refspace and object\n>> database. So we _could_ declare that running 'git gc' in\n>> symlink-shared work tree is insane and educate people to\n>> run that in the original repository. This is _not_ doing\n>> anything.\n>>\n>> (1) We could by convention declare a worktree whose .git/refs\n>> is a symlink, and have git-gc and friends check for it, and\n>> either refuse to run or automatically chdir and run there.\n>>\n>> If we were to do this, we probably should check more than\n>> just .git/refs but some other symlinks under .git/ as well.\n>>\n>> (2) We could dereference .git/packed-refs, when it is a\n>> symlink, by hand, just like we dereference a symlink HEAD\n>> by hand (see resolve_ref() in refs.c), and run the\n>> creat-to-temp-and-then-rename sequence to update the real\n>> file that is pointed at by it.\n>>\n>\n> Its not all the clear which one is the best, but (2) sounds as the most\n> promosing aproach. Hopefully, I'll have time to cook up a patch this\n> evening.\n\nPersonally I think (1) might be slightly better, in the refuse to run \nform. gc is a repository operation, not a working directory one - and by \nrefusing to run in a workdir this is made clear. You could print out a \nmessage that includes the location of the actual repo to be more friendly \nthough.\n\nBut whatever solution you go for, you can't use _any_ workdir that points \nat a repo that is having gc run on, either directly or indirectly, without \nrisky odd behaviour.\n\n-- \nJulian\n\n ---\nQ:\tHow many supply-siders does it take to change a light bulb?\nA:\tNone. The darkness will cause the light bulb to change by itself.\n"},{"id":"39785","messageId":"alpine.LFD.0.98.0704180908360.2828@woody.linux-foundation.org","threadId":"7720","inReplyTo":"20070418102823.GA5586@xp.machine.xx","subject":"Re: [PATCH] pack-refs: dereference .git/packed-refs if it is a symlink","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T16:09:13Z","receivedAt":"2007-04-18T16:09:13Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Apr 2007, Peter Baumann wrote:\n>\n> git-new-workdir creates a new working directory where everything\n> necessary, including .git/packed-refs, is symlinked to your master repo.\n> But git-pack-refs breaks the symlink, so you could accidentally loose some\n> refs. This fixes it to first dereference .git/packed-refs if it is a\n> symlink.\n\nWouldn't it be nicer to instead make \"git gc\" *notice* the fact that we're \nin a workdir, and just \"cd\" to the main git repository instead?\n\n\t\tLinus\n"},{"id":"39788","messageId":"7vfy6xird9.fsf@assigned-by-dhcp.cox.net","threadId":"7720","inReplyTo":"Pine.LNX.4.64.0704181251040.19261@reaper.quantumfyre.co.uk","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T16:23:14Z","receivedAt":"2007-04-18T16:23:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Julian Phillips writes:\n\n>>> (1) We could by convention declare a worktree whose .git/refs\n>>> is a symlink, and have git-gc and friends check for it, and\n>>> either refuse to run or automatically chdir and run there.\n>>>\n>>> If we were to do this, we probably should check more than\n>>> just .git/refs but some other symlinks under .git/ as well.\n>>>\n>>> (2) We could dereference .git/packed-refs, when it is a\n>>> symlink, by hand, just like we dereference a symlink HEAD\n>>> by hand (see resolve_ref() in refs.c), and run the\n>>> creat-to-temp-and-then-rename sequence to update the real\n>>> file that is pointed at by it.\n>>>\n>>\n>> Its not all the clear which one is the best, but (2) sounds as the most\n>> promosing aproach. Hopefully, I'll have time to cook up a patch this\n>> evening.\n>\n> Personally I think (1) might be slightly better, in the refuse to run\n> form. gc is a repository operation, not a working directory one - and\n> by refusing to run in a workdir this is made clear. You could print\n> out a message that includes the location of the actual repo to be more\n> friendly though.\n\nI've seen Peter's patch that attempts to do (2), and I think\nthat probably is a right direction. A worktree that borrows a\nrepository from another worktree is trying to allow you to do\nas many things you would normally do in the original worktree,\nwith a caveat: certain things are less safe and/or confusing and\nyou must know what you are doing if you use such a setting.\n\n> But whatever solution you go for, you can't use _any_ workdir that\n> points at a repo that is having gc run on, either directly or\n> indirectly, without risky odd behaviour.\n\nAnd I think the above is just one of certain things that are\nless safe (one \"confusing\" is that working on the same branch\nwould result in gremlin updates). \n\nThere still is an issue of what to do if the .git/packed-refs is\na symlink to a symlink. Peter's patch does a wrong thing, by\ncreat-then-rename overwriting the symlinked target; at least we\nshould detect that case and error out, I think.\n\nRecursively dereferencing the symbolic link by hand to a limit\nto avoid infinite recursion (error out when we reach the limit)\nwould be a more elaborate solution that probably is the right\nthing to do.\n"},{"id":"39797","messageId":"20070418174350.GB5913@xp.machine.xx","threadId":"7720","inReplyTo":"7vfy6xird9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-18T17:43:50Z","receivedAt":"2007-04-18T17:43:50Z","isPatch":false,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Wed, Apr 18, 2007 at 09:23:14AM -0700, Junio C Hamano wrote:\n> Julian Phillips writes:\n> \n> >>> (1) We could by convention declare a worktree whose .git/refs\n> >>> is a symlink, and have git-gc and friends check for it, and\n> >>> either refuse to run or automatically chdir and run there.\n> >>>\n> >>> If we were to do this, we probably should check more than\n> >>> just .git/refs but some other symlinks under .git/ as well.\n> >>>\n> >>> (2) We could dereference .git/packed-refs, when it is a\n> >>> symlink, by hand, just like we dereference a symlink HEAD\n> >>> by hand (see resolve_ref() in refs.c), and run the\n> >>> creat-to-temp-and-then-rename sequence to update the real\n> >>> file that is pointed at by it.\n> >>>\n> >>\n> >> Its not all the clear which one is the best, but (2) sounds as the most\n> >> promosing aproach. Hopefully, I'll have time to cook up a patch this\n> >> evening.\n> >\n> > Personally I think (1) might be slightly better, in the refuse to run\n> > form. gc is a repository operation, not a working directory one - and\n> > by refusing to run in a workdir this is made clear. You could print\n> > out a message that includes the location of the actual repo to be more\n> > friendly though.\n> \n> I've seen Peter's patch that attempts to do (2), and I think\n> that probably is a right direction. A worktree that borrows a\n> repository from another worktree is trying to allow you to do\n> as many things you would normally do in the original worktree,\n> with a caveat: certain things are less safe and/or confusing and\n> you must know what you are doing if you use such a setting.\n> \n> > But whatever solution you go for, you can't use _any_ workdir that\n> > points at a repo that is having gc run on, either directly or\n> > indirectly, without risky odd behaviour.\n> \n> And I think the above is just one of certain things that are\n> less safe (one \"confusing\" is that working on the same branch\n> would result in gremlin updates). \n> \n> There still is an issue of what to do if the .git/packed-refs is\n> a symlink to a symlink. Peter's patch does a wrong thing, by\n> creat-then-rename overwriting the symlinked target; at least we\n> should detect that case and error out, I think.\n> \n> Recursively dereferencing the symbolic link by hand to a limit\n> to avoid infinite recursion (error out when we reach the limit)\n> would be a more elaborate solution that probably is the right\n> thing to do.\n>\nI thought about the case where packed-refs is a symlink to another symlink\nand then decided that it's not worth to implement this because a workdir\nshould be linked to a _repo_ and not another workdir.\n\n-Peter\n"},{"id":"39798","messageId":"20070418174753.GC5913@xp.machine.xx","threadId":"7720","inReplyTo":"alpine.LFD.0.98.0704180908360.2828@woody.linux-foundation.org","subject":"Re: [PATCH] pack-refs: dereference .git/packed-refs if it is a symlink","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-18T17:47:53Z","receivedAt":"2007-04-18T17:47:53Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Wed, Apr 18, 2007 at 09:09:13AM -0700, Linus Torvalds wrote:\n> \n> \n> On Wed, 18 Apr 2007, Peter Baumann wrote:\n> >\n> > git-new-workdir creates a new working directory where everything\n> > necessary, including .git/packed-refs, is symlinked to your master repo.\n> > But git-pack-refs breaks the symlink, so you could accidentally loose some\n> > refs. This fixes it to first dereference .git/packed-refs if it is a\n> > symlink.\n> \n> Wouldn't it be nicer to instead make \"git gc\" *notice* the fact that we're \n> in a workdir, and just \"cd\" to the main git repository instead?\n> \n> \t\tLinus\n> \n\nDon't think so. Because then all the low level tools aren't aware of this.\nAnd restricting a WorkDir to use only porcelanish commands isn't what I\nwant. And teaching every tool about symklinked workdirs doesn't sound right\nto me.\n\n-Peter\n"},{"id":"39801","messageId":"7vlkgph7i0.fsf@assigned-by-dhcp.cox.net","threadId":"7720","inReplyTo":"20070418174350.GB5913@xp.machine.xx","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T18:17:43Z","receivedAt":"2007-04-18T18:17:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Baumann writes:\n\n> On Wed, Apr 18, 2007 at 09:23:14AM -0700, Junio C Hamano wrote:\n>> \n>> Recursively dereferencing the symbolic link by hand to a limit\n>> to avoid infinite recursion (error out when we reach the limit)\n>> would be a more elaborate solution that probably is the right\n>> thing to do.\n>>\n> I thought about the case where packed-refs is a symlink to another symlink\n> and then decided that it's not worth to implement this because a workdir\n> should be linked to a _repo_ and not another workdir.\n\nThat's incredibly weak, as the initial motivation of this patch\nis that you did not want to say \"you should run gc only in the\n_repo_ not in workdir\".\n"},{"id":"39804","messageId":"20070418183156.GF5913@xp.machine.xx","threadId":"7720","inReplyTo":"7vlkgph7i0.fsf@assigned-by-dhcp.cox.net","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-18T18:31:56Z","receivedAt":"2007-04-18T18:31:56Z","isPatch":false,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Wed, Apr 18, 2007 at 11:17:43AM -0700, Junio C Hamano wrote:\n> Peter Baumann writes:\n> \n> > On Wed, Apr 18, 2007 at 09:23:14AM -0700, Junio C Hamano wrote:\n> >> \n> >> Recursively dereferencing the symbolic link by hand to a limit\n> >> to avoid infinite recursion (error out when we reach the limit)\n> >> would be a more elaborate solution that probably is the right\n> >> thing to do.\n> >>\n> > I thought about the case where packed-refs is a symlink to another symlink\n> > and then decided that it's not worth to implement this because a workdir\n> > should be linked to a _repo_ and not another workdir.\n> \n> That's incredibly weak, as the initial motivation of this patch\n> is that you did not want to say \"you should run gc only in the\n> _repo_ not in workdir\".\n> \n\nYes. That's my motivation and it works right now\n\n\tgit init a\n\t\n\tgit commit -a\n\n\tgit-new-workdir a b \t# allowed\n\tgit-new-workdir a c\t# allowed\n\n\tgit-new-workdir b d\t# NOT ALLOWED\n\nThe user should only create new work dirs which refere to the repo and not\nto another workdir.\n\nBut *iff* thats the only point for keeping my patch out I'll fix it, but\nnot tonight. (Leaving now ...)\n\n-Peter\n"},{"id":"39805","messageId":"7v647th6cv.fsf@assigned-by-dhcp.cox.net","threadId":"7720","inReplyTo":"20070418183156.GF5913@xp.machine.xx","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T18:42:24Z","receivedAt":"2007-04-18T18:42:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Baumann writes:\n\n\n\nGetting more and more annoyed by your stupid Mail-Followup-To...\nI do *not* want to bother Julian with a message that points out\na flaw (in my opinion) in YOUR reasoning but you are forcing me\nto send my message that way, which I have to waste time\ncorrecting every time. Grumble.\n\n\n\n> On Wed, Apr 18, 2007 at 11:17:43AM -0700, Junio C Hamano wrote:\n>> Peter Baumann writes:\n>> ...\n>> > I thought about the case where packed-refs is a symlink to another symlink\n>> > and then decided that it's not worth to implement this because a workdir\n>> > should be linked to a _repo_ and not another workdir.\n>> \n>> That's incredibly weak, as the initial motivation of this patch\n>> is that you did not want to say \"you should run gc only in the\n>> _repo_ not in workdir\".\n>\n> Yes. That's my motivation and it works right now\n>\n> \tgit init a\n> \t\n> \tgit commit -a\n>\n> \tgit-new-workdir a b \t# allowed\n> \tgit-new-workdir a c\t# allowed\n>\n> \tgit-new-workdir b d\t# NOT ALLOWED\n\nBut I do not think you are disallowing it; instead you are\nmaking the same problem appear without telling the user.\n\nAlso, how is the above different from this?\n\n\tgit init a\n cd a ; git gc ; cd ..\t# allowed\n\tgit new-workdir a b\n\tcd b ; git gc ; cd ..\t# NOT ALLOWED\n\nYou are saying \"you should run workdir only in the _repo_ not in\nworkdir\".\n\nAs I already said, certain things work differently between a\nproper repository and a worktree that borrows .git/refs from a\nproper repository, and you always have to know what you are\ndoing when you use such a setup. If your goal is to minimize\nthe difference, I do not think it makes much sense to allow gc\nand not allow new-workdir.\n\nOn the other hand, if we admit that things work differently, I\nthink erroring out gc or pack-refs when we see .git/packed-refs\nis a symbolic link is much simpler, less error prone and easier\nto explain.\n"},{"id":"39806","messageId":"Pine.LNX.4.64.0704181939220.10528@reaper.quantumfyre.co.uk","threadId":"7720","inReplyTo":"20070418183156.GF5913@xp.machine.xx","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-18T18:43:16Z","receivedAt":"2007-04-18T18:43:16Z","isPatch":false,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Wed, 18 Apr 2007, Peter Baumann wrote:\n\n> Yes. That's my motivation and it works right now\n>\n> \tgit init a\n> \t\n> \tgit commit -a\n>\n> \tgit-new-workdir a b \t# allowed\n> \tgit-new-workdir a c\t# allowed\n>\n> \tgit-new-workdir b d\t# NOT ALLOWED\n\nbtw, if you copy git-new-workdir to $GIT_EXEC_PATH then you can do\n\n \tgit new-workdir a b\n\n(and the bash completion script works too. :D)\n\nIt's cunning stuff this git program ...\n\n-- \nJulian\n\n ---\nAll the world's a stage and most of us are desperately unrehearsed.\n \t\t-- Sean O'Casey\n"},{"id":"39827","messageId":"20070418210819.GG5913@xp.machine.xx","threadId":"7720","inReplyTo":"7v647th6cv.fsf@assigned-by-dhcp.cox.net","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-18T21:08:20Z","receivedAt":"2007-04-18T21:08:20Z","isPatch":false,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Wed, Apr 18, 2007 at 11:42:24AM -0700, Junio C Hamano wrote:\n> Peter Baumann writes:\n> \n> \n> \n> Getting more and more annoyed by your stupid Mail-Followup-To...\n> I do *not* want to bother Julian with a message that points out\n> a flaw (in my opinion) in YOUR reasoning but you are forcing me\n> to send my message that way, which I have to waste time\n> correcting every time. Grumble.\n> \n> \n\nHm. Sorry. I don't understand. I'm just pressing 'g' for group reply in\nmutt which should do the right thing; even your mail has a CC to Julian\nset so I _really_ don't understand the problem. I addressed him in the\nbegining because he was the author of git-new-workdir. But please\nforgive me if I'm breaking some netiquette rules but I just started to\nhang out activly on mailinglists ...\n\n> \n> > On Wed, Apr 18, 2007 at 11:17:43AM -0700, Junio C Hamano wrote:\n> >> Peter Baumann writes:\n> >> ...\n> >> > I thought about the case where packed-refs is a symlink to another symlink\n> >> > and then decided that it's not worth to implement this because a workdir\n> >> > should be linked to a _repo_ and not another workdir.\n> >> \n> >> That's incredibly weak, as the initial motivation of this patch\n> >> is that you did not want to say \"you should run gc only in the\n> >> _repo_ not in workdir\".\n> >\n> > Yes. That's my motivation and it works right now\n> >\n> > \tgit init a\n> > \t\n> > \tgit commit -a\n> >\n> > \tgit-new-workdir a b \t# allowed\n> > \tgit-new-workdir a c\t# allowed\n> >\n> > \tgit-new-workdir b d\t# NOT ALLOWED\n> \n> But I do not think you are disallowing it; instead you are\n> making the same problem appear without telling the user.\n> \n> Also, how is the above different from this?\n> \n> \tgit init a\n> cd a ; git gc ; cd ..\t# allowed\n> \tgit new-workdir a b\n> \tcd b ; git gc ; cd ..\t# NOT ALLOWED\n> \n\nSorry, you lost me here. Your above sequence _is_ allowed and that was\njust the point of the patch. I lightly tested it that it does the right\nthing, so perhaps I'm missing something?\n\nWhat isn't allowed is the following:\n\n\tmkdir a; cd a; git-init; cd ..\n\tgit new-workdir a b\n\tcd b; git gc ; cd .. # IS ALLOWED\n\tgit new-workdir b c\n\tcd b; git gc ; cd .. # NOT ALLOWED\n\nBecause now you created a new workdir c which doesn't point to a repo,\nbut only to another _workdir_ b. And only in this case you get a symlink\nchain like this:\n\nc/.git/packed-refs -> b/.git/packed-refs -> a/.git/packed-refs\n\nThis is even dissallowed by the code in git-new-workdir (Sorry, I just\nsaw it now; otherwise I wouldn't spend so much time in arguing this)):\n\n# don't link to a workdir\nif test -L \"$orig_git/.git/config\"\nthen\n die \"\\\"$orig_git\\\" is a working directory only, please specify\" \\\n \"a complete repository.\"\nfi\n\n> You are saying \"you should run workdir only in the _repo_ not in\n> workdir\".\n> \n\nThis sentence doesn't make any sense to me. Did you mean \"you should run\ngc only ...\" ?\n\n> As I already said, certain things work differently between a\n> proper repository and a worktree that borrows .git/refs from a\n> proper repository, and you always have to know what you are\n> doing when you use such a setup. If your goal is to minimize\n> the difference, I do not think it makes much sense to allow gc\n> and not allow new-workdir.\n> \n\nI think you missunderstud me. Hopefully the above explanation clears this\nmissunderstanding. The case I feared (symlink chain of workdirs) is not\nallowed in git-new-workdir from the very begining of this script, so\nthere shouldn't be any problem with the symlink handling in my patch.\n\n> On the other hand, if we admit that things work differently, I\n> think erroring out gc or pack-refs when we see .git/packed-refs\n> is a symbolic link is much simpler, less error prone and easier\n> to explain.\n> \n\nBut with my patch it just works! I really tested it again. The link\nin b/.git/packed-refs -> a/.git/packed-refs (using the example from above)\nisn't broken up and in the new generated packed-refs are stored inside\nthe repo a (as they should).\n\n-Peter\n"},{"id":"39830","messageId":"7v4pndfjym.fsf@assigned-by-dhcp.cox.net","threadId":"7720","inReplyTo":"20070418210819.GG5913@xp.machine.xx","subject":"Re: [BUG] git-new-workdir doesn't understand packed refs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T21:31:29Z","receivedAt":"2007-04-18T21:31:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Baumann writes:\n\n> On Wed, Apr 18, 2007 at 11:42:24AM -0700, Junio C Hamano wrote:\n>> Peter Baumann writes:\n>> \n>> \n>> \n>> Getting more and more annoyed by your stupid Mail-Followup-To...\n>> I do *not* want to bother Julian with a message that points out\n>> a flaw (in my opinion) in YOUR reasoning but you are forcing me\n>> to send my message that way, which I have to waste time\n>> correcting every time. Grumble.\n>> \n>> \n>\n> Hm. Sorry. I don't understand. I'm just pressing 'g' for group reply in\n> mutt which should do the right thing; even your mail has a CC to Julian\n> set so I _really_ don't understand the problem. I addressed him in the\n> begining because he was the author of git-new-workdir. But please\n> forgive me if I'm breaking some netiquette rules but I just started to\n> hang out activly on mailinglists ...\n\nBecause you had Mail-Followup-To: set to point at me and Julian,\nwhen I say \"followup\", by default I get this in my MUA:\n\n To: Julian Phillips \n Cc: git@vger.kernel.org\n Subject: Re: [BUG] git-new-workdir doesn't understand packed refs\n\nMany people prioritize their e-mails depending on where in the\nheader their name appears (ones that have you on Cc: typically\ngets lower priority than the ones addressed specifically to you\nby having you on To: line), and if Julian is doing that, sending\nmy message in which I want to talk to YOU that way would steal\nfrom Julian's time. So as a general netiquette, I end up hand\nfixing it, putting you on To: and demoting Julian to Cc:.\n\nI know why you (or some version of mutt) do so. It saves you\nfrom filtering incoming duplicates (one addressed to you,\nanother addressed to the mailing list you subscribe to), but it\nis a misfeature.\n\nAnyhow...\n\n>> Also, how is the above different from this?\n>> \n>> \tgit init a\n>> cd a ; git gc ; cd ..\t# allowed\n>> \tgit new-workdir a b\n>> \tcd b ; git gc ; cd ..\t# NOT ALLOWED\n>> \n>\n> Sorry, you lost me here. Your above sequence _is_ allowed and that was\n> just the point of the patch. I lightly tested it that it does the right\n> thing, so perhaps I'm missing something?\n\nWhat I was getting at was that if you do not allow new-workdir\nto be done off of a symlinked one, that was like not allowing gc\nin a symlinked one. Both are limitations we _could_ lift. But\nI'd like to take that back, because...\n\n> This is even dissallowed by the code in git-new-workdir (Sorry, I just\n> saw it now; otherwise I wouldn't spend so much time in arguing this)):\n>\n> # don't link to a workdir\n> if test -L \"$orig_git/.git/config\"\n> then\n> die \"\\\"$orig_git\\\" is a working directory only, please specify\" \\\n> \"a complete repository.\"\n> fi\n\n... I missed this one. People cannot make a symlinked one off\nof another by using new-workdir script, which means perhaps\nsomething like this on top of your patch would be safe enough.\nSorry for the confusion.\n\n> But with my patch it just works! I really tested it again. The link\n> in b/.git/packed-refs -> a/.git/packed-refs (using the example from above)\n> isn't broken up and in the new generated packed-refs are stored inside\n> the repo a (as they should).\n\nOh, I never questioned that you made that basic case work. I\nwas worried about not making sure the symlink we are looking at\nreally is the case we are willing to handle, and not erroring\nout if that is not the case, perhaps like the attached patch on\ntop of yours.\n\nAn additional test or two in t/t3210 would be nice to accompany\nthis change.\n\n\ndiff --git a/builtin-pack-refs.c b/builtin-pack-refs.c\nindex afa9b5a..1ce4f55 100644\n--- a/builtin-pack-refs.c\n+++ b/builtin-pack-refs.c\n@@ -123,6 +123,9 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"readlink failed\\n\");\n \t\t}\n \t\tbuf[st.st_size] = '\\0';\n+\t\tif (!lstat(buf, &st) && S_ISLNK(st.st_mode))\n+\t\t\tdie(\"cannot have doubly symlinked packed-refs file: %s\",\n+\t\t\t ref_file_name);\n \t\tref_file_name = buf;\n \t}\n \n"},{"id":"39863","messageId":"20070419053518.GK5913@xp.machine.xx","threadId":"7720","inReplyTo":"7v4pndfjym.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Add test for symlinked .git/packed-refs","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-19T05:35:18Z","receivedAt":"2007-04-19T05:35:18Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"Signed-off-by: Peter Baumann \n---\nOn Wed, Apr 18, 2007 at 02:31:29PM -0700, Junio C Hamano wrote:\n> \n> Oh, I never questioned that you made that basic case work. I\n> was worried about not making sure the symlink we are looking at\n> really is the case we are willing to handle, and not erroring\n> out if that is not the case, perhaps like the attached patch on\n> top of yours.\n> \n> An additional test or two in t/t3210 would be nice to accompany\n> this change.\n> \n\nSomething like this?\n\n t/t3210-pack-refs.sh | 8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh\nindex f0c7e22..bf63954 100755\n--- a/t/t3210-pack-refs.sh\n+++ b/t/t3210-pack-refs.sh\n@@ -105,4 +105,12 @@ test_expect_success 'pack, prune and repack' '\n \tdiff all-of-them again\n '\n \n+test_expect_success \\\n+\t'derefence symlinks for packed-refs' \\\n+\t'mv -f .git/packed-refs .git/real_packed-refs &&\n+\tln -s real_packed-refs .git/packed-refs &&\n+\tgit-tag z &&\n+\tgit-pack-refs --all --prune &&\n+\tdiff .git/real_packed-refs .git/packed-refs'\n+\n test_done\n-- \n1.5.1\n"},{"id":"39865","messageId":"7vabx499u2.fsf@assigned-by-dhcp.cox.net","threadId":"7720","inReplyTo":"20070419053518.GK5913@xp.machine.xx","subject":"Re: [PATCH] Add test for symlinked .git/packed-refs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-19T06:06:45Z","receivedAt":"2007-04-19T06:06:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Baumann writes:\n\n> Signed-off-by: Peter Baumann \n> ---\n> On Wed, Apr 18, 2007 at 02:31:29PM -0700, Junio C Hamano wrote:\n>> \n>> Oh, I never questioned that you made that basic case work. I\n>> was worried about not making sure the symlink we are looking at\n>> really is the case we are willing to handle, and not erroring\n>> out if that is not the case, perhaps like the attached patch on\n>> top of yours.\n>> \n>> An additional test or two in t/t3210 would be nice to accompany\n>> this change.\n>> \n>\n> Something like this?\n\nThat's a good start, but I expected to see at least tests for\ntwo cases: a case in which .git/packed-refs symlink points at an\nactual file (i.e. the original repository has run pack-refs) and\nanother case in which .git/packed-refs symlink is dangling\n(i.e. the original repository hasn't run pack-refs). I\nunderstand that the borrower \"worktree\" can have .git/packed-refs\nsymlink pointing at the repositories .git/packed-refs yet to be\nborn.\n"},{"id":"39993","messageId":"20070420165256.GA14318@xp.machine.xx","threadId":"7720","inReplyTo":"7vabx499u2.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] pack-refs: dereference .git/packed-refs if it is a symlink","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-04-20T16:52:56Z","receivedAt":"2007-04-20T16:52:56Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"git-new-workdir creates a new working directory where everything\nnecessary, including .git/packed-refs, is symlinked to your master repo.\nBut git-pack-refs breaks the symlink, so you could accidentally loose some\nrefs.\n\nThis fixes git-pack-refs to first dereference .git/packed-refs if it is a\nsymlink. While we are it, add some tests to prevent this from happening\nagain.\n\nSigned-off-by: Peter Baumann \n---\nOn Wed, Apr 18, 2007 at 11:06:45PM -0700, Junio C Hamano wrote:\n> Peter Baumann writes:\n> \n> > Signed-off-by: Peter Baumann \n> > ---\n> > On Wed, Apr 18, 2007 at 02:31:29PM -0700, Junio C Hamano wrote:\n> >> An additional test or two in t/t3210 would be nice to accompany\n> >> this change.\n> >> \n> >\n> > Something like this?\n> \n> That's a good start, but I expected to see at least tests for\n> two cases: a case in which .git/packed-refs symlink points at an\n> actual file (i.e. the original repository has run pack-refs) and\n> another case in which .git/packed-refs symlink is dangling\n> (i.e. the original repository hasn't run pack-refs). I\n> understand that the borrower \"worktree\" can have .git/packed-refs\n> symlink pointing at the repositories .git/packed-refs yet to be\n> born.\n> builtin-pack-refs.c | 18 +++++++++++++++++-\n\nAs I couldn't find anything related to this in your repo, I added a test\nfor a danling symklink and integrated your little fix to check for\ndoubly symlinked files for easier handling and to not mess up the\nhistory with all does tiny \"fixes\"\n\nGreetings,\n Peter\n\n t/t3210-pack-refs.sh | 15 +++++++++++++++\n 2 files changed, 32 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-pack-refs.c b/builtin-pack-refs.c\nindex d080e30..1ce4f55 100644\n--- a/builtin-pack-refs.c\n+++ b/builtin-pack-refs.c\n@@ -89,6 +89,8 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n {\n \tint fd, i;\n \tstruct pack_refs_cb_data cbdata;\n+\tstruct stat st;\n+\tchar *ref_file_name;\n \n \tmemset(&cbdata, 0, sizeof(cbdata));\n \n@@ -113,7 +115,21 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)\n \tif (i != argc)\n \t\tusage(builtin_pack_refs_usage);\n \n-\tfd = hold_lock_file_for_update(&packed, git_path(\"packed-refs\"), 1);\n+\tref_file_name = git_path(\"packed-refs\");\n+\tif (!lstat(ref_file_name, &st) && S_ISLNK(st.st_mode)) {\n+\t\tchar *buf = xmalloc(st.st_size + 1);\n+\t\tif (readlink(ref_file_name, buf, st.st_size + 1) != st.st_size) {\n+\t\t\tfree(buf);\n+\t\t\tdie(\"readlink failed\\n\");\n+\t\t}\n+\t\tbuf[st.st_size] = '\\0';\n+\t\tif (!lstat(buf, &st) && S_ISLNK(st.st_mode))\n+\t\t\tdie(\"cannot have doubly symlinked packed-refs file: %s\",\n+\t\t\t ref_file_name);\n+\t\tref_file_name = buf;\n+\t}\n+\n+\tfd = hold_lock_file_for_update(&packed, ref_file_name, 1);\n \tcbdata.refs_file = fdopen(fd, \"w\");\n \tif (!cbdata.refs_file)\n \t\tdie(\"unable to create ref-pack file structure (%s)\",\ndiff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh\nindex f0c7e22..5756304 100755\n--- a/t/t3210-pack-refs.sh\n+++ b/t/t3210-pack-refs.sh\n@@ -105,4 +105,19 @@ test_expect_success 'pack, prune and repack' '\n \tdiff all-of-them again\n '\n \n+test_expect_success \\\n+\t'derefence symlinks for packed-refs' \\\n+\t'mv -f .git/packed-refs .git/real_packed-refs &&\n+\tln -s `pwd`/.git/real_packed-refs .git/packed-refs &&\n+\tgit-tag z &&\n+\tgit-pack-refs --prune &&\n+\tdiff .git/real_packed-refs .git/packed-refs'\n+\n+test_expect_success \\\n+\t'derefence dangling symlinks for packed-refs' \\\n+\t'git branch dangling_symlink &&\n+\trm .git/real_packed-refs\n+\tgit-pack-refs --all --prune &&\n+\tdiff .git/real_packed-refs .git/packed-refs'\n+\n test_done\n-- \n1.5.1\n"},{"id":"40071","messageId":"7vk5w5trvl.fsf@assigned-by-dhcp.cox.net","threadId":"7720","inReplyTo":"20070420165256.GA14318@xp.machine.xx","subject":"Re: [PATCH] pack-refs: dereference .git/packed-refs if it is a symlink","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-21T20:05:50Z","receivedAt":"2007-04-21T20:05:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Baumann writes:\n\n> git-new-workdir creates a new working directory where everything\n> necessary, including .git/packed-refs, is symlinked to your master repo.\n> But git-pack-refs breaks the symlink, so you could accidentally loose some\n> refs.\n>\n> This fixes git-pack-refs to first dereference .git/packed-refs if it is a\n> symlink. While we are it, add some tests to prevent this from happening\n> again.\n\nBecause you are only fixing the case where the worktree is\nborrowing the packed-refs file from a real repository with a\nsymlink trick, and we do not know if somebody had his\npacked-refs as a symlink to some random place for reasons other\nthan creating a lightweight worktree (maybe there was a\nmistake), I am wondering if it makes sense to be more strict\nabout the value we read from readlink().\n\nFor example, if it does not end with \"/packed-refs\", doesn't it\nsuggest that the reason because the symlink is there is\ndifferent from the case you are handling (i.e. it is not a\npacked-refs symlink in a lightweight worktree that points at the\ncorresponding real repository)? I wonder if in such a case we\nwould want to signal an error, instead of overwriting whatever\nreal file the symlink points at. Or is it too strict and\nparanoid? I dunno.\n"}]}