{"thread":{"id":"6913","subject":"git-cvsserver and binary files","startedAt":"2007-02-22T15:04:30Z","lastAt":"2007-03-01T11:44:12Z","messageCount":17,"participants":["Andy Parkins","Junio C Hamano","Martin Langhoff","Karl Hasselström"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"35229","messageId":"200702221504.32848.andyparkins@gmail.com","threadId":"6913","inReplyTo":null,"subject":"git-cvsserver and binary files","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-02-22T15:04:30Z","receivedAt":"2007-02-22T15:04:30Z","isPatch":false,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"Hello,\n\nA colleague of mine is using TortoiseCVS to access git-cvserver on my system \nto do development on Windows with a repository stored on my machine.\n\nIt's all gone fairly well, nice and easy to use.  However, we've found that \nimages stored in the repository are being corrupted on checkout to the \nWindows machine.  Even though they are binary files, they're having the line \nendings rewritten (in a really strange way as well).\n\nI don't think git itself is at fault; the source files are all retaining their \noriginal unix line endings, and the images are uncorrupted when I check them \nout.  I've done a bit of research though and found that CVS marks binary \nfiles in the repository, not in the client, so I assume that git-cvsserver is \nat fault.  It's telling the client side that the image files are text.\n\nTo be as close to git as possible, I reckon that git-cvsserver should be \ntelling the remote that /every/ file is binary and leave the line endings \nalone (although that would perhaps annoy some windows users).  I'd much \nprefer it if what was checked out was what was checked in.  I'm at a bit of a \nloss as to how to do it though.  I don't know enough about the CVS protocol \nto know which part of git-cvsserver is telling the client the file type.\n\nShould I even try to make all binary checkouts via CVS or should I try and use \nthe auocrlf work that's going on now?\n\n\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"35231","messageId":"200702221606.42638.andyparkins@gmail.com","threadId":"6913","inReplyTo":"200702221504.32848.andyparkins@gmail.com","subject":"[PATCH] Added \"-kb\" to all the entries lines sent to the client","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-02-22T16:06:42Z","receivedAt":"2007-02-22T16:06:42Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"git doesn't distinguish between binary and text files - so force the\nclient to do the same by sending everything as a binary.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\nDON'T APPLY TO REPOSITORY\n\nTurns out the CVS protocol isn't as hard as I thought.\n\nhttp://soc.if.usp.br/doc/cvs/html-cvsclient/cvsclient_5.html#SEC6\n\nWas all I needed.  I've changed every entries line to send \"-kb\" as one of\noptions.  I believe this will make all files into binaries as far as CVS\nclients are concerned.\n\nI am certain this is too heavy handed for most users.  I submit this patch\nonly to help other poor souls who might have the same problem in the\nfuture.  (Hello poor soul).\n\nPerhaps when the whole .gitattributes system has settled down that could be\nused to conditionally set -kb\n\n git-cvsserver.perl |   18 +++++++++---------\n 1 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex f6ddf34..e8d74ae 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -374,7 +374,7 @@ sub req_add\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        print \"/$filepart/0///\\n\";\n+        print \"/$filepart/0//-kb/\\n\";\n \n         $addcount++;\n     }\n@@ -455,7 +455,7 @@ sub req_remove\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        print \"/$filepart/-1.$wrev///\\n\";\n+        print \"/$filepart/-1.$wrev//-kb/\\n\";\n \n         $rmcount++;\n     }\n@@ -726,7 +726,7 @@ sub req_co\n        print $state->{CVSROOT} . \"/$module/\" . ( defined ( $git->{dir} ) and \n$git->{dir} ne \"./\" ? $git->{dir} . \"/\" : \"\" ) . \"$git->{name}\\n\";\n \n         # this is an \"entries\" line\n-        print \"/$git->{name}/1.$git->{revision}///\\n\";\n+        print \"/$git->{name}/1.$git->{revision}//-kb/\\n\";\n         # permissions\n         print \"u=$git->{mode},g=$git->{mode},o=$git->{mode}\\n\";\n \n@@ -917,8 +917,8 @@ sub req_update\n \t\tprint $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n \n \t\t# this is an \"entries\" line\n-\t\t$log->debug(\"/$filepart/1.$meta->{revision}///\");\n-\t\tprint \"/$filepart/1.$meta->{revision}///\\n\";\n+\t\t$log->debug(\"/$filepart/1.$meta->{revision}//-kb/\");\n+\t\tprint \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n \n \t\t# permissions\n \t\t$log->debug(\"SEND : u=$meta->{mode},g=$meta->{mode},o=$meta->{mode}\");\n@@ -961,8 +961,8 @@ sub req_update\n                     print \"Update-existing $dirpart\\n\";\n                     \n$log->debug($state->{CVSROOT} . \"/$state->{module}/$filename\");\n                     print \n$state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    $log->debug(\"/$filepart/1.$meta->{revision}///\");\n-                    print \"/$filepart/1.$meta->{revision}///\\n\";\n+                    $log->debug(\"/$filepart/1.$meta->{revision}//-kb/\");\n+                    print \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n                 }\n             }\n             elsif ( $return == 1 )\n@@ -975,7 +975,7 @@ sub req_update\n                 {\n                     print \"Update-existing $dirpart\\n\";\n                     print \n$state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    print \"/$filepart/1.$meta->{revision}/+//\\n\";\n+                    print \"/$filepart/1.$meta->{revision}/+/-kb/\\n\";\n                 }\n             }\n             else\n@@ -1207,7 +1207,7 @@ sub req_ci\n         } else {\n             print \"Checked-in $dirpart\\n\";\n             print \"$filename\\n\";\n-            print \"/$filepart/1.$meta->{revision}///\\n\";\n+            print \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n         }\n     }\n \n-- \n1.5.0.1.51.g5a369\n"},{"id":"35263","messageId":"7v4ppedj3r.fsf@assigned-by-dhcp.cox.net","threadId":"6913","inReplyTo":"200702221606.42638.andyparkins@gmail.com","subject":"Re: [PATCH] Added \"-kb\" to all the entries lines sent to the client","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-22T20:37:28Z","receivedAt":"2007-02-22T20:37:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> Perhaps when the whole .gitattributes system has settled down that could be\n> used to conditionally set -kb\n\nIndeed.  And this patch is a great help by identifying where to\ninsert necessary '-kb'.\n\nIf the patch had a stub implementation to determine if -kb is\nneeded (and the stub always says \"yes\" for your purpose, or \"no\"\nfor backward compatibility for non-text files), it could be\napplied to the mainline for wider testing.\n\nsub path_is_binary {\n        my ($filename) = @_;\n\n\t# If you do not mind LF lineendings and care more about\n        # binary files, return true from here; otherwise you may\n        # want to change this to return false.\n        # We'll ask the much talked about .gitattributes\n        # mechanism when it's ready.\n        return 1;\n}\n\nsub req_add {\n        ...\n        $kb = path_is_binary($filename) ? \"-kb\" : \"\";\n        print \"Checked-in $dirpart\\n\";\n        print \"$filename\\n\";\n        print \"/$filepart/0//$kb/\\n\";\n\n        $addcount++;\n}\n"},{"id":"35611","messageId":"200702271345.09341.andyparkins@gmail.com","threadId":"6913","inReplyTo":"7v4ppedj3r.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-02-27T13:45:08Z","receivedAt":"2007-02-27T13:45:08Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"The config option gitcvs.allbinary may be set to force all entries to\nget the -kb flag.\n\nIn the future the gitattributes system will probably be a more\nappropriate way of doing this, but that will easily slot in as the\nentries lines sent to the CVS client now have their kopts set via the\nfunction kopts_from_path().\n\nIn the interim it might be better to not just have a all-or-nothing\napproach, but rather detect based on file extension (or file contents?).\nThat would slot in easily here as well.  However, I personally prefer\neverything to be binary-safe, so I just switch the switch.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\nHow about this instead?\n\nI've added one new function - kopts_from_path() - which is passed the\npath of the file having an \"Entries\" line generated.  It can do what it\nlikes to convert that into the options component.  I've just used a\nconfig option to decide whether everything is binary or nothing is binary.\n\nDefault is to be the same as previous behaviour - you have to specifically\nask for the binary flag.\n\n git-cvsserver.perl |   47 ++++++++++++++++++++++++++++++++++++++---------\n 1 files changed, 38 insertions(+), 9 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 56f21b6..15b9443 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -374,7 +374,8 @@ sub req_add\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        print \"/$filepart/0//-kb/\\n\";\n+        my $kopts = kopts_from_path($filepart);\n+        print \"/$filepart/0//$kopts/\\n\";\n \n         $addcount++;\n     }\n@@ -455,7 +456,8 @@ sub req_remove\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        print \"/$filepart/-1.$wrev//-kb/\\n\";\n+        my $kopts = kopts_from_path($filepart);\n+        print \"/$filepart/-1.$wrev//$kopts/\\n\";\n \n         $rmcount++;\n     }\n@@ -726,7 +728,8 @@ sub req_co\n        print $state->{CVSROOT} . \"/$module/\" . ( defined ( $git->{dir} ) and \n$git->{dir} ne \"./\" ? $git->{dir} . \"/\" : \"\" ) . \"$git->{name}\\n\";\n \n         # this is an \"entries\" line\n-        print \"/$git->{name}/1.$git->{revision}//-kb/\\n\";\n+        my $kopts = kopts_from_path($git->{name});\n+        print \"/$git->{name}/1.$git->{revision}//$kopts/\\n\";\n         # permissions\n         print \"u=$git->{mode},g=$git->{mode},o=$git->{mode}\\n\";\n \n@@ -917,8 +920,9 @@ sub req_update\n \t\tprint $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n \n \t\t# this is an \"entries\" line\n-\t\t$log->debug(\"/$filepart/1.$meta->{revision}//-kb/\");\n-\t\tprint \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n+\t\tmy $kopts = kopts_from_path($filepart);\n+\t\t$log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n+\t\tprint \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n \n \t\t# permissions\n \t\t$log->debug(\"SEND : u=$meta->{mode},g=$meta->{mode},o=$meta->{mode}\");\n@@ -961,8 +965,9 @@ sub req_update\n                     print \"Update-existing $dirpart\\n\";\n                     \n$log->debug($state->{CVSROOT} . \"/$state->{module}/$filename\");\n                     print \n$state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    $log->debug(\"/$filepart/1.$meta->{revision}//-kb/\");\n-                    print \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n+                    my $kopts = kopts_from_path($filepart);\n+                    $log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n+                    print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n                 }\n             }\n             elsif ( $return == 1 )\n@@ -975,7 +980,8 @@ sub req_update\n                 {\n                     print \"Update-existing $dirpart\\n\";\n                     print \n$state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    print \"/$filepart/1.$meta->{revision}/+/-kb/\\n\";\n+                    my $kopts = kopts_from_path($filepart);\n+                    print \"/$filepart/1.$meta->{revision}/+/$kopts/\\n\";\n                 }\n             }\n             else\n@@ -1206,7 +1212,8 @@ sub req_ci\n         } else {\n             print \"Checked-in $dirpart\\n\";\n             print \"$filename\\n\";\n-            print \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n+            my $kopts = kopts_from_path($filepart);\n+            print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n         }\n     }\n \n@@ -1887,6 +1894,28 @@ sub filecleanup\n     return $filename;\n }\n \n+# Given a path, this function returns a string containing the kopts\n+# that should go into that path's Entries line.  For example, a binary\n+# file should get -kb.\n+sub kopts_from_path\n+{\n+\tmy ($path) = @_;\n+\n+\t# Once it exists, the git attributes system should be used to look up\n+\t# what attributes apply to this path.\n+\n+\t# Until then, take the setting from the config file\n+    unless ( defined ( $cfg->{gitcvs}{allbinary} ) and $cfg->{gitcvs}\n{allbinary} =~ /^\\s*(1|true|yes)\\s*$/i )\n+    {\n+\t\t# Return \"\" to give no special treatment to any path\n+\t\treturn \"\";\n+    } else {\n+\t\t# Alternatively, to have all files treated as if they are binary (which\n+\t\t# is more like git itself), always return the \"-kb\" option\n+\t\treturn \"-kb\";\n+    }\n+}\n+\n package GITCVS::log;\n \n ####\n-- \n1.5.0.2.778.gdcb06\n"},{"id":"35612","messageId":"200702271346.55520.andyparkins@gmail.com","threadId":"6913","inReplyTo":"7v4ppedj3r.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-02-27T13:46:55Z","receivedAt":"2007-02-27T13:46:55Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"The config option gitcvs.allbinary may be set to force all entries to\nget the -kb flag.\n\nIn the future the gitattributes system will probably be a more\nappropriate way of doing this, but that will easily slot in as the\nentries lines sent to the CVS client now have their kopts set via the\nfunction kopts_from_path().\n\nIn the interim it might be better to not just have a all-or-nothing\napproach, but rather detect based on file extension (or file contents?).\nThat would slot in easily here as well.  However, I personally prefer\neverything to be binary-safe, so I just switch the switch.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\nThis time with word wrap turned off.  Sorry Junio.\n\n\n git-cvsserver.perl |   47 ++++++++++++++++++++++++++++++++++++++---------\n 1 files changed, 38 insertions(+), 9 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 56f21b6..15b9443 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -374,7 +374,8 @@ sub req_add\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        print \"/$filepart/0//-kb/\\n\";\n+        my $kopts = kopts_from_path($filepart);\n+        print \"/$filepart/0//$kopts/\\n\";\n \n         $addcount++;\n     }\n@@ -455,7 +456,8 @@ sub req_remove\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        print \"/$filepart/-1.$wrev//-kb/\\n\";\n+        my $kopts = kopts_from_path($filepart);\n+        print \"/$filepart/-1.$wrev//$kopts/\\n\";\n \n         $rmcount++;\n     }\n@@ -726,7 +728,8 @@ sub req_co\n        print $state->{CVSROOT} . \"/$module/\" . ( defined ( $git->{dir} ) and $git->{dir} ne \"./\" ? $git->{dir} . \"/\" : \"\" ) . \"$git->{name}\\n\";\n \n         # this is an \"entries\" line\n-        print \"/$git->{name}/1.$git->{revision}//-kb/\\n\";\n+        my $kopts = kopts_from_path($git->{name});\n+        print \"/$git->{name}/1.$git->{revision}//$kopts/\\n\";\n         # permissions\n         print \"u=$git->{mode},g=$git->{mode},o=$git->{mode}\\n\";\n \n@@ -917,8 +920,9 @@ sub req_update\n \t\tprint $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n \n \t\t# this is an \"entries\" line\n-\t\t$log->debug(\"/$filepart/1.$meta->{revision}//-kb/\");\n-\t\tprint \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n+\t\tmy $kopts = kopts_from_path($filepart);\n+\t\t$log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n+\t\tprint \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n \n \t\t# permissions\n \t\t$log->debug(\"SEND : u=$meta->{mode},g=$meta->{mode},o=$meta->{mode}\");\n@@ -961,8 +965,9 @@ sub req_update\n                     print \"Update-existing $dirpart\\n\";\n                     $log->debug($state->{CVSROOT} . \"/$state->{module}/$filename\");\n                     print $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    $log->debug(\"/$filepart/1.$meta->{revision}//-kb/\");\n-                    print \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n+                    my $kopts = kopts_from_path($filepart);\n+                    $log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n+                    print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n                 }\n             }\n             elsif ( $return == 1 )\n@@ -975,7 +980,8 @@ sub req_update\n                 {\n                     print \"Update-existing $dirpart\\n\";\n                     print $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    print \"/$filepart/1.$meta->{revision}/+/-kb/\\n\";\n+                    my $kopts = kopts_from_path($filepart);\n+                    print \"/$filepart/1.$meta->{revision}/+/$kopts/\\n\";\n                 }\n             }\n             else\n@@ -1206,7 +1212,8 @@ sub req_ci\n         } else {\n             print \"Checked-in $dirpart\\n\";\n             print \"$filename\\n\";\n-            print \"/$filepart/1.$meta->{revision}//-kb/\\n\";\n+            my $kopts = kopts_from_path($filepart);\n+            print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n         }\n     }\n \n@@ -1887,6 +1894,28 @@ sub filecleanup\n     return $filename;\n }\n \n+# Given a path, this function returns a string containing the kopts\n+# that should go into that path's Entries line.  For example, a binary\n+# file should get -kb.\n+sub kopts_from_path\n+{\n+\tmy ($path) = @_;\n+\n+\t# Once it exists, the git attributes system should be used to look up\n+\t# what attributes apply to this path.\n+\n+\t# Until then, take the setting from the config file\n+    unless ( defined ( $cfg->{gitcvs}{allbinary} ) and $cfg->{gitcvs}{allbinary} =~ /^\\s*(1|true|yes)\\s*$/i )\n+    {\n+\t\t# Return \"\" to give no special treatment to any path\n+\t\treturn \"\";\n+    } else {\n+\t\t# Alternatively, to have all files treated as if they are binary (which\n+\t\t# is more like git itself), always return the \"-kb\" option\n+\t\treturn \"-kb\";\n+    }\n+}\n+\n package GITCVS::log;\n \n ####\n-- \n1.5.0.2.778.gdcb06\n"},{"id":"35735","messageId":"7vslcr9mtl.fsf@assigned-by-dhcp.cox.net","threadId":"6913","inReplyTo":"200702271346.55520.andyparkins@gmail.com","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-27T23:56:38Z","receivedAt":"2007-02-27T23:56:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> +    unless ( defined ( $cfg->{gitcvs}{allbinary} ) and $cfg->{gitcvs}{allbinary} =~ /^\\s*(1|true|yes)\\s*$/i )\n> +    {\n> +\t\t# Return \"\" to give no special treatment to any path\n> +\t\treturn \"\";\n> +    } else {\n> +\t\t# Alternatively, to have all files treated as if they are binary (which\n> +\t\t# is more like git itself), always return the \"-kb\" option\n> +\t\treturn \"-kb\";\n> +    }\n> +}\n\nThis is not your fault as you copied existing code to check\nboolean, but I am unhappy every time I see \"git-config -l\"\nforces such an eyesore on us X-<.\n\nAnyway, will apply.  Thanks.\n"},{"id":"35795","messageId":"46a038f90702280336k6d368b8aj88ce8d3d822b1789@mail.gmail.com","threadId":"6913","inReplyTo":"200702271345.09341.andyparkins@gmail.com","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-02-28T11:36:17Z","receivedAt":"2007-02-28T11:36:17Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 2/28/07, Andy Parkins <andyparkins@gmail.com> wrote:\n> The config option gitcvs.allbinary may be set to force all entries to\n> get the -kb flag.\n\nHi Andy,\n\nI am a bit confused here... why would we want to do this? We don't\nwant to support keyword expansion, and that happens at the server end\nanyway. CVS clients are pretty dumb and will just write out the file\nwe give them. When we are going to diff, they send the file to the\nserver, and the server runs the diff. Etc. No smarts at the client end\nthat care about -k options.\n\nOr are you trying to control newline mangling on Windows/Mac clients?\nI'm not even sure where that mangling happens. If that's the case, a\nrepo-wide toggle is useless, you really want the per-file-type rules\nyou mention.\n\ndazed and confused...\n\n\n\nmartin\n"},{"id":"35799","messageId":"200702281301.58026.andyparkins@gmail.com","threadId":"6913","inReplyTo":"46a038f90702280336k6d368b8aj88ce8d3d822b1789@mail.gmail.com","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-02-28T13:01:56Z","receivedAt":"2007-02-28T13:01:56Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Wednesday 2007 February 28 11:36, Martin Langhoff wrote:\n\n> I am a bit confused here... why would we want to do this? We don't\n> want to support keyword expansion, and that happens at the server end\n\nWell, at the moment you don't, but that might happen in the future with \ncertain settings in .gitattributes.  It might be nice to have a switch to \ndisable that available.  However, that's not why I added this.\n\n> anyway. CVS clients are pretty dumb and will just write out the file\n> we give them. When we are going to diff, they send the file to the\n> server, and the server runs the diff. Etc. No smarts at the client end\n> that care about -k options.\n\nI think your supposition that no client cares about the -k options is wrong. \nCVS clients don't just write the file untouched.  CVS stores all text files \nwith UNIX line endings, the client then uses the text/binary flag to decide \nif line-ending conversion should happen.  Checking out to unix would of \ncourse not require any conversion so the file would be untouched.  Not so for \nWindows clients.  That was what prompted me to make this patch.  I keep some \nbinary images in the git repository, when they were checked out with \nTortoiseCVS they were having all the 0x0a (LF) bytes changed to CR LF, and \nthus mangling the images.  \n\n> Or are you trying to control newline mangling on Windows/Mac clients?\n> I'm not even sure where that mangling happens. If that's the case, a\n\nIt seems to be happening in the client.  In fact it's a given that it happens \nin the client as only the client knows what the local line ending rules are.\n\n> repo-wide toggle is useless, you really want the per-file-type rules\n> you mention.\n\n\"Useless\" is a strong word; especially as I'm finding it very useful :-). \n\nYes, a per-file option would be better - but git has no support for that at \npresent, so I need a work around.  Most windows editors will cope fine with \nUNIX line endings, so not converting them doesn't cause a problem.  On the \nother hand, mangling every binary file /does/ matter.  So, for me, the better \ndefault is to not mangle anything (i.e. tell CVS that everything is binary \nand should be preserved as is).\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"35904","messageId":"46a038f90702281540o520cc214xa929a3e3c4e70883@mail.gmail.com","threadId":"6913","inReplyTo":"200702281301.58026.andyparkins@gmail.com","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-02-28T23:40:50Z","receivedAt":"2007-02-28T23:40:50Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 3/1/07, Andy Parkins <andyparkins@gmail.com> wrote:\n> > Or are you trying to control newline mangling on Windows/Mac clients?\n> > I'm not even sure where that mangling happens. If that's the case, a\n>\n> It seems to be happening in the client.  In fact it's a given that it happens\n> in the client as only the client knows what the local line ending rules are.\n\nOk - that's an interesting bit of info. I wasn't sure.\n\n> > repo-wide toggle is useless, you really want the per-file-type rules\n> > you mention.\n>\n> \"Useless\" is a strong word; especially as I'm finding it very useful :-).\n\nI guess what I mean is that the common case on windows is going to be\nfor users who want their binary files un-corrupted, and their text\nfiles newline-converted.\n\nIn fact, I think we should have it set to default to binary -- which\ndoes the best job of preserving data. And override that based on file\nextension (configurable) or  check the \"head\" of the file cheaply for\nbinary-ness before we update the file on the client side.\n\nI agree with the idea of doing something smart with -kb flags, but\nthis is not a good move. For the common case among Windows TortoiseCVS\nusers, the switch proposed introduces the ability to switch between\none broken mode to another broken mode.\n\n(And in terms of temporary workarounds, TortoiseCVS has a switch in\nitself to disable newline conversion.)\n\ncheers,\n\n\nmartin\n"},{"id":"35933","messageId":"200703010840.54377.andyparkins@gmail.com","threadId":"6913","inReplyTo":"46a038f90702281540o520cc214xa929a3e3c4e70883@mail.gmail.com","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-03-01T08:40:50Z","receivedAt":"2007-03-01T08:40:50Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Wednesday 2007 February 28 23:40, Martin Langhoff wrote:\n\n> I guess what I mean is that the common case on windows is going to be\n> for users who want their binary files un-corrupted, and their text\n> files newline-converted.\n\nGiven that windows editors/tools generally cope quite well with UNIX line \nendings, the common case could well be that no one cares that the new line \nconversion isn't happening (it's certainly the case for me).\n\n> In fact, I think we should have it set to default to binary -- which\n\nMe too.  However, I didn't want to change existing behaviour.  That die has \nbeen cast unfortunately.\n\n> does the best job of preserving data. And override that based on file\n> extension (configurable) or  check the \"head\" of the file cheaply for\n> binary-ness before we update the file on the client side.\n\nYou're right in theory, but I really don't like the idea of auto-detection; \nthings like that /always/ go wrong and when they do there isn't a way to undo \nwhat it's done.  Douglas Adams had it right when he said:\n\n\"The major difference between a thing that might go wrong and a thing that \ncannot possibly go wrong is that when a thing that cannot possibly go wrong \ngoes wrong it usually turns out to be impossible to get at or repair.\"\n\n> I agree with the idea of doing something smart with -kb flags, but\n> this is not a good move. For the common case among Windows TortoiseCVS\n> users, the switch proposed introduces the ability to switch between\n> one broken mode to another broken mode.\n\nI don't understand.  What is broken about it?  As far as I can tell the -kb \nflag doesn't do anything smart, it /disables/ the smartness and tells the \nclient that the file is binary.  As you say - both modes are broken, so \nsupplying a switch isn't crazy because one broken mode might suit better than \nanother.  I agree that neither is ideal, but until the \"ideal\" is \nimplemented, it's better to have the option than not have it.\n\nI think I'm missing something that you are worrying about and that I haven't \nnoticed.  Does -kb do something that I'm not aware of?  Is there a more \ncorrect way of telling the client that a file is binary?\n\n> (And in terms of temporary workarounds, TortoiseCVS has a switch in\n> itself to disable newline conversion.)\n\nI'd prefer if the configuration is kept on the server side as much as \npossible.  Disabling newline conversion in the client is exactly the same \nsolution as putting -kb everywhere in the server, except in the server it \nonly needs doing once; and when a better method is eventually found the \nclients can continue without having to change anything.\n\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"35939","messageId":"46a038f90703010113o256f19a2qb1c16f4c85e5bd1c@mail.gmail.com","threadId":"6913","inReplyTo":"200703010840.54377.andyparkins@gmail.com","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-03-01T09:13:37Z","receivedAt":"2007-03-01T09:13:37Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 3/1/07, Andy Parkins <andyparkins@gmail.com> wrote:\n> On Wednesday 2007 February 28 23:40, Martin Langhoff wrote:\n>\n> > I guess what I mean is that the common case on windows is going to be\n> > for users who want their binary files un-corrupted, and their text\n> > files newline-converted.\n>\n> Given that windows editors/tools generally cope quite well with UNIX line\n> endings, the common case could well be that no one cares that the new line\n> conversion isn't happening (it's certainly the case for me).\n\nWell... the guys working on the MinGW port of git have railroaded us\nwith the opposite position. If it's going to work in Windows, you\nbetter accept it has to handle newline conversion correctly or it's\nbroken...\n\n...\n\n> > I agree with the idea of doing something smart with -kb flags, but\n> > this is not a good move. For the common case among Windows TortoiseCVS\n> > users, the switch proposed introduces the ability to switch between\n> > one broken mode to another broken mode.\n>\n> I don't understand.  What is broken about it?\n\nThe interesting case to fix this for is a mixed binary and text repo\nwith windows users wanting newline conversion (if they really don't,\nthey can tell TortoiseCVS as much). For that scenario, current\nbehaviour is broken (binaries get mangled), and setting -kb on\neverything breaks newline conversion.\n\nAnd that scenario can only be addressed sending appropriate flags for\neach file, not with a blanket switch.\n\n>  As you say - both modes are broken, so\n> supplying a switch isn't crazy because one broken mode might suit better than\n> another.\n\nExcept that it's not that hard to do something better -- I was hoping\nto prep a patch today, but things got frantic at the office.\n\n> I think I'm missing something that you are worrying about and that I haven't\n> noticed.  Does -kb do something that I'm not aware of?  Is there a more\n> correct way of telling the client that a file is binary?\n\nJust that I think it's easy enough to implement something that sets\n-k mode on file extension, which is actually _much_ better and makes\nthe blanket setting pointless. And perhaps we can get binary\nautodetection working well but accepting overrides.\n\ncheers,\n\n\nmartin\n"},{"id":"35949","messageId":"200703010941.20161.andyparkins@gmail.com","threadId":"6913","inReplyTo":"46a038f90703010113o256f19a2qb1c16f4c85e5bd1c@mail.gmail.com","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-03-01T09:41:16Z","receivedAt":"2007-03-01T09:41:16Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Thursday 2007 March 01 09:13, Martin Langhoff wrote:\n\n> Well... the guys working on the MinGW port of git have railroaded us\n> with the opposite position. If it's going to work in Windows, you\n> better accept it has to handle newline conversion correctly or it's\n> broken...\n\nI do accept that, but as you said - it's broken whether you pick all binary or \nall text.  I'm not advocating that my solution is final, but it is better to \nnot convert text than to mangle binary.\n\n> The interesting case to fix this for is a mixed binary and text repo\n> with windows users wanting newline conversion (if they really don't,\n> they can tell TortoiseCVS as much). For that scenario, current\n> behaviour is broken (binaries get mangled), and setting -kb on\n> everything breaks newline conversion.\n>\n> And that scenario can only be addressed sending appropriate flags for\n> each file, not with a blanket switch.\n\nErm, yes, I know that.  But who is going to set that switch?  This isn't real \nCVS where the repository records that information.  At the moment git does \nnot know whether any given file is binary or text.\n\n> Except that it's not that hard to do something better -- I was hoping\n> to prep a patch today, but things got frantic at the office.\n\nI think it is hard; as git doesn't supply the needed information.  I suppose \nit could be stored in the SQLlite table, but that is seriously borked.  That \ntable is a necessary evil, not a place to start storing any old flag.\n\n> Just that I think it's easy enough to implement something that sets\n> -k mode on file extension, which is actually _much_ better and makes\n> the blanket setting pointless. And perhaps we can get binary\n> autodetection working well but accepting overrides.\n\nAs I said, I don't like autodetection.  It /will/ get it wrong.  As for using \nfile extension - well that's wrong too if it's coded into the source.\n\nIn a previous life, I used to develop a unix program on a windows computer; \nthe UNIX directory was mounted as a samba share and built and run via an ssh \nconnection.  How is git-cvsserver meant to know about a crazy situation like \nthat and \"auto-detect\" the right thing?  It can't.\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"35950","messageId":"7vk5y1uwhg.fsf@assigned-by-dhcp.cox.net","threadId":"6913","inReplyTo":"200703010941.20161.andyparkins@gmail.com","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-01T09:46:51Z","receivedAt":"2007-03-01T09:46:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> Erm, yes, I know that.  But who is going to set that switch?\n> This isn't real CVS where the repository records that\n> information.  At the moment git does not know whether any\n> given file is binary or text.\n\nWhat do you think I have been hacking around pathattr stuff\ntoday for ;-)? \n"},{"id":"35952","messageId":"7vr6s9th4f.fsf@assigned-by-dhcp.cox.net","threadId":"6913","inReplyTo":"7vk5y1uwhg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-01T10:04:00Z","receivedAt":"2007-03-01T10:04:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> Andy Parkins <andyparkins@gmail.com> writes:\n>\n>> Erm, yes, I know that.  But who is going to set that switch?\n>> This isn't real CVS where the repository records that\n>> information.  At the moment git does not know whether any\n>> given file is binary or text.\n>\n> What do you think I have been hacking around pathattr stuff\n> today for ;-)? \n\nA bit more clarification.  Although I won't be hacking on it\nanymore since it is almost my bedtime,...\n\nI think the 4 patch series I sent out tonight was fun but\nslightly misdesigned.  I should have made the classification of\npaths and actions on them separate sections.  That is, instead\nof saying:\n\n\t[pathattr \"a/v\"]\n        \tpath = \"*.mpg\"\n                path = \"*.mp3\"\n                path = \"*.jpg\"\n\t\tconv_i = none\n                conv_o = none\n\t[pathattr \"text\"]\n        \tpath = \"*\" ; all the rest\n                conv_i = crlf\n                conv_o = crlf\n\nwe should make two kinds of configuration.  Classification of\npaths is property of the project and does not depend on where\nthe user uses the paths from the project:\n\n\t[pathattr \"a/v\"]\n        \tpath = \"*.mpg\"\n                path = \"*.mp3\"\n                path = \"*.jpg\"\n\t[pathattr \"text\"]\n        \tpath = \"*\" ; all the rest\n\nwhile how they are handled can be platform dependent.  On UNIX,\nyou might have this in $HOME/.gitconfig:\n\n\t[handler \"a/v\"]\n        \tpretty = \"cmd xine %s\" ; nb. there is no 'cmd' yet...\n\t[handler \"text\"]\n\t\tpretty = \"pipe fmt -\"\n\nwhile on another system, you might have:\n\n\t[handler \"a/v\"]\n        \tpretty = \"cmd mediaplayer %s\"\n\t[handler \"text\"]\n                conv_i = crlf\n                conv_o = crlf\n\nOne thing to note is that [pathattr \"kind\"] may need to behave\nlike existing .gitignore in that they are read from directories,\nbut [handler \"kind\"] does not have to.\n\nWe probably need to expose pathattr_lookup(path) to scripts, and\ncvsserver could use that interface to query.  The interface may\nlook like this:\n\n\t$ git pathattr --lookup porn.mpg\n        a/v\n        $ git config --get handler.a/v\n\tcmd xine %s\n\nand kopts_from_path would look like:\n\n\tsub kopts_from_path {\n        \tmy ($path) = shift;\n                my $kind = `git pathattr $path`;\n\t\tmy $conv_o = `git config --get \"handler.$kind\"`\n\n                if ($conv_o eq 'crlf') {\n\t\t\treturn '-kb';\n\t\t}\n\t\treturn '';\n\t}\n\nHmm?\n"},{"id":"35963","messageId":"7vhct5rztc.fsf@assigned-by-dhcp.cox.net","threadId":"6913","inReplyTo":"7vr6s9th4f.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-01T11:03:11Z","receivedAt":"2007-03-01T11:03:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> we should make two kinds of configuration.  Classification of\n> paths is property of the project and does not depend on where\n> the user uses the paths from the project:\n> ...\n> while how they are handled can be platform dependent.  On UNIX,\n> you might have this in $HOME/.gitconfig:\n>\n> \t[handler \"a/v\"]\n>         \tpretty = \"cmd xine %s\" ; nb. there is no 'cmd' yet...\n> \t[handler \"text\"]\n> \t\tpretty = \"pipe fmt -\"\n>\n> while on another system, you might have:\n>\n> \t[handler \"a/v\"]\n>         \tpretty = \"cmd mediaplayer %s\"\n> \t[handler \"text\"]\n>                 conv_i = crlf\n>                 conv_o = crlf\n\nOnce we separate out the handler section, we can introduce more\nuseful variables in it (not the \"pretty\", whose sole purpose was\nto have fun and serve as a demonstration).  Obvious ones are:\n\n\t; takes temporary files old, mine, his and writes the\n        ; result in another\n\tmerge = \"cmd external-merge-command %s %s %s %s\"\n\n\t; takes two temporary files\n        diff = \"cmd external-diff-command %s %s\"\n"},{"id":"35966","messageId":"200703011140.11108.andyparkins@gmail.com","threadId":"6913","inReplyTo":"7vhct5rztc.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-03-01T11:40:10Z","receivedAt":"2007-03-01T11:40:10Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Thursday 2007 March 01 11:03, Junio C Hamano wrote:\n\n> Once we separate out the handler section, we can introduce more\n> useful variables in it (not the \"pretty\", whose sole purpose was\n> to have fun and serve as a demonstration).  Obvious ones are:\n>\n> \t; takes temporary files old, mine, his and writes the\n>         ; result in another\n> \tmerge = \"cmd external-merge-command %s %s %s %s\"\n>\n> \t; takes two temporary files\n>         diff = \"cmd external-diff-command %s %s\"\n\nI was part way through assembling a document when I read this; I hadn't \nthought of the external merge option.  I'll add that and send the document \nsoon.\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"35967","messageId":"20070301114412.GA13535@diana.vm.bytemark.co.uk","threadId":"6913","inReplyTo":"7vhct5rztc.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] cvsserver: Make always-binary mode a config file option","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-03-01T11:44:12Z","receivedAt":"2007-03-01T11:44:12Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-03-01 03:03:11 -0800, Junio C Hamano wrote:\n\n>       ; takes temporary files old, mine, his and writes the\n>         ; result in another\n>       merge = \"cmd external-merge-command %s %s %s %s\"\n\nBetter make this \"%1 %2 %3 %4\", \"%old %mine %his %result\", or\nsomething along those lines, so as to not force a particular argument\norder on the external script.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"}]}