{"thread":{"id":"19025","subject":"Git-SVN on Cygwin: svn+ssh good, https awkward","startedAt":"2009-04-23T16:31:09Z","lastAt":"2009-04-24T17:31:25Z","messageCount":7,"participants":["Matthias Andree","Eric Blake","Eric Wong"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"112095","messageId":"op.usulh7a71e62zd@balu.cs.uni-paderborn.de","threadId":"19025","inReplyTo":null,"subject":"Git-SVN on Cygwin: svn+ssh good, https awkward","fromName":"Matthias Andree","fromEmail":"matthias.andree@gmx.de","sentAt":"2009-04-23T16:31:09Z","receivedAt":"2009-04-23T16:31:09Z","isPatch":false,"sender":{"key":"matthias.andree@gmx.de","avatar":null},"body":"Hi Eric,\n\n=== Short story ===\n\ngit-svn on Cygwin works well with svn+ssh://, but needs some fixes around  \ntempfile and/or subprocess handling for https:// - please help.\n\n\n=== Long story ===\n\nFirst the good news: git-svn on Cygwin seems to work just fine with  \nsvn+ssh://... style Subversion URLs. I've been doing this for quite a  \nwhile now with various repos, never had issues: I can init, clone, fetch  \nrebase, dcommit just fine.\n\nI'm looking for some help https:// access on Cygwin (Windows XP SP3  \nunderneath; more version info at the end of the message).\n\nI've debugged it a little, but am not sufficiently acquainted with the  \ngit-svn Perl source to do changes myself; I hope I can describe the issue  \nwell enough that you (or somebody else) can suggest code changes to  \nimprove the experience.\n\n\n0. PROBLEM/SYMPTOM:\n\nlogin@mycomp ~/compnet2.gitsvn\n$ git svn fetch\n         M       file1\n         A       file2\n         A       file3\n         A       file4\nr15 = 70a56f818a057442e29161170e28c5df4b38a811 (git-svn)\nPermission denied: Can't open  \n'/cygdrive/c/DOKUME~1/login/LOKALE~1/Temp/tempfile.tmp': Permission denied  \nat /usr/local/libexec/git-core/git-svn line 2540\n\nwhich is, in my version:\n\n   2537                  }\n   2538                  $ed = SVN::Git::Fetcher->new($self);\n   2539          }\n+ 2540          unless ($self->ra->gs_do_update($last_rev, $rev, $self,  \n$ed)) {\n   2541                  die \"SVN connection failed somewhere...\\n\";\n   2542          }\n   2543          $self->make_log_entry($rev, \\@parents, $ed);\n\n\nApparently, we die when fetching a new revision for the 2nd time. I can  \nrestart git svn fetch and it will pick up where it left off. It will make  \nprogress, but is painful.\n\ngit svn rebase has the same issues. I haven't tested other operations such  \nas dcommit yet.\n\n\n1. WORKAROUND for fetch/rebase:\n\nwhile ! git svn fetch ; do : ; done\ngit svn rebase\n\n\n2. A SHORT LOG OF FINDING THE CAUSE:\n\n$ perl -d /usr/local/libexec/git-core/git-svn fetch\n...\nmain::(/usr/local/libexec/git-core/git-svn:10):\n10:     $AUTHOR = 'Eric Wong <normalperson@yhbt.net>';\n   DB<1> c 2540\nGit::SVN::do_fetch(/usr/local/libexec/git-core/git-svn:2540):\n2540:           unless ($self->ra->gs_do_update($last_rev, $rev, $self,  \n$ed)) {\n   DB<2> c 2540\n...\nr16 = d7829595222a013e703fe4377996984b52921789 (git-svn)\nGit::SVN::do_fetch(/usr/local/libexec/git-core/git-svn:2540):\n2540:           unless ($self->ra->gs_do_update($last_rev, $rev, $self,  \n$ed)) {\n\nNow watch this (we're immediately before the problem):\n\n$ ls -l /cygdrive/c/DOKUME~1/login/LOKALE~1/Temp/\n??????????? ? ?       ?       ?            ? tempfile.tmp\n$ ls -l /cygdrive/c/DOKUME~1/login/LOKALE~1/Temp/tempfile.tmp\nls: cannot access /cygdrive/c/DOKUME~1/login/LOKALE~1/Temp/tempfile.tmp:  \nNo such file or directory\n\nThis zombie behaviour of trashing the file flags happens if Cygwin is  \nasked to unlink() a file to which there are still open handles. It can't  \ndelete the file right away (due to the open handles), so it marks it  \ndelete-on-close instead, which then happens when the process(es) terminate.\n\n$ ~/Desktop/handle.exe tempfile.tmp\n...\ngit.exe            pid: 2236    62C:  \nC:\\DOKUME~1\\mandree\\LOKALE~1\\Temp\\tempfile.tmp\ngit.exe            pid: 5552    62C:  \nC:\\DOKUME~1\\mandree\\LOKALE~1\\Temp\\tempfile.tmp\ngit-hash-object.exe pid: 5320    62C:  \nC:\\DOKUME~1\\mandree\\LOKALE~1\\Temp\\tempfile.tmp\n\nSo we have three processes that have the file open. I cannot say if they  \nmean the same file, or a different one, but this is the problem.\n\nHere's the pstree dump to help identify the users of this file, with  \ncmdline arguments:\n\nperl,5660 -d -I... /usr/local/libexec/git-core/git-svn fetch\n   |-git,2236  --batch\n   |-git,5552 git-hash-object -w --stdin-paths\n   |   `-git-hash-object,5320\n   `-git,1128 update-index -z --index-info\n\nLet's continue in the Perl debugger and crash:\n\n   DB<3> c\nPermission denied: Can't open  \n'/cygdrive/c/DOKUME~1/login/LOKALE~1/Temp/tempfile.tmp': Permission denied  \nat /usr/local/libexec/git-core/git-svn line 2540\n\n\nBottom line: the second time we hit this \"unless  \n($self->ra->gs_do_update($last_rev, $rev, $self, $ed))\" line, we die  \nbecause there are still processes that have the tempfile open and thus the  \nformer unlink() on the tempfile.tmp couldn't become effective.\n\n\n\n3. UNDERLYING CYGWIN LIMITATIONS\n\n<http://wiki.osdev.org/Cygwin_Issues#unlink.28.29> documents now  \n(2009-04-23 16:15 UTC):\n* unlink isn't atomic (as on other platforms also)\n...\nOn cygwin unlink() is deferred to the end of the process, when unlink  \nfails because of locks or delete on close (ERROR_SHARING_VIOLATION).\n\n* unlink a file and subsequent creation of a new file in the same location  \nis not supported.\n\n* \"delete on close\" (unlinking an open file) is supported.\n\n\nI can provide a short C program to demonstrate this behaviour.\n\n\n\n4. FIX SUGGESTIONS AND QUESTIONS\n\nCan we defer unlink()ing the tempfile until all handles to it are closed?\n\nCan we let complete and wait for all processes that hold handles to the  \ntempfile.tmp file before attempting to create a new copy?\n\nCan we use unique tempfile names (timestamps, counter, random number  \ngenerator) instead of the hardcoded \"tempfile.tmp\"? This is probably a  \ngood idea anyways to evade symlink attacks.\n\n\nEither should work to fix the issue.\n\n\nI'm willing to test patches.\n\n\n5. VERSION INFORMATION:\n\nCygwin:\nCYGWIN_NT-5.1 balu 1.5.25(0.156/4/2) 2008-06-12 19:34 i686 Cygwin\n\nGit:\ngit version 1.6.3.rc1.51.gea0b7\n\nSubversion:\nsvn, version 1.6.1 (r37116)\n    compiled Apr 11 2009, 12:07:06\n\nCopyright (C) 2000-2009 CollabNet.\nSubversion is open source software, see http://subversion.tigris.org/\nThis product includes software developed by CollabNet  \n(http://www.Collab.Net/).\n\nThe following repository access (RA) modules are available:\n\n* ra_neon : Module for accessing a repository via WebDAV protocol using  \nNeon.\n   - handles 'http' scheme\n   - handles 'https' scheme\n* ra_svn : Module for accessing a repository using the svn network  \nprotocol.\n   - with Cyrus SASL authentication\n   - handles 'svn' scheme\n* ra_local : Module for accessing a repository on local disk.\n   - handles 'file' scheme\n* ra_serf : Module for accessing a repository via WebDAV protocol using  \nserf.\n   - handles 'http' scheme\n   - handles 'https' scheme\n\n\n\nThanks for any efforts to help!\n\n-- \nMatthias Andree\n"},{"id":"112097","messageId":"op.usulx6xo1e62zd@balu.cs.uni-paderborn.de","threadId":"19025","inReplyTo":"op.usulh7a71e62zd@balu.cs.uni-paderborn.de","subject":"Re: Git-SVN on Cygwin: svn+ssh good, https awkward","fromName":"Matthias Andree","fromEmail":"matthias.andree@gmx.de","sentAt":"2009-04-23T16:40:44Z","receivedAt":"2009-04-23T16:40:44Z","isPatch":false,"sender":{"key":"matthias.andree@gmx.de","avatar":null},"body":"As a followup to my own message,\n\nhere's one more link to explain Cygwin/Windows limitations that seem to  \nbreak git-svn here:\nhttp://markmail.org/message/4o4yk6b4u6e44wyh\n\nThanks in advance for any help offered with git-svn.\n\n-- \nMatthias Andree\n"},{"id":"112101","messageId":"loom.20090423T175124-165@post.gmane.org","threadId":"19025","inReplyTo":"op.usulh7a71e62zd@balu.cs.uni-paderborn.de","subject":"Re: Git-SVN on Cygwin: svn+ssh good, https awkward","fromName":"Eric Blake","fromEmail":"ebb9@byu.net","sentAt":"2009-04-23T17:58:24Z","receivedAt":"2009-04-23T17:58:24Z","isPatch":false,"sender":{"key":"eblake@redhat.com","avatar":"https://avatars.githubusercontent.com/u/32933908?v=4"},"body":"Matthias Andree <matthias.andree <at> gmx.de> writes:\n\n> 3. UNDERLYING CYGWIN LIMITATIONS\n> \n> * unlink a file and subsequent creation of a new file in the same location  \n> is not supported.\n> \n> * \"delete on close\" (unlinking an open file) is supported.\n\nThese last two points are old information.  I would first recommend that you \ntry running the new cygwin 1.7 (still in beta), where unlink() semantics have \nbeen greatly improved (you can actually unlink() an in-use file and recreate a \nnew file by the same name while the old handle is still open).\n\nhttp://cygwin.com/#beta-test\n\n> 4. FIX SUGGESTIONS AND QUESTIONS\n> \n> Can we defer unlink()ing the tempfile until all handles to it are closed?\n> \n> Can we let complete and wait for all processes that hold handles to the  \n> tempfile.tmp file before attempting to create a new copy?\n> \n> Can we use unique tempfile names (timestamps, counter, random number  \n> generator) instead of the hardcoded \"tempfile.tmp\"? This is probably a  \n> good idea anyways to evade symlink attacks.\n\nAlthough newer cygwin may fix your particular problem, this is not a bad \nsuggestion for msysgit, so I suspect someone may be bothered enough by this to \nwrite a patch.  I, however, will not be the one doing it.\n\n-- \nEric Blake\n"},{"id":"112103","messageId":"op.usuqcyp81e62zd@balu.cs.uni-paderborn.de","threadId":"19025","inReplyTo":"loom.20090423T175124-165@post.gmane.org","subject":"Re: Git-SVN on Cygwin: svn+ssh good, https awkward","fromName":"Matthias Andree","fromEmail":"matthias.andree@gmx.de","sentAt":"2009-04-23T18:16:00Z","receivedAt":"2009-04-23T18:16:00Z","isPatch":false,"sender":{"key":"matthias.andree@gmx.de","avatar":null},"body":"Am 23.04.2009, 19:58 Uhr, schrieb Eric Blake <ebb9@byu.net>:\n\n>> Can we use unique tempfile names (timestamps, counter, random number\n>> generator) instead of the hardcoded \"tempfile.tmp\"? This is probably a\n>> good idea anyways to evade symlink attacks.\n>\n> Although newer cygwin may fix your particular problem, this is not a bad\n> suggestion for msysgit, so I suspect someone may be bothered enough by  \n> this to write a patch.  I, however, will not be the one doing it.\n\nHi Eric,\n\nthanks for the hints and pointers to Cygwin 1.7 beta, however I'd suggest  \nto make such changes upstream (i. e. in git.git) for the benefit of  \neverybody else.\n\nIt also looks to me as though minor changes in the actual baseline Git  \ncode may be sufficient to get git-svn working on Cygwin 1.5 (stable)  \nalready. I'd prefer such a minor intrusion to hacking away on forks. :-)\n\nOtherwise I'd start looking at how git-svn handles https:// and svn+ssh://  \nmethods differently from each other next week or so.\n\nLooking forward to further insights.\n\n-- \nMatthias Andree\n"},{"id":"112106","messageId":"op.usuq4xw71e62zd@balu.cs.uni-paderborn.de","threadId":"19025","inReplyTo":"loom.20090423T175124-165@post.gmane.org","subject":"Re: Git-SVN on Cygwin: svn+ssh good, https awkward","fromName":"Matthias Andree","fromEmail":"matthias.andree@gmx.de","sentAt":"2009-04-23T18:32:47Z","receivedAt":"2009-04-23T18:32:47Z","isPatch":false,"sender":{"key":"matthias.andree@gmx.de","avatar":null},"body":"Am 23.04.2009, 19:58 Uhr, schrieb Eric Blake <ebb9@byu.net>:\n\n> These last two points are old information.  I would first recommend that  \n> you try running the new cygwin 1.7 (still in beta), where unlink()  \n> semantics have been greatly improved (you can actually unlink() an  \n> in-use file and recreate a new file by the same name while the old  \n> handle is still open).\n>\n> http://cygwin.com/#beta-test\n\nIndeed, with a fresh Cygwin 1.7 install, all this seems to work smoothly.  \nI'd rather not bet on Cygwin release dates though, and can't judge on  \nshort notice what else will break with Cygwin 1.7... so I'd still  \nappreciate help with fixing git-svn on Cygwin 1.5.\n\n-- \nMatthias Andree\n"},{"id":"112113","messageId":"20090423190308.GA10437@dcvr.yhbt.net","threadId":"19025","inReplyTo":"op.usulh7a71e62zd@balu.cs.uni-paderborn.de","subject":"Re: Git-SVN on Cygwin: svn+ssh good, https awkward","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2009-04-23T19:03:08Z","receivedAt":"2009-04-23T19:03:08Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Matthias Andree <matthias.andree@gmx.de> wrote:\n> Hi Eric,\n>\n> === Short story ===\n>\n> git-svn on Cygwin works well with svn+ssh://, but needs some fixes around \n> tempfile and/or subprocess handling for https:// - please help.\n\nHi Matthias,\n\nMarcus Griep did a lot of work for more efficiently handling of\ntempfiles in Git.pm a few months ago, so maybe he has more insight into\nhow things work...\n\ngit-svn used to use IO::File->new_tmpfile() which was much simpler and\nprobably less prone to portability problems, but cycled through inodes\ntoo quickly for Marcus (and probably some other people).\n\n\nSpeaking only for myself and not other contributors:\n\n  I have no interest in dealing with crazy systems like Windows.  Ever.\n\n  However, I'll gladly accept patches to make things work if they don't\n  break existing use cases.\n\n  Keep in mind that I'm an extremist.  I dislike Windows (and some other\n  crazy software systems) to the point that I refuse to deal with them\n  even in paid contracts and employment.\n\n-- \nEric Wong\n"},{"id":"112219","messageId":"op.uswiynhl1e62zd@balu.cs.uni-paderborn.de","threadId":"19025","inReplyTo":"20090423190308.GA10437@dcvr.yhbt.net","subject":"Re: Git-SVN on Cygwin: svn+ssh good, https awkward","fromName":"Matthias Andree","fromEmail":"matthias.andree@gmx.de","sentAt":"2009-04-24T17:31:25Z","receivedAt":"2009-04-24T17:31:25Z","isPatch":false,"sender":{"key":"matthias.andree@gmx.de","avatar":null},"body":"Am 23.04.2009, 21:03 Uhr, schrieb Eric Wong <normalperson@yhbt.net>:\n\n> Matthias Andree <matthias.andree@gmx.de> wrote:\n>> Hi Eric,\n>>\n>> === Short story ===\n>>\n>> git-svn on Cygwin works well with svn+ssh://, but needs some fixes  \n>> around\n>> tempfile and/or subprocess handling for https:// - please help.\n>\n> Hi Matthias,\n>\n> Marcus Griep did a lot of work for more efficiently handling of\n> tempfiles in Git.pm a few months ago, so maybe he has more insight into\n> how things work...\n>\n> git-svn used to use IO::File->new_tmpfile() which was much simpler and\n> probably less prone to portability problems, but cycled through inodes\n> too quickly for Marcus (and probably some other people).\n\nI debugged this a bit further, and the damage (i. e. removal of the  \ntempfile) apparently happens\nin $pool->clear; in line 4355. I single-stepped it, and the  \napr_pool_clear(...) is the culprit, it unlinks() the tempfile, making this  \nlocation unusable.\n\nThe temp file is generated when the Reporter object is created through  \n$self->do_update in line 4336.\n\nIt remains unclear to me who generates the non-unique filename (it's  \n...\\Temp\\tempfile.tmp for me), I've not found the code that generates the  \nfile names.\n\nQuestions:\n\n- How can I either make sure that the temporary file name for the reporter  \ngets either a unique name (near line 4336, through SVN::Ra...)\n\n- or is that the temp file truncated, rather than deleted, near line 4355  \n(through SVN::Pool::clear)?\n\n- Is there any way to influence how the SVN::Ra::Reporter obtains  \ntemporary files?\nI seem to be unable to trace this down to the actual functions, but then  \nagain, my perlboot is rather holey...\n\nAny help?\n\n\n   4132  package Git::SVN::Ra;\n   4133  use vars qw/@ISA $config_dir $_log_window_size/;\n   4134  use strict;\n   4135  use warnings;\n   4136  my ($ra_invalid, $can_do_switch, %ignored_err, $RA);\n   4137\n   ....\n   4324  sub gs_do_update {\n   4325          my ($self, $rev_a, $rev_b, $gs, $editor) = @_;\n   4326          my $new = ($rev_a == $rev_b);\n   4327          my $path = $gs->{path};\n   4328\n   4329          if ($new && -e $gs->{index}) {\n   4330                  unlink $gs->{index} or die\n   4331                    \"Couldn't unlink index: $gs->{index}: $!\\n\";\n   4332          }\n   4333          my $pool = SVN::Pool->new;\n   4334          $editor->set_path_strip($path);\n   4335          my (@pc) = split m#/#, $path;\n: 4336          my $reporter = $self->do_update($rev_b, (@pc ? shift @pc :  \n''),\n   4337                                          1, $editor, $pool);\n   4338          my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();\n   4339\n   4340          # Since we can't rely on svn_ra_reparent being available,  \nwe'll\n   4341          # just have to do some magic with set_path to make it so\n   4342          # we only want a partial path.\n   4343          my $sp = '';\n   4344          my $final = join('/', @pc);\n   4345          while (@pc) {\n   4346                  $reporter->set_path($sp, $rev_b, 0, @lock, $pool);\n   4347                  $sp .= '/' if length $sp;\n   4348                  $sp .= shift @pc;\n   4349          }\n   4350          die \"BUG: '$sp' != '$final'\\n\" if ($sp ne $final);\n   4351\n   4352          $reporter->set_path($sp, $rev_a, $new, @lock, $pool);\n   4353\n   4354          $reporter->finish_report($pool);\n: 4355          $pool->clear;\n   4356          $editor->{git_commit_ok};\n   4357  }\n\n-- \nMatthias Andree\n"}]}