{"thread":{"id":"13526","subject":"[PATCH 2/3] implement gitcvs.usecrlfattr","startedAt":"2008-05-15T04:35:45Z","lastAt":"2008-05-20T03:05:58Z","messageCount":11,"participants":["Matthew Ogilvie","Junio C Hamano","Martin Langhoff","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"77019","messageId":"1210826148-8708-1-git-send-email-mmogilvi_git@miniinfo.net","threadId":"13526","inReplyTo":null,"subject":"[PATCH 0/3] git-cvsserver: Add support for some binary files","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2008-05-15T04:35:45Z","receivedAt":"2008-05-15T04:35:45Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"This series of patches extends git-cvsserver to support telling the\nCVS client to set the -kb (binary) mode for files that git considers\nto be binary (and not for text files).  It includes updates to\ndocumentation and tests.\n\nBy default the new binary support is not enabled.  To enable it,\nyou should set \"gitcvs.usecrlfattr\" and \"gitcvs.allbinary=guess\",\nas described in the updated documentation.\n\n-----------------\n\nThis patch series is usable now, but there are some things I'm not\nsure about, and things that could still use improvement:\n\n1. As currently implemented, the second patch (for checking file\nattributes) forks a separate instance of git-check-attr for every\nfile it needs to look up in the repository.  Each invocation involves\nreading the index file, so things may get kind of slow if\nthere are a whole lot of files in the repository.  It might be\nworth reorganizing things so that it can ask about multiple\nfiles in one invocation of git-check-attr, but such a change would\nprobably be invasive enough to warrant a separate patch.\n\n2. Is there a better/more intuitive way of configuring this?  Perhaps\n\"gitcvs.autocrlf\" that is similar to \"core.autocrlf\"?  But it seems\nunfriendly to drop default and \"gitcvs.allbinary\" modes; some\nusers may have set things up such that those modes are needed.\n\n3. I'm not sure about the best way to handle repeatably changing\ncurrent directory.  The first patch tries to make a somewhat general\nmechanism to manage it, but I keep thinking in the back of my mind\nthat it might be better to set up a working directory first thing,\nand then minimize any further directory changes after that.  Does\nanyone have any thoughts about this?\n\n4. Possibly additional enhancements including:\na. Strip out '\\r' from \"text\" files, so when the CVS client\nadds '\\r', you don't wind up with double '\\r's per line.\nb. Additional conversions like in convert.c, done on server side.\nIncluding safecrlf, smudge/clean filters, etc.\nc. If a new .gitattributes file is sent by the client, use it\nin preference over the one from the most recent commit.  As it\nis now, a user might need to commit the new .gitattributes before\ncommitting anything else.  This might be much easier if a new\noverall design for setting up and using a working directory was\nused (see above).\n\n5. It might make things clearer to refactor the special case\ntransmitfile() modes to be implemented as separate functions that\nuse open_blob_or_die().  Probably a separate patch, if done at all.\n\n6. Additional tweaks to the documentation?  For example, should\nthere be a note on \"core.autocrlf\" that binary support in emulation\ntools may use other configuration variables...\n\nMatthew Ogilvie (3):\n      git-cvsserver: add mechanism for managing working tree and current directory\n      implement gitcvs.usecrlfattr\n      git-cvsserver: add ability to guess -kb from contents\n\n Documentation/config.txt        |   26 ++-\n Documentation/git-cvsserver.txt |   32 ++-\n git-cvsserver.perl              |  500 ++++++++++++++++++++++++++++++++++-----\n t/t9401-git-cvsserver-crlf.sh   |  337 ++++++++++++++++++++++++++\n 4 files changed, 826 insertions(+), 69 deletions(-)\n create mode 100755 t/t9401-git-cvsserver-crlf.sh\n"},{"id":"77017","messageId":"1210826148-8708-2-git-send-email-mmogilvi_git@miniinfo.net","threadId":"13526","inReplyTo":"1210826148-8708-1-git-send-email-mmogilvi_git@miniinfo.net","subject":"[PATCH 1/3] git-cvsserver: add mechanism for managing working tree and current directory","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2008-05-15T04:35:46Z","receivedAt":"2008-05-15T04:35:46Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"There are various reasons git-cvsserver needs to manipulate the current\ndirectory, and this patch attempts to clarify and validate such changes:\n\n1. Temporary empty working directory (with index) for certain operations\n   that require an index file to work.\n2. Use a temporary directory with temporary file names for doing\n   merges of user's dirty sandbox state with latest changes in\n   repository.\n3. Coming up soon: Set up an index and either a valid or empty\n   working directory when calling git-check-attr to decide\n   if a file should be marked binary (-kb).\n\nSigned-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>\n---\n\nI'm not sure about this.  I get the vague sense it might be better to\njust always set up a (usually empty except for the index file and\nuser's changed files) working directory early in processing, and\nminimize chdir calls after that.  It might also make it\npossible to have a new .gitattributes file take effect immediately.\nBut that would be a much more invasive change.\n\n git-cvsserver.perl |  252 ++++++++++++++++++++++++++++++++++++++++++++--------\n 1 files changed, 213 insertions(+), 39 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 29dbfc9..674892b 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -21,6 +21,7 @@ use bytes;\n \n use Fcntl;\n use File::Temp qw/tempdir tempfile/;\n+use File::Path qw/rmtree/;\n use File::Basename;\n use Getopt::Long qw(:config require_order no_ignore_case);\n \n@@ -86,6 +87,17 @@ my $methods = {\n # $state holds all the bits of information the clients sends us that could\n # potentially be useful when it comes to actually _doing_ something.\n my $state = { prependdir => '' };\n+\n+# Work is for managing temporary working directory\n+my $work =\n+    {\n+        state => undef,  # undef, 1 (empty), 2 (with stuff)\n+        workDir => undef,\n+        index => undef,\n+        emptyDir => undef,\n+        tmpDir => undef\n+    };\n+\n $log->info(\"--------------- STARTING -----------------\");\n \n my $usage =\n@@ -189,6 +201,9 @@ while (<STDIN>)\n $log->debug(\"Processing time : user=\" . (times)[0] . \" system=\" . (times)[1]);\n $log->info(\"--------------- FINISH -----------------\");\n \n+chdir '/';\n+exit 0;\n+\n # Magic catchall method.\n #    This is the method that will handle all commands we haven't yet\n #    implemented. It simply sends a warning to the log file indicating a\n@@ -1101,10 +1116,10 @@ sub req_update\n             $log->info(\"Updating '$filename'\");\n             my ( $filepart, $dirpart ) = filenamesplit($meta->{name},1);\n \n-            my $dir = tempdir( DIR => $TEMP_DIR, CLEANUP => 1 ) . \"/\";\n+            my $mergeDir = setupTmpDir();\n \n-            chdir $dir;\n             my $file_local = $filepart . \".mine\";\n+            my $mergedFile = \"$mergeDir/$file_local\";\n             system(\"ln\",\"-s\",$state->{entries}{$filename}{modified_filename}, $file_local);\n             my $file_old = $filepart . \".\" . $oldmeta->{revision};\n             transmitfile($oldmeta->{filehash}, { targetfile => $file_old });\n@@ -1115,11 +1130,13 @@ sub req_update\n             $log->info(\"Merging $file_local, $file_old, $file_new\");\n             print \"M Merging differences between 1.$oldmeta->{revision} and 1.$meta->{revision} into $filename\\n\";\n \n-            $log->debug(\"Temporary directory for merge is $dir\");\n+            $log->debug(\"Temporary directory for merge is $mergeDir\");\n \n             my $return = system(\"git\", \"merge-file\", $file_local, $file_old, $file_new);\n             $return >>= 8;\n \n+            cleanupTmpDir();\n+\n             if ( $return == 0 )\n             {\n                 $log->info(\"Merged successfully\");\n@@ -1168,13 +1185,11 @@ sub req_update\n                 # transmit file, format is single integer on a line by itself (file\n                 # size) followed by the file contents\n                 # TODO : we should copy files in blocks\n-                my $data = `cat $file_local`;\n+                my $data = `cat $mergedFile`;\n                 $log->debug(\"File size : \" . length($data));\n                 print length($data) . \"\\n\";\n                 print $data;\n             }\n-\n-            chdir \"/\";\n         }\n \n     }\n@@ -1195,6 +1210,7 @@ sub req_ci\n     if ( $state->{method} eq 'pserver')\n     {\n         print \"error 1 pserver access cannot commit\\n\";\n+        cleanupWorkTree();\n         exit;\n     }\n \n@@ -1202,6 +1218,7 @@ sub req_ci\n     {\n         $log->warn(\"file 'index' already exists in the git repository\");\n         print \"error 1 Index already exists in git repo\\n\";\n+        cleanupWorkTree();\n         exit;\n     }\n \n@@ -1209,31 +1226,20 @@ sub req_ci\n     my $updater = GITCVS::updater->new($state->{CVSROOT}, $state->{module}, $log);\n     $updater->update();\n \n-    my $tmpdir = tempdir ( DIR => $TEMP_DIR );\n-    my ( undef, $file_index ) = tempfile ( DIR => $TEMP_DIR, OPEN => 0 );\n-    $log->info(\"Lockless commit start, basing commit on '$tmpdir', index file is '$file_index'\");\n-\n-    $ENV{GIT_DIR} = $state->{CVSROOT} . \"/\";\n-    $ENV{GIT_WORK_TREE} = \".\";\n-    $ENV{GIT_INDEX_FILE} = $file_index;\n-\n     # Remember where the head was at the beginning.\n     my $parenthash = `git show-ref -s refs/heads/$state->{module}`;\n     chomp $parenthash;\n     if ($parenthash !~ /^[0-9a-f]{40}$/) {\n \t    print \"error 1 pserver cannot find the current HEAD of module\";\n+\t    cleanupWorkTree();\n \t    exit;\n     }\n \n-    chdir $tmpdir;\n+    setupWorkTree($parenthash);\n \n-    # populate the temporary index\n-    system(\"git-read-tree\", $parenthash);\n-    unless ($? == 0)\n-    {\n-\tdie \"Error running git-read-tree $state->{module} $file_index $!\";\n-    }\n-    $log->info(\"Created index '$file_index' for head $state->{module} - exit status $?\");\n+    $log->info(\"Lockless commit start, basing commit on '$work->{workDir}', index file is '$work->{index}'\");\n+\n+    $log->info(\"Created index '$work->{index}' for head $state->{module} - exit status $?\");\n \n     my @committedfiles = ();\n     my %oldmeta;\n@@ -1271,7 +1277,7 @@ sub req_ci\n         {\n             # fail everything if an up to date check fails\n             print \"error 1 Up to date check failed for $filename\\n\";\n-            chdir \"/\";\n+            cleanupWorkTree();\n             exit;\n         }\n \n@@ -1313,7 +1319,7 @@ sub req_ci\n     {\n         print \"E No files to commit\\n\";\n         print \"ok\\n\";\n-        chdir \"/\";\n+        cleanupWorkTree();\n         return;\n     }\n \n@@ -1336,7 +1342,7 @@ sub req_ci\n     {\n         $log->warn(\"Commit failed (Invalid commit hash)\");\n         print \"error 1 Commit failed (unknown reason)\\n\";\n-        chdir \"/\";\n+        cleanupWorkTree();\n         exit;\n     }\n \n@@ -1348,7 +1354,7 @@ sub req_ci\n \t\t{\n \t\t\t$log->warn(\"Commit failed (update hook declined to update ref)\");\n \t\t\tprint \"error 1 Commit failed (update hook declined)\\n\";\n-\t\t\tchdir \"/\";\n+\t\t\tcleanupWorkTree();\n \t\t\texit;\n \t\t}\n \t}\n@@ -1358,6 +1364,7 @@ sub req_ci\n \t\t\t\"refs/heads/$state->{module}\", $commithash, $parenthash)) {\n \t\t$log->warn(\"update-ref for $state->{module} failed.\");\n \t\tprint \"error 1 Cannot commit -- update first\\n\";\n+\t\tcleanupWorkTree();\n \t\texit;\n \t}\n \n@@ -1414,7 +1421,7 @@ sub req_ci\n         }\n     }\n \n-    chdir \"/\";\n+    cleanupWorkTree();\n     print \"ok\\n\";\n }\n \n@@ -1757,15 +1764,9 @@ sub req_annotate\n     argsfromdir($updater);\n \n     # we'll need a temporary checkout dir\n-    my $tmpdir = tempdir ( DIR => $TEMP_DIR );\n-    my ( undef, $file_index ) = tempfile ( DIR => $TEMP_DIR, OPEN => 0 );\n-    $log->info(\"Temp checkoutdir creation successful, basing annotate session work on '$tmpdir', index file is '$file_index'\");\n-\n-    $ENV{GIT_DIR} = $state->{CVSROOT} . \"/\";\n-    $ENV{GIT_WORK_TREE} = \".\";\n-    $ENV{GIT_INDEX_FILE} = $file_index;\n+    setupWorkTree();\n \n-    chdir $tmpdir;\n+    $log->info(\"Temp checkoutdir creation successful, basing annotate session work on '$work->{workDir}', index file is '$ENV{GIT_INDEX_FILE}'\");\n \n     # foreach file specified on the command line ...\n     foreach my $filename ( @{$state->{args}} )\n@@ -1789,10 +1790,10 @@ sub req_annotate\n \tsystem(\"git-read-tree\", $lastseenin);\n \tunless ($? == 0)\n \t{\n-\t    print \"E error running git-read-tree $lastseenin $file_index $!\\n\";\n+\t    print \"E error running git-read-tree $lastseenin $ENV{GIT_INDEX_FILE} $!\\n\";\n \t    return;\n \t}\n-\t$log->info(\"Created index '$file_index' with commit $lastseenin - exit status $?\");\n+\t$log->info(\"Created index '$ENV{GIT_INDEX_FILE}' with commit $lastseenin - exit status $?\");\n \n         # do a checkout of the file\n         system('git-checkout-index', '-f', '-u', $filename);\n@@ -1808,7 +1809,7 @@ sub req_annotate\n         # git-jsannotate telling us about commits we are hiding\n         # from the client.\n \n-        my $a_hints = \"$tmpdir/.annotate_hints\";\n+        my $a_hints = \"$work->{workDir}/.annotate_hints\";\n         if (!open(ANNOTATEHINTS, '>', $a_hints)) {\n             print \"E failed to open '$a_hints' for writing: $!\\n\";\n             return;\n@@ -1862,7 +1863,7 @@ sub req_annotate\n     }\n \n     # done; get out of the tempdir\n-    chdir \"/\";\n+    cleanupWorkDir();\n \n     print \"ok\\n\";\n \n@@ -2115,6 +2116,179 @@ sub filecleanup\n     return $filename;\n }\n \n+sub validateGitDir\n+{\n+    if( !defined($state->{CVSROOT}) )\n+    {\n+        print \"error 1 CVSROOT not specified\\n\";\n+        cleanupWorkTree();\n+        exit;\n+    }\n+    if( $ENV{GIT_DIR} ne ($state->{CVSROOT} . '/') )\n+    {\n+        print \"error 1 Internally inconsistent CVSROOT\\n\";\n+        cleanupWorkTree();\n+        exit;\n+    }\n+}\n+\n+# Setup working directory in a work tree with the requested version\n+# loaded in the index.\n+sub setupWorkTree\n+{\n+    my ($ver) = @_;\n+\n+    validateGitDir();\n+\n+    if( ( defined($work->{state}) && $work->{state} != 1 ) ||\n+        defined($work->{tmpDir}) )\n+    {\n+        $log->warn(\"Bad work tree state management\");\n+        print \"error 1 Internal setup multiple work trees without cleanup\\n\";\n+        cleanupWorkTree();\n+        exit;\n+    }\n+\n+    $work->{workDir} = tempdir ( DIR => $TEMP_DIR );\n+\n+    if( !defined($work->{index}) )\n+    {\n+        (undef, $work->{index}) = tempfile ( DIR => $TEMP_DIR, OPEN => 0 );\n+    }\n+\n+    chdir $work->{workDir} or\n+        die \"Unable to chdir to $work->{workDir}\\n\";\n+\n+    $log->info(\"Setting up GIT_WORK_TREE as '.' in '$work->{workDir}', index file is '$work->{index}'\");\n+\n+    $ENV{GIT_WORK_TREE} = \".\";\n+    $ENV{GIT_INDEX_FILE} = $work->{index};\n+    $work->{state} = 2;\n+\n+    if($ver)\n+    {\n+        system(\"git\",\"read-tree\",$ver);\n+        unless ($? == 0)\n+        {\n+            $log->warn(\"Error running git-read-tree\");\n+            die \"Error running git-read-tree $ver in $work->{workDir} $!\\n\";\n+        }\n+    }\n+    # else # req_annotate reads tree for each file\n+}\n+\n+# Ensure current directory is in some kind of working directory,\n+# with a recent version loaded in the index.\n+sub ensureWorkTree\n+{\n+    if( defined($work->{tmpDir}) )\n+    {\n+        $log->warn(\"Bad work tree state management [ensureWorkTree()]\");\n+        print \"error 1 Internal setup multiple dirs without cleanup\\n\";\n+        cleanupWorkTree();\n+        exit;\n+    }\n+    if( $work->{state} )\n+    {\n+        return;\n+    }\n+\n+    validateGitDir();\n+\n+    if( !defined($work->{emptyDir}) )\n+    {\n+        $work->{emptyDir} = tempdir ( DIR => $TEMP_DIR, OPEN => 0);\n+    }\n+    chdir $work->{emptyDir} or\n+        die \"Unable to chdir to $work->{emptyDir}\\n\";\n+\n+    my $ver = `git show-ref -s refs/heads/$state->{module}`;\n+    chomp $ver;\n+    if ($ver !~ /^[0-9a-f]{40}$/)\n+    {\n+        $log->warn(\"Error from git show-ref -s refs/head$state->{module}\");\n+        print \"error 1 cannot find the current HEAD of module\";\n+        cleanupWorkTree();\n+        exit;\n+    }\n+\n+    if( !defined($work->{index}) )\n+    {\n+        (undef, $work->{index}) = tempfile ( DIR => $TEMP_DIR, OPEN => 0 );\n+    }\n+\n+    $ENV{GIT_WORK_TREE} = \".\";\n+    $ENV{GIT_INDEX_FILE} = $work->{index};\n+    $work->{state} = 1;\n+\n+    system(\"git\",\"read-tree\",$ver);\n+    unless ($? == 0)\n+    {\n+        die \"Error running git-read-tree $ver $!\\n\";\n+    }\n+}\n+\n+# Cleanup working directory that is not needed any longer.\n+sub cleanupWorkTree\n+{\n+    if( ! $work->{state} )\n+    {\n+        return;\n+    }\n+\n+    chdir \"/\" or die \"Unable to chdir '/'\\n\";\n+\n+    if( defined($work->{workDir}) )\n+    {\n+        rmtree( $work->{workDir} );\n+        undef $work->{workDir};\n+    }\n+    undef $work->{state};\n+}\n+\n+# Setup a temporary directory (not a working tree), typically for\n+# merging dirty state as in req_update.\n+sub setupTmpDir\n+{\n+    $work->{tmpDir} = tempdir ( DIR => $TEMP_DIR );\n+    chdir $work->{tmpDir} or die \"Unable to chdir $work->{tmpDir}\\n\";\n+\n+    return $work->{tmpDir};\n+}\n+\n+# Clean up a previously setupTmpDir.  Restore previous work tree if\n+# appropriate.\n+sub cleanupTmpDir\n+{\n+    if ( !defined($work->{tmpDir}) )\n+    {\n+        $log->warn(\"cleanup tmpdir that has not been setup\");\n+        die \"Cleanup tmpDir that has not been setup\\n\";\n+    }\n+    if( defined($work->{state}) )\n+    {\n+        if( $work->{state} == 1 )\n+        {\n+            chdir $work->{emptyDir} or\n+                die \"Unable to chdir to $work->{emptyDir}\\n\";\n+        }\n+        elsif( $work->{state} == 2 )\n+        {\n+            chdir $work->{workDir} or\n+                die \"Unable to chdir to $work->{emptyDir}\\n\";\n+        }\n+        else\n+        {\n+            $log->warn(\"Inconsistent work dir state\");\n+            die \"Inconsistent work dir state\\n\";\n+        }\n+    }\n+    else\n+    {\n+        chdir \"/\" or die \"Unable to chdir '/'\\n\";\n+    }\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-- \n1.5.4.3.340.g97b97\n"},{"id":"77016","messageId":"1210826148-8708-3-git-send-email-mmogilvi_git@miniinfo.net","threadId":"13526","inReplyTo":"1210826148-8708-2-git-send-email-mmogilvi_git@miniinfo.net","subject":"[PATCH 2/3] implement gitcvs.usecrlfattr","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2008-05-15T04:35:47Z","receivedAt":"2008-05-15T04:35:47Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"If gitcvs.usecrlfattr is set to true, git-cvsserver will consult\nthe \"crlf\" for each file to determine if it should mark the file\nas binary (-kb).\n\nSigned-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>\n---\n\nThere may be a performance issue with using a separate invocation of\ngit-check-attr for every file.  Perhaps an additional patch is needed\nto reorganize things to check multiple files in one invocation.\n\n Documentation/config.txt        |   23 ++++--\n Documentation/git-cvsserver.txt |   26 +++++-\n git-cvsserver.perl              |   71 +++++++++++++---\n t/t9401-git-cvsserver-crlf.sh   |  178 +++++++++++++++++++++++++++++++++++++++\n 4 files changed, 276 insertions(+), 22 deletions(-)\n create mode 100755 t/t9401-git-cvsserver-crlf.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex a6fc5a2..820795f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -645,11 +645,21 @@ gitcvs.logfile::\n \tPath to a log file where the CVS server interface well... logs\n \tvarious stuff. See linkgit:git-cvsserver[1].\n \n+gitcvs.usecrlfattr\n+\tIf true, the server will look up the `crlf` attribute for\n+\tfiles to determine the '-k' modes to use. If `crlf` is set,\n+\tthe '-k' mode will be left blank, so cvs clients will\n+\ttreat it as text. If `crlf` is explicitly unset, the file\n+\twill be set with '-kb' mode, which supresses any newline munging\n+\tthe client might otherwise do. If `crlf` is not specified,\n+\tthen 'gitcvs.allbinary' is used. See linkgit:gitattribute[5].\n+\n gitcvs.allbinary::\n-\tIf true, all files are sent to the client in mode '-kb'. This\n-\tcauses the client to treat all files as binary files which suppresses\n-\tany newline munging it otherwise might do. A work-around for the\n-\tfact that there is no way yet to set single files to mode '-kb'.\n+\tIf true, all files not otherwise specified using\n+\t'gitcvs.usecrlfattr' and an explicitly set or unset `crlf`\n+\tattribute are sent to the client in mode '-kb'. This\n+\tcauses the client to treat them as binary files which\n+\tsuppresses any newline munging it otherwise might do.\n \n gitcvs.dbname::\n \tDatabase used by git-cvsserver to cache revision information\n@@ -680,8 +690,9 @@ gitcvs.dbTableNamePrefix::\n \tlinkgit:git-cvsserver[1] for details).  Any non-alphabetic\n \tcharacters will be replaced with underscores.\n \n-All gitcvs variables except for 'gitcvs.allbinary' can also be\n-specified as 'gitcvs.<access_method>.<varname>' (where 'access_method'\n+All gitcvs variables except for 'gitcvs.usecrlfattr' and\n+'gitcvs.allbinary' can also be specified as\n+'gitcvs.<access_method>.<varname>' (where 'access_method'\n is one of \"ext\" and \"pserver\") to make them apply only for the given\n access method.\n \ndiff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\nindex b110671..8393028 100644\n--- a/Documentation/git-cvsserver.txt\n+++ b/Documentation/git-cvsserver.txt\n@@ -301,11 +301,27 @@ checkout, diff, status, update, log, add, remove, commit.\n Legacy monitoring operations are not supported (edit, watch and related).\n Exports and tagging (tags and branches) are not supported at this stage.\n \n-The server should set the '-k' mode to binary when relevant, however,\n-this is not really implemented yet. For now, you can force the server\n-to set '-kb' for all files by setting the `gitcvs.allbinary` config\n-variable. In proper GIT tradition, the contents of the files are\n-always respected. No keyword expansion or newline munging is supported.\n+CRLF Line Ending Conversions\n+^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n+\n+By default the server leaves the '-k' mode blank for all files,\n+which causes the cvs client to treat them as a text files, subject\n+to crnl conversion on some platforms.\n+\n+You can make the server use `crnl` attributes to set the '-k' modes\n+for files by setting the `gitcvs.usecrlfattr` config variable.\n+In this case, if `crlf` is explicitly unset ('-crnl'), then the\n+will set '-kb' mode, for binary files.  If it `crlf` is set,\n+then the '-k' mode will explicitly be left blank.  See\n+also linkgit:gitattributes[5] for more information about the `crlf`\n+attribute.\n+\n+Alternatively, if `gitcvs.usecrlfattr` config is not enabled\n+or if the `crlf` attribute is unspecified for a filename, then\n+the server uses the `gitcvs.allbinary` for the default setting.\n+If `gitcvs.allbinary` is set, then the files not otherwise\n+specified will default to '-kb' mode. Otherwise the '-k' mode\n+is left blank.\n \n Dependencies\n ------------\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 674892b..58206ae 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -502,7 +502,7 @@ sub req_add\n                 print $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n \n                 # this is an \"entries\" line\n-                my $kopts = kopts_from_path($filepart);\n+                my $kopts = kopts_from_path($filename);\n                 $log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n                 print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n                 # permissions\n@@ -533,9 +533,25 @@ sub req_add\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        my $kopts = kopts_from_path($filepart);\n+        my $kopts = kopts_from_path($filename);\n         print \"/$filepart/0//$kopts/\\n\";\n \n+        my $requestedKopts = $state->{opt}{k};\n+        if(defined($requestedKopts))\n+        {\n+            $requestedKopts = \"-k$requestedKopts\";\n+        }\n+        else\n+        {\n+            $requestedKopts = \"\";\n+        }\n+        if( $kopts ne $requestedKopts )\n+        {\n+            $log->warn(\"Ignoring requested -k='$requestedKopts'\"\n+                        . \" for '$filename'; detected -k='$kopts' instead\");\n+            #TODO: Also have option to send warning to user?\n+        }\n+\n         $addcount++;\n     }\n \n@@ -615,7 +631,7 @@ sub req_remove\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        my $kopts = kopts_from_path($filepart);\n+        my $kopts = kopts_from_path($filename);\n         print \"/$filepart/-1.$wrev//$kopts/\\n\";\n \n         $rmcount++;\n@@ -785,6 +801,7 @@ sub req_co\n     argsplit(\"co\");\n \n     my $module = $state->{args}[0];\n+    $state->{module} = $module;\n     my $checkout_path = $module;\n \n     # use the user specified directory if we're given it\n@@ -862,6 +879,7 @@ sub req_co\n         # Don't want to check out deleted files\n         next if ( $git->{filehash} eq \"deleted\" );\n \n+        my $fullName = $git->{name};\n         ( $git->{name}, $git->{dir} ) = filenamesplit($git->{name});\n \n        if (length($git->{dir}) && $git->{dir} ne './'\n@@ -892,7 +910,7 @@ 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-        my $kopts = kopts_from_path($git->{name});\n+        my $kopts = kopts_from_path($fullName);\n         print \"/$git->{name}/1.$git->{revision}//$kopts/\\n\";\n         # permissions\n         print \"u=$git->{mode},g=$git->{mode},o=$git->{mode}\\n\";\n@@ -1101,7 +1119,7 @@ sub req_update\n \t\tprint $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n \n \t\t# this is an \"entries\" line\n-\t\tmy $kopts = kopts_from_path($filepart);\n+\t\tmy $kopts = kopts_from_path($filename);\n \t\t$log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n \t\tprint \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n \n@@ -1149,7 +1167,7 @@ sub req_update\n                     print \"Merged $dirpart\\n\";\n                     $log->debug($state->{CVSROOT} . \"/$state->{module}/$filename\");\n                     print $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    my $kopts = kopts_from_path($filepart);\n+                    my $kopts = kopts_from_path(\"$dirpart/$filepart\");\n                     $log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n                     print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n                 }\n@@ -1165,7 +1183,7 @@ sub req_update\n                 {\n                     print \"Merged $dirpart\\n\";\n                     print $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    my $kopts = kopts_from_path($filepart);\n+                    my $kopts = kopts_from_path(\"$dirpart/$filepart\");\n                     print \"/$filepart/1.$meta->{revision}/+/$kopts/\\n\";\n                 }\n             }\n@@ -1416,7 +1434,7 @@ sub req_ci\n             }\n             print \"Checked-in $dirpart\\n\";\n             print \"$filename\\n\";\n-            my $kopts = kopts_from_path($filepart);\n+            my $kopts = kopts_from_path($filename);\n             print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n         }\n     }\n@@ -2296,10 +2314,24 @@ 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+    if ( defined ( $cfg->{gitcvs}{usecrlfattr} ) and\n+         $cfg->{gitcvs}{usecrlfattr} =~ /\\s*(1|true|yes)\\s*$/i )\n+    {\n+        my ($val) = check_attr( \"crlf\", $path );\n+        if ( $val eq \"set\" )\n+        {\n+            return \"\";\n+        }\n+        elsif ( $val eq \"unset\" )\n+        {\n+            return \"-kb\"\n+        }\n+        else\n+        {\n+            $log->info(\"Unrecognized check_attr crlf $path : $val\");\n+        }\n+    }\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@@ -2311,6 +2343,23 @@ sub kopts_from_path\n     }\n }\n \n+sub check_attr\n+{\n+    my ($attr,$path) = @_;\n+    ensureWorkTree();\n+    if ( open my $fh, '-|', \"git\", \"check-attr\", $attr, \"--\", $path )\n+    {\n+        my $val = <$fh>;\n+        close $fh;\n+        $val =~ s/.*: ([^:\\r\\n]*)\\s*$/$1/;\n+        return $val;\n+    }\n+    else\n+    {\n+        return undef;\n+    }\n+}\n+\n # Generate a CVS author name from Git author information, by taking\n # the first eight characters of the user part of the email address.\n sub cvs_author\ndiff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh\nnew file mode 100755\nindex 0000000..b7a779b\n--- /dev/null\n+++ b/t/t9401-git-cvsserver-crlf.sh\n@@ -0,0 +1,178 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Matthew Ogilvie\n+# Parts adapted from other tests.\n+#\n+\n+test_description='git-cvsserver -kb modes\n+\n+tests -kb mode for binary files when accessing a git\n+repository using cvs CLI client via git-cvsserver server'\n+\n+. ./test-lib.sh\n+\n+q_to_nul () {\n+    perl -pe 'y/Q/\\000/'\n+}\n+\n+q_to_cr () {\n+    tr Q '\\015'\n+}\n+\n+marked_as () {\n+    foundEntry=\"$(grep \"^/$2/\" \"$1/CVS/Entries\")\"\n+    if [ x\"$foundEntry\" = x\"\" ] ; then\n+       echo \"NOT FOUND: $1 $2 1 $3\" >> \"${WORKDIR}/marked.log\"\n+       return 1\n+    fi\n+    test x\"$(grep \"^/$2/\" \"$1/CVS/Entries\" | cut -d/ -f5)\" = x\"$3\"\n+    stat=$?\n+    echo \"$1 $2 $stat '$3'\" >> \"${WORKDIR}/marked.log\"\n+    return $stat\n+}\n+\n+not_present() {\n+    foundEntry=\"$(grep \"^/$2/\" \"$1/CVS/Entries\")\"\n+    if [ -r \"$1/$2\" ] ; then\n+        echo \"Error: File still exists: $1 $2\" >> \"${WORKDIR}/marked.log\"\n+        return 1;\n+    fi\n+    if [ x\"$foundEntry\" != x\"\" ] ; then\n+        echo \"Error: should not have found: $1 $2\" >> \"${WORKDIR}/marked.log\"\n+        return 1;\n+    else\n+        echo \"Correctly not found: $1 $2\" >> \"${WORKDIR}/marked.log\"\n+        return 0;\n+    fi\n+}\n+\n+cvs >/dev/null 2>&1\n+if test $? -ne 1\n+then\n+    test_expect_success 'skipping git-cvsserver tests, cvs not found' :\n+    test_done\n+    exit\n+fi\n+perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || {\n+    test_expect_success 'skipping git-cvsserver tests, Perl SQLite interface unavailable' :\n+    test_done\n+    exit\n+}\n+\n+unset GIT_DIR GIT_CONFIG\n+WORKDIR=$(pwd)\n+SERVERDIR=$(pwd)/gitcvs.git\n+git_config=\"$SERVERDIR/config\"\n+CVSROOT=\":fork:$SERVERDIR\"\n+CVSWORK=\"$(pwd)/cvswork\"\n+CVS_SERVER=git-cvsserver\n+export CVSROOT CVS_SERVER\n+\n+rm -rf \"$CVSWORK\" \"$SERVERDIR\"\n+test_expect_success 'setup' '\n+    echo \"Simple text file\" >textfile.c &&\n+    echo \"File with embedded NUL: Q <- there\" | q_to_nul > binfile.bin &&\n+    mkdir subdir &&\n+    echo \"Another text file\" > subdir/file.h &&\n+    echo \"Another binary: Q (this time CR)\" | q_to_cr > subdir/withCr.bin &&\n+    echo \"Mixed up NUL, but marked text: Q <- there\" | q_to_nul > mixedUp.c\n+    echo \"Unspecified\" > subdir/unspecified.other &&\n+    echo \"/*.bin -crlf\" > .gitattributes &&\n+    echo \"/*.c crlf\" >> .gitattributes &&\n+    echo \"subdir/*.bin -crlf\" >> .gitattributes &&\n+    echo \"subdir/*.c crlf\" >> .gitattributes &&\n+    echo \"subdir/file.h crlf\" >> .gitattributes &&\n+    git add .gitattributes textfile.c binfile.bin mixedUp.c subdir/* &&\n+    git commit -q -m \"First Commit\" &&\n+    git clone -q --local --bare \"$WORKDIR/.git\" \"$SERVERDIR\" >/dev/null 2>&1 &&\n+    GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.enabled true &&\n+    GIT_DIR=\"$SERVERDIR\" git config gitcvs.logfile \"$SERVERDIR/gitcvs.log\"\n+'\n+\n+test_expect_success 'cvs co (default crlf)' '\n+    GIT_CONFIG=\"$git_config\" cvs -Q co -d cvswork master >cvs.log 2>&1 &&\n+    test x\"$(grep '/-k' cvswork/CVS/Entries cvswork/subdir/CVS/Entries)\" = x\"\"\n+'\n+\n+rm -rf cvswork\n+test_expect_success 'cvs co (allbinary)' '\n+    GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.allbinary true &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q co -d cvswork master >cvs.log 2>&1 &&\n+    marked_as cvswork textfile.c -kb &&\n+    marked_as cvswork binfile.bin -kb &&\n+    marked_as cvswork .gitattributes -kb &&\n+    marked_as cvswork mixedUp.c -kb &&\n+    marked_as cvswork/subdir withCr.bin -kb &&\n+    marked_as cvswork/subdir file.h -kb &&\n+    marked_as cvswork/subdir unspecified.other -kb\n+'\n+\n+rm -rf cvswork cvs.log\n+test_expect_success 'cvs co (use attributes/allbinary)' '\n+    GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.usecrlfattr true &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q co -d cvswork master >cvs.log 2>&1 &&\n+    marked_as cvswork textfile.c \"\" &&\n+    marked_as cvswork binfile.bin -kb &&\n+    marked_as cvswork .gitattributes -kb &&\n+    marked_as cvswork mixedUp.c \"\" &&\n+    marked_as cvswork/subdir withCr.bin -kb &&\n+    marked_as cvswork/subdir file.h \"\" &&\n+    marked_as cvswork/subdir unspecified.other -kb\n+'\n+\n+rm -rf cvswork\n+test_expect_success 'cvs co (use attributes)' '\n+    GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.allbinary false &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q co -d cvswork master >cvs.log 2>&1 &&\n+    marked_as cvswork textfile.c \"\" &&\n+    marked_as cvswork binfile.bin -kb &&\n+    marked_as cvswork .gitattributes \"\" &&\n+    marked_as cvswork mixedUp.c \"\" &&\n+    marked_as cvswork/subdir withCr.bin -kb &&\n+    marked_as cvswork/subdir file.h \"\" &&\n+    marked_as cvswork/subdir unspecified.other \"\"\n+'\n+\n+test_expect_success 'adding files' '\n+    cd cvswork/subdir &&\n+    echo \"more text\" > src.c &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q add src.c >cvs.log 2>&1 &&\n+    marked_as . src.c \"\" &&\n+    echo \"psuedo-binary\" > temp.bin &&\n+    cd .. &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q add subdir/temp.bin >cvs.log 2>&1 &&\n+    marked_as subdir temp.bin \"-kb\" &&\n+    cd subdir &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q ci -m \"adding files\" >cvs.log 2>&1 &&\n+    marked_as . temp.bin \"-kb\" &&\n+    marked_as . src.c \"\"\n+'\n+\n+cd \"$WORKDIR\"\n+test_expect_success 'updating' '\n+    git pull gitcvs.git &&\n+    echo 'hi' > subdir/newfile.bin &&\n+    echo 'junk' > subdir/file.h &&\n+    echo 'hi' > subdir/newfile.c &&\n+    echo 'hello' >> binfile.bin &&\n+    git add subdir/newfile.bin subdir/file.h subdir/newfile.c binfile.bin &&\n+    git commit -q -m \"Add and change some files\" &&\n+    git push gitcvs.git >/dev/null &&\n+    cd cvswork &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q update &&\n+    cd .. &&\n+    marked_as cvswork textfile.c \"\" &&\n+    marked_as cvswork binfile.bin -kb &&\n+    marked_as cvswork .gitattributes \"\" &&\n+    marked_as cvswork mixedUp.c \"\" &&\n+    marked_as cvswork/subdir withCr.bin -kb &&\n+    marked_as cvswork/subdir file.h \"\" &&\n+    marked_as cvswork/subdir unspecified.other \"\" &&\n+    marked_as cvswork/subdir newfile.bin -kb &&\n+    marked_as cvswork/subdir newfile.c \"\" &&\n+    echo \"File with embedded NUL: Q <- there\" | q_to_nul > tmpExpect1 &&\n+    echo \"hello\" >> tmpExpect1 &&\n+    cmp cvswork/binfile.bin tmpExpect1\n+'\n+\n+test_done\n-- \n1.5.4.3.340.g97b97\n"},{"id":"77018","messageId":"1210826148-8708-4-git-send-email-mmogilvi_git@miniinfo.net","threadId":"13526","inReplyTo":"1210826148-8708-3-git-send-email-mmogilvi_git@miniinfo.net","subject":"[PATCH 3/3] git-cvsserver: add ability to guess -kb from contents","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2008-05-15T04:35:48Z","receivedAt":"2008-05-15T04:35:48Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"If \"gitcvs.allbinary\" is set to \"guess\", then any file that has\nnot been explicitly marked as binary or text using the \"crlf\" attribute\nand the \"gitcvs.usecrlfattr\" config will guess binary based on the contents\nof the file.\n\nSigned-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>\n---\n Documentation/config.txt        |   13 ++-\n Documentation/git-cvsserver.txt |   14 ++-\n git-cvsserver.perl              |  193 +++++++++++++++++++++++++++++++++++---\n t/t9401-git-cvsserver-crlf.sh   |  159 ++++++++++++++++++++++++++++++++\n 4 files changed, 354 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 820795f..2c867d3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -655,11 +655,14 @@ gitcvs.usecrlfattr\n \tthen 'gitcvs.allbinary' is used. See linkgit:gitattribute[5].\n \n gitcvs.allbinary::\n-\tIf true, all files not otherwise specified using\n-\t'gitcvs.usecrlfattr' and an explicitly set or unset `crlf`\n-\tattribute are sent to the client in mode '-kb'. This\n-\tcauses the client to treat them as binary files which\n-\tsuppresses any newline munging it otherwise might do.\n+\tThis is used if 'gitcvs.usecrlfattr' does not resolve\n+\tthe correct '-kb' mode to use. If true, all\n+\tunresolved files are sent to the client in\n+\tmode '-kb'. This causes the client to treat them\n+\tas binary files, which suppresses any newline munging it\n+\totherwise might do. Alternatively, if it is set to \"guess\",\n+\tthen the contents of the file are examined to decide if\n+\tit is binary, similar to 'core.autocrlf'.\n \n gitcvs.dbname::\n \tDatabase used by git-cvsserver to cache revision information\ndiff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\nindex 8393028..7611c5b 100644\n--- a/Documentation/git-cvsserver.txt\n+++ b/Documentation/git-cvsserver.txt\n@@ -311,17 +311,23 @@ to crnl conversion on some platforms.\n You can make the server use `crnl` attributes to set the '-k' modes\n for files by setting the `gitcvs.usecrlfattr` config variable.\n In this case, if `crlf` is explicitly unset ('-crnl'), then the\n-will set '-kb' mode, for binary files.  If it `crlf` is set,\n+server will set '-kb' mode for binary files. If `crlf` is set,\n then the '-k' mode will explicitly be left blank.  See\n also linkgit:gitattributes[5] for more information about the `crlf`\n attribute.\n \n Alternatively, if `gitcvs.usecrlfattr` config is not enabled\n or if the `crlf` attribute is unspecified for a filename, then\n-the server uses the `gitcvs.allbinary` for the default setting.\n-If `gitcvs.allbinary` is set, then the files not otherwise\n+the server uses the `gitcvs.allbinary` config for the default setting.\n+If `gitcvs.allbinary` is set, then file not otherwise\n specified will default to '-kb' mode. Otherwise the '-k' mode\n-is left blank.\n+is left blank. But if `gitcvs.allbinary` is set to \"guess\", then\n+the correct '-k' mode will be guessed based on the contents of\n+the file.\n+\n+For best consistency with cvs, it is probably best to override the\n+defaults by setting `gitcvs.usecrlfattr` to true,\n+and `gitcvs.allbinary` to \"guess\".\n \n Dependencies\n ------------\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 58206ae..920bbe1 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -502,7 +502,7 @@ sub req_add\n                 print $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n \n                 # this is an \"entries\" line\n-                my $kopts = kopts_from_path($filename);\n+                my $kopts = kopts_from_path($filename,\"sha1\",$meta->{filehash});\n                 $log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n                 print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n                 # permissions\n@@ -533,7 +533,8 @@ sub req_add\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        my $kopts = kopts_from_path($filename);\n+        my $kopts = kopts_from_path($filename,\"file\",\n+                        $state->{entries}{$filename}{modified_filename});\n         print \"/$filepart/0//$kopts/\\n\";\n \n         my $requestedKopts = $state->{opt}{k};\n@@ -631,7 +632,7 @@ sub req_remove\n \n         print \"Checked-in $dirpart\\n\";\n         print \"$filename\\n\";\n-        my $kopts = kopts_from_path($filename);\n+        my $kopts = kopts_from_path($filename,\"sha1\",$meta->{filehash});\n         print \"/$filepart/-1.$wrev//$kopts/\\n\";\n \n         $rmcount++;\n@@ -910,7 +911,7 @@ 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-        my $kopts = kopts_from_path($fullName);\n+        my $kopts = kopts_from_path($fullName,\"sha1\",$git->{filehash});\n         print \"/$git->{name}/1.$git->{revision}//$kopts/\\n\";\n         # permissions\n         print \"u=$git->{mode},g=$git->{mode},o=$git->{mode}\\n\";\n@@ -1119,7 +1120,7 @@ sub req_update\n \t\tprint $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n \n \t\t# this is an \"entries\" line\n-\t\tmy $kopts = kopts_from_path($filename);\n+\t\tmy $kopts = kopts_from_path($filename,\"sha1\",$meta->{filehash});\n \t\t$log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n \t\tprint \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n \n@@ -1167,7 +1168,8 @@ sub req_update\n                     print \"Merged $dirpart\\n\";\n                     $log->debug($state->{CVSROOT} . \"/$state->{module}/$filename\");\n                     print $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    my $kopts = kopts_from_path(\"$dirpart/$filepart\");\n+                    my $kopts = kopts_from_path(\"$dirpart/$filepart\",\n+                                                \"file\",$mergedFile);\n                     $log->debug(\"/$filepart/1.$meta->{revision}//$kopts/\");\n                     print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n                 }\n@@ -1183,7 +1185,8 @@ sub req_update\n                 {\n                     print \"Merged $dirpart\\n\";\n                     print $state->{CVSROOT} . \"/$state->{module}/$filename\\n\";\n-                    my $kopts = kopts_from_path(\"$dirpart/$filepart\");\n+                    my $kopts = kopts_from_path(\"$dirpart/$filepart\",\n+                                                \"file\",$mergedFile);\n                     print \"/$filepart/1.$meta->{revision}/+/$kopts/\\n\";\n                 }\n             }\n@@ -1434,7 +1437,7 @@ sub req_ci\n             }\n             print \"Checked-in $dirpart\\n\";\n             print \"$filename\\n\";\n-            my $kopts = kopts_from_path($filename);\n+            my $kopts = kopts_from_path($filename,\"sha1\",$meta->{filehash});\n             print \"/$filepart/1.$meta->{revision}//$kopts/\\n\";\n         }\n     }\n@@ -2312,7 +2315,7 @@ sub cleanupTmpDir\n # file should get -kb.\n sub kopts_from_path\n {\n-\tmy ($path) = @_;\n+    my ($path, $srcType, $name) = @_;\n \n     if ( defined ( $cfg->{gitcvs}{usecrlfattr} ) and\n          $cfg->{gitcvs}{usecrlfattr} =~ /\\s*(1|true|yes)\\s*$/i )\n@@ -2332,15 +2335,55 @@ sub kopts_from_path\n         }\n     }\n \n-    unless ( defined ( $cfg->{gitcvs}{allbinary} ) and $cfg->{gitcvs}{allbinary} =~ /^\\s*(1|true|yes)\\s*$/i )\n+    if ( defined ( $cfg->{gitcvs}{allbinary} ) )\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+        if( ($cfg->{gitcvs}{allbinary} =~ /^\\s*(1|true|yes)\\s*$/i) )\n+        {\n+            return \"-kb\";\n+        }\n+        elsif( ($cfg->{gitcvs}{allbinary} =~ /^\\s*guess\\s*$/i) )\n+        {\n+            if( $srcType eq \"sha1Or-k\" &&\n+                !defined($name) )\n+            {\n+                my ($ret)=$state->{entries}{$path}{options};\n+                if( !defined($ret) )\n+                {\n+                    $ret=$state->{opt}{k};\n+                    if(defined($ret))\n+                    {\n+                        $ret=\"-k$ret\";\n+                    }\n+                    else\n+                    {\n+                        $ret=\"\";\n+                    }\n+                }\n+                if( ! ($ret=~/^(|-kb|-kkv|-kkvl|-kk|-ko|-kv)$/) )\n+                {\n+                    print \"E Bad -k option\\n\";\n+                    $log->warn(\"Bad -k option: $ret\");\n+                    die \"Error: Bad -k option: $ret\\n\";\n+                }\n+\n+                return $ret;\n+            }\n+            else\n+            {\n+                if( is_binary($srcType,$name) )\n+                {\n+                    $log->debug(\"... as binary\");\n+                    return \"-kb\";\n+                }\n+                else\n+                {\n+                    $log->debug(\"... as text\");\n+                }\n+            }\n+        }\n     }\n+    # Return \"\" to give no special treatment to any path\n+    return \"\";\n }\n \n sub check_attr\n@@ -2360,6 +2403,124 @@ sub check_attr\n     }\n }\n \n+# This should have the same heuristics as convert.c:is_binary() and related.\n+# Note that the bare CR test is done by callers in convert.c.\n+sub is_binary\n+{\n+    my ($srcType,$name) = @_;\n+    $log->debug(\"is_binary($srcType,$name)\");\n+\n+    # Minimize amount of interpreted code run in the inner per-character\n+    # loop for large files, by totalling each character value and\n+    # then analyzing the totals.\n+    my @counts;\n+    my $i;\n+    for($i=0;$i<256;$i++)\n+    {\n+        $counts[$i]=0;\n+    }\n+\n+    my $fh = open_blob_or_die($srcType,$name);\n+    my $line;\n+    while( defined($line=<$fh>) )\n+    {\n+        # Any '\\0' and bare CR are considered binary.\n+        if( $line =~ /\\0|(\\r[^\\n])/ )\n+        {\n+            close($fh);\n+            return 1;\n+        }\n+\n+        # Count up each character in the line:\n+        my $len=length($line);\n+        for($i=0;$i<$len;$i++)\n+        {\n+            $counts[ord(substr($line,$i,1))]++;\n+        }\n+    }\n+    close $fh;\n+\n+    # Don't count CR and LF as either printable/nonprintable\n+    $counts[ord(\"\\n\")]=0;\n+    $counts[ord(\"\\r\")]=0;\n+\n+    # Categorize individual character count into printable and nonprintable:\n+    my $printable=0;\n+    my $nonprintable=0;\n+    for($i=0;$i<256;$i++)\n+    {\n+        if( $i < 32 &&\n+            $i != ord(\"\\b\") &&\n+            $i != ord(\"\\t\") &&\n+            $i != 033 &&       # ESC\n+            $i != 014 )        # FF\n+        {\n+            $nonprintable+=$counts[$i];\n+        }\n+        elsif( $i==127 )  # DEL\n+        {\n+            $nonprintable+=$counts[$i];\n+        }\n+        else\n+        {\n+            $printable+=$counts[$i];\n+        }\n+    }\n+\n+    return ($printable >> 7) < $nonprintable;\n+}\n+\n+# Returns open file handle.  Possible invocations:\n+#  - open_blob_or_die(\"file\",$filename);\n+#  - open_blob_or_die(\"sha1\",$filehash);\n+sub open_blob_or_die\n+{\n+    my ($srcType,$name) = @_;\n+    my ($fh);\n+    if( $srcType eq \"file\" )\n+    {\n+        if( !open $fh,\"<\",$name )\n+        {\n+            $log->warn(\"Unable to open file $name: $!\");\n+            die \"Unable to open file $name: $!\\n\";\n+        }\n+    }\n+    elsif( $srcType eq \"sha1\" || $srcType eq \"sha1Or-k\" )\n+    {\n+        unless ( defined ( $name ) and $name =~ /^[a-zA-Z0-9]{40}$/ )\n+        {\n+            $log->warn(\"Need filehash\");\n+            die \"Need filehash\\n\";\n+        }\n+\n+        my $type = `git cat-file -t $name`;\n+        chomp $type;\n+\n+        unless ( defined ( $type ) and $type eq \"blob\" )\n+        {\n+            $log->warn(\"Invalid type '$type' for '$name'\");\n+            die ( \"Invalid type '$type' (expected 'blob')\" )\n+        }\n+\n+        my $size = `git cat-file -s $name`;\n+        chomp $size;\n+\n+        $log->debug(\"open_blob_or_die($name) size=$size, type=$type\");\n+\n+        unless( open $fh, '-|', \"git\", \"cat-file\", \"blob\", $name )\n+        {\n+            $log->warn(\"Unable to open sha1 $name\");\n+            die \"Unable to open sha1 $name\\n\";\n+        }\n+    }\n+    else\n+    {\n+        $log->warn(\"Unknown type of blob source: $srcType\");\n+        die \"Unknown type of blob source: $srcType\\n\";\n+    }\n+    return $fh;\n+}\n+\n # Generate a CVS author name from Git author information, by taking\n # the first eight characters of the user part of the email address.\n sub cvs_author\ndiff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh\nindex b7a779b..e27a1c5 100755\n--- a/t/t9401-git-cvsserver-crlf.sh\n+++ b/t/t9401-git-cvsserver-crlf.sh\n@@ -175,4 +175,163 @@ test_expect_success 'updating' '\n     cmp cvswork/binfile.bin tmpExpect1\n '\n \n+rm -rf cvswork\n+test_expect_success 'cvs co (use attributes/guess)' '\n+    GIT_DIR=\"$SERVERDIR\" git config gitcvs.allbinary guess &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q co -d cvswork master >cvs.log 2>&1 &&\n+    marked_as cvswork textfile.c \"\" &&\n+    marked_as cvswork binfile.bin -kb &&\n+    marked_as cvswork .gitattributes \"\" &&\n+    marked_as cvswork mixedUp.c \"\" &&\n+    marked_as cvswork/subdir withCr.bin -kb &&\n+    marked_as cvswork/subdir file.h \"\" &&\n+    marked_as cvswork/subdir unspecified.other \"\" &&\n+    marked_as cvswork/subdir newfile.bin -kb &&\n+    marked_as cvswork/subdir newfile.c \"\"\n+'\n+\n+test_expect_success 'setup multi-line files' '\n+    ( echo \"line 1\" &&\n+      echo \"line 2\" &&\n+      echo \"line 3\" &&\n+      echo \"line 4 with NUL: Q <-\" ) | q_to_nul > multiline.c &&\n+    git add multiline.c &&\n+    ( echo \"line 1\" &&\n+      echo \"line 2\" &&\n+      echo \"line 3\" &&\n+      echo \"line 4\" ) | q_to_nul > multilineTxt.c &&\n+    git add multilineTxt.c &&\n+    git commit -q -m \"multiline files\" &&\n+    git push gitcvs.git >/dev/null\n+'\n+\n+rm -rf cvswork\n+test_expect_success 'cvs co (guess)' '\n+    GIT_DIR=\"$SERVERDIR\" git config --bool gitcvs.usecrlfattr false &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q co -d cvswork master >cvs.log 2>&1 &&\n+    marked_as cvswork textfile.c \"\" &&\n+    marked_as cvswork binfile.bin -kb &&\n+    marked_as cvswork .gitattributes \"\" &&\n+    marked_as cvswork mixedUp.c -kb &&\n+    marked_as cvswork multiline.c -kb &&\n+    marked_as cvswork multilineTxt.c \"\" &&\n+    marked_as cvswork/subdir withCr.bin -kb &&\n+    marked_as cvswork/subdir file.h \"\" &&\n+    marked_as cvswork/subdir unspecified.other \"\" &&\n+    marked_as cvswork/subdir newfile.bin \"\" &&\n+    marked_as cvswork/subdir newfile.c \"\"\n+'\n+\n+test_expect_success 'cvs co another copy (guess)' '\n+    GIT_CONFIG=\"$git_config\" cvs -Q co -d cvswork2 master >cvs.log 2>&1 &&\n+    marked_as cvswork2 textfile.c \"\" &&\n+    marked_as cvswork2 binfile.bin -kb &&\n+    marked_as cvswork2 .gitattributes \"\" &&\n+    marked_as cvswork2 mixedUp.c -kb &&\n+    marked_as cvswork2 multiline.c -kb &&\n+    marked_as cvswork2 multilineTxt.c \"\" &&\n+    marked_as cvswork2/subdir withCr.bin -kb &&\n+    marked_as cvswork2/subdir file.h \"\" &&\n+    marked_as cvswork2/subdir unspecified.other \"\" &&\n+    marked_as cvswork2/subdir newfile.bin \"\" &&\n+    marked_as cvswork2/subdir newfile.c \"\"\n+'\n+\n+test_expect_success 'add text (guess)' '\n+    cd cvswork &&\n+    echo \"simpleText\" > simpleText.c &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q add simpleText.c &&\n+    cd .. &&\n+    marked_as cvswork simpleText.c \"\"\n+'\n+\n+test_expect_success 'add bin (guess)' '\n+    cd cvswork &&\n+    echo \"simpleBin: NUL: Q <- there\" | q_to_nul > simpleBin.bin &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q add simpleBin.bin &&\n+    cd .. &&\n+    marked_as cvswork simpleBin.bin -kb\n+'\n+\n+test_expect_success 'remove files (guess)' '\n+    cd cvswork &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q rm -f subdir/file.h &&\n+    cd subdir &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q rm -f withCr.bin &&\n+    cd ../.. &&\n+    marked_as cvswork/subdir withCr.bin -kb &&\n+    marked_as cvswork/subdir file.h \"\"\n+'\n+\n+test_expect_success 'cvs ci (guess)' '\n+    cd cvswork &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q ci -m \"add/rm files\" >cvs.log 2>&1 &&\n+    cd .. &&\n+    marked_as cvswork textfile.c \"\" &&\n+    marked_as cvswork binfile.bin -kb &&\n+    marked_as cvswork .gitattributes \"\" &&\n+    marked_as cvswork mixedUp.c -kb &&\n+    marked_as cvswork multiline.c -kb &&\n+    marked_as cvswork multilineTxt.c \"\" &&\n+    not_present cvswork/subdir withCr.bin &&\n+    not_present cvswork/subdir file.h &&\n+    marked_as cvswork/subdir unspecified.other \"\" &&\n+    marked_as cvswork/subdir newfile.bin \"\" &&\n+    marked_as cvswork/subdir newfile.c \"\" &&\n+    marked_as cvswork simpleBin.bin -kb &&\n+    marked_as cvswork simpleText.c \"\"\n+'\n+\n+test_expect_success 'update subdir of other copy (guess)' '\n+    cd cvswork2/subdir &&\n+    GIT_CONFIG=\"$git_config\" cvs -Q update &&\n+    cd ../.. &&\n+    marked_as cvswork2 textfile.c \"\" &&\n+    marked_as cvswork2 binfile.bin -kb &&\n+    marked_as cvswork2 .gitattributes \"\" &&\n+    marked_as cvswork2 mixedUp.c -kb &&\n+    marked_as cvswork2 multiline.c -kb &&\n+    marked_as cvswork2 multilineTxt.c \"\" &&\n+    not_present cvswork2/subdir withCr.bin &&\n+    not_present cvswork2/subdir file.h &&\n+    marked_as cvswork2/subdir unspecified.other \"\" &&\n+    marked_as cvswork2/subdir newfile.bin \"\" &&\n+    marked_as cvswork2/subdir newfile.c \"\" &&\n+    not_present cvswork2 simpleBin.bin &&\n+    not_present cvswork2 simpleText.c\n+'\n+\n+echo \"starting update/merge\" >> \"${WORKDIR}/marked.log\"\n+test_expect_success 'update/merge full other copy (guess)' '\n+    git pull gitcvs.git master &&\n+    sed \"s/3/replaced_3/\" < multilineTxt.c > ml.temp &&\n+    mv ml.temp multilineTxt.c &&\n+    git add multilineTxt.c &&\n+    git commit -q -m \"modify multiline file\" >> \"${WORKDIR}/marked.log\" &&\n+    git push gitcvs.git >/dev/null &&\n+    cd cvswork2 &&\n+    sed \"s/1/replaced_1/\" < multilineTxt.c > ml.temp &&\n+    mv ml.temp multilineTxt.c &&\n+    GIT_CONFIG=\"$git_config\" cvs update > cvs.log 2>&1 &&\n+    cd .. &&\n+    marked_as cvswork2 textfile.c \"\" &&\n+    marked_as cvswork2 binfile.bin -kb &&\n+    marked_as cvswork2 .gitattributes \"\" &&\n+    marked_as cvswork2 mixedUp.c -kb &&\n+    marked_as cvswork2 multiline.c -kb &&\n+    marked_as cvswork2 multilineTxt.c \"\" &&\n+    not_present cvswork2/subdir withCr.bin &&\n+    not_present cvswork2/subdir file.h &&\n+    marked_as cvswork2/subdir unspecified.other \"\" &&\n+    marked_as cvswork2/subdir newfile.bin \"\" &&\n+    marked_as cvswork2/subdir newfile.c \"\" &&\n+    marked_as cvswork2 simpleBin.bin -kb &&\n+    marked_as cvswork2 simpleText.c \"\" &&\n+    echo \"line replaced_1\" > tmpExpect2 &&\n+    echo \"line 2\" >> tmpExpect2 &&\n+    echo \"line replaced_3\" >> tmpExpect2 &&\n+    echo \"line 4\" | q_to_nul >> tmpExpect2 &&\n+    cmp cvswork2/multilineTxt.c tmpExpect2\n+'\n+\n test_done\n-- \n1.5.4.3.340.g97b97\n"},{"id":"77165","messageId":"7v7idteqzn.fsf@gitster.siamese.dyndns.org","threadId":"13526","inReplyTo":"1210826148-8708-1-git-send-email-mmogilvi_git@miniinfo.net","subject":"Re: [PATCH 0/3] git-cvsserver: Add support for some binary files","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2008-05-17T00:03:40Z","receivedAt":"2008-05-17T00:03:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:\n\n> This series of patches extends git-cvsserver to support telling the\n> CVS client to set the -kb (binary) mode for files that git considers\n> to be binary (and not for text files).  It includes updates to\n> documentation and tests.\n\nI am unfortunately not familiar with this part of the system and I'd need\nto summon help from experts, but it looks rather nicely done.\n\nI saw a few places that said \"crnl\" instead of \"crlf\" in the\ndocumentation, which I munged locally before queuing.\n\nI noticed kopts_from_path in patch 3/3 takes $srcType of \"sha1Or-k\" but I\ncould not spot which caller gives such token to the function.\n"},{"id":"77239","messageId":"20080518221053.GA880@comcast.net","threadId":"13526","inReplyTo":"7v7idteqzn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/3] git-cvsserver: Add support for some binary files","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2008-05-18T22:10:54Z","receivedAt":"2008-05-18T22:10:54Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"On Fri, May 16, 2008 at 05:03:40PM -0700, Junio C Hamano wrote:\n> Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:\n> \n> > This series of patches extends git-cvsserver to support telling the\n> > CVS client to set the -kb (binary) mode for files that git considers\n> > to be binary (and not for text files).  It includes updates to\n> > documentation and tests.\n> \n> I am unfortunately not familiar with this part of the system and I'd need\n> to summon help from experts, but it looks rather nicely done.\n> \n> I saw a few places that said \"crnl\" instead of \"crlf\" in the\n> documentation, which I munged locally before queuing.\n\nSounds good.\n\n> \n> I noticed kopts_from_path in patch 3/3 takes $srcType of \"sha1Or-k\" but I\n> could not spot which caller gives such token to the function.\n\nOops.  The \"sha1Or-k\" cases can and probably should be removed\ncompletely.\n\nI can generate another patch if you would like.\n\nIt's a remnant of an approach I had been working on earlier,\nwhen I thought there might be cases when I needed to fall back on\nthe -k option the user specified on the command line because I didn't\nhave the file contents.  But careful study revealed what I needed\nelsewhere in the data structures.\n\n--\nMatthew Ogilvie   [mmogilvi_git@miniinfo.net]\n"},{"id":"77241","messageId":"46a038f90805181538v56aee5b8y33d68b226a62494f@mail.gmail.com","threadId":"13526","inReplyTo":"7v7idteqzn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/3] git-cvsserver: Add support for some binary files","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2008-05-18T22:38:21Z","receivedAt":"2008-05-18T22:38:21Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Sat, May 17, 2008 at 12:03 PM, Junio C Hamano <junio@pobox.com> wrote:\n> Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:\n>\n>> This series of patches extends git-cvsserver to support telling the\n>> CVS client to set the -kb (binary) mode for files that git considers\n>> to be binary (and not for text files).  It includes updates to\n>> documentation and tests.\n>\n> I am unfortunately not familiar with this part of the system and I'd need\n> to summon help from experts, but it looks rather nicely done.\n\nLooks good.\n\nI was at first a bit troubled - \"cvsserver doesn't do keyword\nexpansion anyway\" was my first thought - but it makes sense to have\nthis to help newline-munging clients.\n\nIIRC, one thing that is _not_ handled well in CVS -k flag changes on\nthe server side (since -k modes are not versioned). If we are\nguessing, this may be more likely to happen, or at least more likely\nto _surprise_ people.\n\nMatthew, have you had a chance to test k mode changes against clients?\nAre we reasonably bug-compatible with the original turd^H^H^Hhing? ;-)\n\nSorry about the latency!\n\ncheers,\n\n\n\nm\n-- \n martin.langhoff@gmail.com\n martin@laptop.org -- School Server Architect\n - ask interesting questions\n - don't get distracted with shiny stuff - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"},{"id":"77259","messageId":"20080519073535.GA2885@comcast.net","threadId":"13526","inReplyTo":"46a038f90805181538v56aee5b8y33d68b226a62494f@mail.gmail.com","subject":"Re: [PATCH 0/3] git-cvsserver: Add support for some binary files","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2008-05-19T07:35:35Z","receivedAt":"2008-05-19T07:35:35Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"On Mon, May 19, 2008 at 10:38:21AM +1200, Martin Langhoff wrote:\n> On Sat, May 17, 2008 at 12:03 PM, Junio C Hamano <junio@pobox.com> wrote:\n> > Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:\n> >\n> >> This series of patches extends git-cvsserver to support telling the\n> >> CVS client to set the -kb (binary) mode for files that git considers\n> >> to be binary (and not for text files).  It includes updates to\n> >> documentation and tests.\n> >\n> > I am unfortunately not familiar with this part of the system and I'd need\n> > to summon help from experts, but it looks rather nicely done.\n> \n> IIRC, one thing that is _not_ handled well in CVS -k flag changes on\n> the server side (since -k modes are not versioned). If we are\n> guessing, this may be more likely to happen, or at least more likely\n> to _surprise_ people.\n\nSince git-cvsserver can (and does) refigure and send a different -k\nevery time it sends a new version of a file, it could be argued\nthat git-cvsserver actually fixes the non-versioned -k issue,\nat least partly, from some perspectives.  (There's still the issue\nof when a new .gitattributes file takes effect, etc.)\n\n> \n> Matthew, have you had a chance to test k mode changes against clients?\n> Are we reasonably bug-compatible with the original turd^H^H^Hhing? ;-)\n\nSo far I've only done limited testing with Linux CVS 1.12.12.\nNo newline-munging clients; the test cases just check the\nCVS/Entries file, not the file contents.  Most of my testing has\nbeen the test cases included with the patch.\n\nI don't expect any new compatibility problems from this.  The old code\nwould send exactly the same data for text files and binary files\nas this new code.  It was just limited to sending all files as text\nor all files as binary (existing gitcvs.allbinary), instead of\nallowing a mix.\n\nI'll try to do some newline-munging tests with Cygwin CVS (talking\nto a server on Linux) sometime this next week.\n\nI've heard the most finicky CVS client is probably the\none embedded in the Eclipse plugin.  Apparently it has had trouble\nwith minor tweaks in new versions of official CVS, let alone\nan emulation.  But given that I have never even tried Eclipse, I\nprobably am not a good choice for testing it, and probably wont.\n\n----------\n\nGenerally my motivation here is to make it easier for\nan organization like my day job to transition to git.  I generally\ndon't intend to use git-cvsserver myself much, especially not from\nplatforms that need the newline-munging.\n\nI perceive one remaining big issue for git-cvsserver to be\na good replacement for real CVS: The ability to properly\nsupport \"cvs update -r VERSION\", where VERSION could\nbe any branch, tag, CVS version number, or git commit hash.\nGit-cvsserver can partially support this by checking out a\ntotally different sandbox as \"cvs checkout VERSION\" (notice\nno -r), but without the ability to switch versions in place,\nthat is an awkward workaround at best.  Fixing this seems\nreally involved (extending the DB scheme, etc), unless\nclients either treat the CVS version as an opaque string, or\ncould be easily patched to do so (so that we could just stick\na GIT hash where a CVS version string is expected).  Also, some\nform of submodule support would also be nice, but not\ncritical (the right thing here is still vague in my mind,\nwhich is hardly surprising as there has recently been\ndiscussion about awkward use cases for submodules even\nwithin native git itself).  Merging with \"cvs update -j ...\"\nand creating tags/branches with CVS are relatively unimportant:\nrare enough that they could probably just be reserved for\ngit itself.  Does anyone have any thoughts about any of this?\n\n            - Matthew\n"},{"id":"77263","messageId":"alpine.DEB.1.00.0805191033080.30431@racer","threadId":"13526","inReplyTo":"20080519073535.GA2885@comcast.net","subject":"Re: [PATCH 0/3] git-cvsserver: Add support for some binary files","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-19T09:34:47Z","receivedAt":"2008-05-19T09:34:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 19 May 2008, Matthew Ogilvie wrote:\n\n> I perceive one remaining big issue for git-cvsserver to be a good \n> replacement for real CVS: The ability to properly support \"cvs update -r \n> VERSION\", where VERSION could be any branch, tag, CVS version number, or \n> git commit hash. Git-cvsserver can partially support this by checking \n> out a totally different sandbox as \"cvs checkout VERSION\" (notice no \n> -r), but without the ability to switch versions in place, that is an \n> awkward workaround at best.\n\nI might be missing something obvious, but would it not be better to _not_ \ncheck out anything, but serve every object straight from the object \ndatabase (possibly with CR/LF mangling)?\n\nCiao,\nDscho\n"},{"id":"77268","messageId":"46a038f90805190353o42fe59f2lfee9b6befdd588db@mail.gmail.com","threadId":"13526","inReplyTo":"20080519073535.GA2885@comcast.net","subject":"Re: [PATCH 0/3] git-cvsserver: Add support for some binary files","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2008-05-19T10:53:34Z","receivedAt":"2008-05-19T10:53:34Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Mon, May 19, 2008 at 7:35 PM, Matthew Ogilvie\n<mmogilvi_git@miniinfo.net> wrote:\n> I've heard the most finicky CVS client is probably the\n> one embedded in the Eclipse plugin.  Apparently it has had trouble\n> with minor tweaks in new versions of official CVS, let alone\n> an emulation.  But given that I have never even tried Eclipse, I\n> probably am not a good choice for testing it, and probably wont.\n\nIndeed. We've had endless trouble with Eclipse :-(\n\n> Generally my motivation here is to make it easier for\n> an organization like my day job to transition to git.  I generally\n> don't intend to use git-cvsserver myself much, especially not from\n> platforms that need the newline-munging.\n\nThat's exactly the reason why interest in cvsserver is always fleeting\n-- people hack on it during their team's transition. Perhaps you can\nget some help from an Eclipse-wielding member of your team ;-)\n\n> I perceive one remaining big issue for git-cvsserver to be\n> a good replacement for real CVS: The ability to properly\n> support \"cvs update -r VERSION\", where VERSION could\n\nThat would be good, and is not too hard. You can mostly simulate that\nextending the sqlite DB.\n\nWith that in place, a _very_ cool thing would be to add a special\n\"initial run\" script, intended for projects that have just been\nimported from a real CVS repo. The initial run script would look at\nthe CVS repo and add the needed version skew to make the revision\nnumbers of each file in sqlite match the cvs repo. For sane imports\nthis would work pretty well, and there's an amount of safe \"skew\" you\ncan add for slightly not-sane imports.\n\nThe end result is that a project can switch from CVS to git +\ngit-cvsserver and end users would not need to change their CVS\ncheckouts at all. Covert cvs->git migration, and users switch to git\non their own schedule.\n\nWRT to your other notes, I agree that cvs update -j -j support isn't\ninteresting -- users that do merge will want to be on the git side of\nthings -- but it isn't hard. Submodules is probable not worth the\nhassle - at least not yet :-) and a nested CVS checkout works\ntransparently - in some cases moreso than git submodules!\n\ncheers,\n\n\n\nm\n-- \n martin.langhoff@gmail.com\n martin@laptop.org -- School Server Architect\n - ask interesting questions\n - don't get distracted with shiny stuff - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"},{"id":"77302","messageId":"20080520030557.GA1438@comcast.net","threadId":"13526","inReplyTo":"alpine.DEB.1.00.0805191033080.30431@racer","subject":"Re: [PATCH 0/3] git-cvsserver: Add support for some binary files","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi_git@miniinfo.net","sentAt":"2008-05-20T03:05:58Z","receivedAt":"2008-05-20T03:05:58Z","isPatch":true,"sender":{"key":"mmogilvi_git@miniinfo.net","avatar":null},"body":"On Mon, May 19, 2008 at 10:34:47AM +0100, Johannes Schindelin wrote:\n> Hi,\n>\n> On Mon, 19 May 2008, Matthew Ogilvie wrote:\n>\n> > I perceive one remaining big issue for git-cvsserver to be a good\n> > replacement for real CVS: The ability to properly support \"cvs update -r\n> > VERSION\", where VERSION could be any branch, tag, CVS version number, or\n> > git commit hash. Git-cvsserver can partially support this by checking\n> > out a totally different sandbox as \"cvs checkout VERSION\" (notice no\n> > -r), but without the ability to switch versions in place, that is an\n> > awkward workaround at best.\n>\n> I might be missing something obvious, but would it not be better to _not_\n> check out anything, but serve every object straight from the object\n> database (possibly with CR/LF mangling)?\n\nAh, an opportunity to explain a few things about how git-cvsserver\nworks generally:\n\n1. git-cvsserver serves most objects straight from the object database\nalready (actually via git-cat-file), and will continue to do\nso.  The exception is when it merges user's changed files\nwith new versions from the repository, for which git-cvsserver\nuses temporary files.\n\n2. git-cvsserver does need to use a temporary git index file for\nsome things, though.  Usually it is using an empty working\ndirectory with such an index file.  User-modified files get\ntemporary names that get tracked internally, but it might make\nsense for modified .gitattributes files (at least) to be put into the\notherwise empty working directory with the right name so that the\nchanges effect the current cvs command.\n\n3. CR/LF mangling needs to be done by the CVS client, not the server.\nA windows client would do such mangling, while a Linux client would not\n(both talking to the same server).  With my patch, the server just\ntells the client when a file is binary, so that it won't be\nmangled even on windows.\n\n4. git-cvsserver currently does not support the \"-r\" argument to\ncheckout or update (to get a particular a branch, a tag, or a\nversion number).  Instead, as a kind of workaround, it has a hook to\ntreat the CVS \"module\" argument (primarily intended to be a\nproject name in a repository with multiple projects) as a branch\nor tag name instead.  But the \"module\" can't be switched on the fly;\nyou have to checkout a completely new sandbox to get a different\nmodule (or with git-cvsserver, another branch).\n\n- Matthew Ogilvie\n"}]}