{"thread":{"id":"12716","subject":"[PATCH] git-cvsserver: handle change type T","startedAt":"2008-03-16T19:00:21Z","lastAt":"2008-03-17T19:47:34Z","messageCount":8,"participants":["Paolo Bonzini","Junio C Hamano","Martin Langhoff"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"72228","messageId":"E1Jay7s-0000dO-IX@fencepost.gnu.org","threadId":"12716","inReplyTo":null,"subject":"[PATCH] git-cvsserver: handle change type T","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-03-16T19:00:21Z","receivedAt":"2008-03-16T19:00:21Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"git-cvsserver does not support changes of type T (file type change,\ne.g. symlink->real file). This patch treats them the same as changes\nof type M.\n---\n git-cvsserver.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex afe3d0b..1b6b20e 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -2633,7 +2633,7 @@ sub update\n };\n $self->insert_rev($name, $head->{$name}{revision}, $hash, $commit->{hash}, $commit->{date}, $commit->{author}, $git_perms);\n }\n- elsif ( $change eq \"M\" )\n+ elsif ( $change eq \"M\" || $change eq \"T\" )\n {\n #$log->debug(\"MODIFIED $name\");\n $head->{$name} = {\n-- \n1.5.3.4.910.gc5122-dirty\n"},{"id":"72242","messageId":"7v63vmicvf.fsf@gitster.siamese.dyndns.org","threadId":"12716","inReplyTo":"E1Jay7s-0000dO-IX@fencepost.gnu.org","subject":"Re: [PATCH] git-cvsserver: handle change type T","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-16T21:21:24Z","receivedAt":"2008-03-16T21:21:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paolo Bonzini writes:\n\n> git-cvsserver does not support changes of type T (file type change,\n> e.g. symlink->real file). This patch treats them the same as changes\n> of type M.\n\nHmm.\n\nDo cvs clients even know how to handle symlinks, or will this patch just\nstart sending the readlink(2) result as if it is just a regular file\ncontents?\n"},{"id":"72246","messageId":"47DD9890.3060005@gnu.org","threadId":"12716","inReplyTo":"7v63vmicvf.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver: handle change type T","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-03-16T22:00:48Z","receivedAt":"2008-03-16T22:00:48Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"Junio C Hamano wrote:\n> Paolo Bonzini writes:\n> \n>> git-cvsserver does not support changes of type T (file type change,\n>> e.g. symlink->real file). This patch treats them the same as changes\n>> of type M.\n> \n> Hmm.\n> \n> Do cvs clients even know how to handle symlinks, or will this patch just\n> start sending the readlink(2) result as if it is just a regular file\n> contents?\n\nIt was before too. The problem arises if one found the problem, deleted \nthe symlink, added the regular file -- and now cvsserver stops working \nat all. :-(\n\nPaolo\n"},{"id":"72249","messageId":"7v4pb6gv7x.fsf@gitster.siamese.dyndns.org","threadId":"12716","inReplyTo":"47DD9890.3060005@gnu.org","subject":"Re: [PATCH] git-cvsserver: handle change type T","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-16T22:28:02Z","receivedAt":"2008-03-16T22:28:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paolo Bonzini writes:\n\n> Junio C Hamano wrote:\n>> Paolo Bonzini writes:\n>>\n>>> git-cvsserver does not support changes of type T (file type change,\n>>> e.g. symlink->real file). This patch treats them the same as changes\n>>> of type M.\n>>\n>> Hmm.\n>>\n>> Do cvs clients even know how to handle symlinks, or will this patch just\n>> start sending the readlink(2) result as if it is just a regular file\n>> contents?\n>\n> It was before too. The problem arises if one found the problem,\n> deleted the symlink, added the regular file -- and now cvsserver stops\n> working at all. :-(\n\nI fully agree that \"stops working at all\" was baaaaad, and your patch may\nbe an improvement.\n\nWhat I am questioning is what the code should do (IOW, what would be good\nfor cvs clients to see). Should it send a symlink as a regular file, or\nsimply just omit it? If the answer is \"the former\", then we do not have\nto worry about it any further after applying your patch. Otherwise we\nstill have to do something _else_ after applying your patch. I did not\nknow which way we would want to proceed, and I still don't.\n"},{"id":"72272","messageId":"47DE23A0.6040100@gnu.org","threadId":"12716","inReplyTo":"7v4pb6gv7x.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver: handle change type T","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-03-17T07:54:08Z","receivedAt":"2008-03-17T07:54:08Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"\n> What I am questioning is what the code should do (IOW, what would be good\n> for cvs clients to see). Should it send a symlink as a regular file, or\n> simply just omit it? If the answer is \"the former\", then we do not have\n> to worry about it any further after applying your patch. Otherwise we\n> still have to do something _else_ after applying your patch. I did not\n> know which way we would want to proceed, and I still don't.\n\nThe reason why my patch was enough for me, is that users would just use \nthe cvs bridge to stay up-to-date, not to spelunk the history (maybe to \nspelunk it with annotate, but not to look at old versions etc.). For \nthat, all I need is that current checkouts are okay. We have to decide \nif other cvsserver use cases are common enough.\n\nIf they are, I think the optimal way to proceed could be to detect \nrelative symlinks and resolve them (sending the linked file's content), \nand do \"something else\" (omit it, send the destination path as a regular \nfile?) for absolute symlinks.\n\nPaolo\n"},{"id":"72273","messageId":"7vve3lbwz9.fsf@gitster.siamese.dyndns.org","threadId":"12716","inReplyTo":"47DD9890.3060005@gnu.org","subject":"Re: [PATCH] git-cvsserver: handle change type T","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-17T08:01:14Z","receivedAt":"2008-03-17T08:01:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paolo Bonzini writes:\n\n> Junio C Hamano wrote:\n> ...\n>> Do cvs clients even know how to handle symlinks, or will this patch just\n>> start sending the readlink(2) result as if it is just a regular file\n>> contents?\n>\n> It was before too. The problem arises if one found the problem,\n> deleted the symlink, added the regular file -- and now cvsserver stops\n> working at all. :-(\n\nYeah, I see why $git_perms fall back to \"rw\" now around l.2620 -- if file\nmode is 120000, none of the r/w/x would have been added in.\n\nMartin, likes, dislikes?\n"},{"id":"72313","messageId":"46a038f90803171234m7e3f1cbfgb65311bfb64e6e0d@mail.gmail.com","threadId":"12716","inReplyTo":"7vve3lbwz9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver: handle change type T","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2008-03-17T19:34:39Z","receivedAt":"2008-03-17T19:34:39Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Mon, Mar 17, 2008 at 9:01 PM, Junio C Hamano wrote:\n> Martin, likes, dislikes?\n\nHmmm. Trying to make up my mind on this. It looks like symlink support\nis broken in cvsserver, it is something we did not even consider, so I\nam trying to visualise _how_ broken it is, and what it would take to\n\n - refuse to handle symlinks (temporarily, until we can guarantee our\nhandling is sane)\n - translate symlinks that are internal - hard, and probably not even\na good idea\n - handle symlinks directly - is the client safe from malicious symlinks?\n\nI have never used symlinks with either git or cvs, so I am unsure as\nto whether their handling is compatible. Do either of them perform any\nsanity checks on the targetof the symlink?\n\ncheers,\n\n\n\nm\n"},{"id":"72315","messageId":"47DECAD6.9060606@gnu.org","threadId":"12716","inReplyTo":"46a038f90803171234m7e3f1cbfgb65311bfb64e6e0d@mail.gmail.com","subject":"Re: [PATCH] git-cvsserver: handle change type T","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-03-17T19:47:34Z","receivedAt":"2008-03-17T19:47:34Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"> I have never used symlinks with either git or cvs, so I am unsure as\n> to whether their handling is compatible. Do either of them perform any\n> sanity checks on the targetof the symlink?\n\nI doubt that CVS actually supports symlinks. git makes no sanity checks.\n\nPaolo\n"}]}