{"thread":{"id":"8085","subject":"Another fast-import/import-tars issue","startedAt":"2007-05-11T20:08:18Z","lastAt":"2007-05-16T18:49:51Z","messageCount":4,"participants":["Chris Riddoch","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"41852","messageId":"6efbd9b70705111308v47a76b04n9328ebf393a209e6@mail.gmail.com","threadId":"8085","inReplyTo":null,"subject":"Another fast-import/import-tars issue","fromName":"Chris Riddoch","fromEmail":"riddochc@gmail.com","sentAt":"2007-05-11T20:08:18Z","receivedAt":"2007-05-11T20:08:18Z","isPatch":false,"sender":{"key":"riddochc@gmail.com","avatar":null},"body":"Hi, folks.\n\nI believe I've uncovered an issue in fast-import, but I don't know the\ncode well enough yet to debug it. So, I'll produce my evidence and\nlet others work on finding the solution. It should be pretty easy to\nreproduce.\n\nFirst, I'm running: 1.5.2.rc1.9.g6644\n\nGrab the tarball of Perl 5.8.8 - http://www.perl.com/CPAN/src/perl-5.8.8.tar.bz2\n\nNote its md5, just so you know it's not corrupted from the outset.\nb8c118d4360846829beb30b02a6b91a7 perl-5.8.8.tar.gz\na377c0c67ab43fd96eeec29ce19e8382 perl-5.8.8.tar.bz2\n\nTry this:\n\n$ tar -xjf perl-5.8.8.tar.bz2\n$ cd perl-5.8.8\n$ git init\n$ git add .\n$ git commit -a -m \"Import from working tree copy\"\n\nNow, for convenience of debugging, I have myself a script I call\n~/bin/fast-import-filter.sh:\n\n#!/bin/bash\ntee fast-import.log | git fast-import --quiet\n\nThen, I have a slightly-changed ~/bin/import-tars script, like so:\n\n20c20,21\n< open(FI, '|-', 'git', 'fast-import', '--quiet')\n---\n> #open(FI, '|-', 'git', 'fast-import', '--quiet')\n> open(FI, '|-', 'fast-import-filter.sh')\n\nNow,\n\n$ import-tars.pl ../perl-5.8.8.tar.bz2\n\nOkay, so the trees pointed to by the tips of the master and\nimport-tars branches *should* be identical here, right?\n\n$ git diff-tree master: import-tars: | wc -l\n229\n\nNot so good.\n\n-- \nepistemological humility\n Chris Riddoch\n"},{"id":"42317","messageId":"Pine.LNX.4.64.0705161659530.6410@racer.site","threadId":"8085","inReplyTo":"6efbd9b70705111308v47a76b04n9328ebf393a209e6@mail.gmail.com","subject":"[PATCH] import-tars: Use the \"Link indicator\" to identify directories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-05-16T16:22:26Z","receivedAt":"2007-05-16T16:22:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nEarlier, we used the mode to determine if a name was associated with\na directory. This fails, since some tar programs do not set the mode\ncorrectly. However, the link indicator _has_ to be set correctly.\n\nNoticed by Chris Riddoch.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Fri, 11 May 2007, Chris Riddoch wrote:\n\n\t> I believe I've uncovered an issue in fast-import, but I don't \n\t> know the code well enough yet to debug it. So, I'll produce my \n\t> evidence and let others work on finding the solution. It should \n\t> be pretty easy to reproduce.\n\n\tIt was easy. Thanks.\n\n\tThe problem is -- again -- that a directory is overwritten, since \n\tit is not recognized as a directory. Earlier, I tried to use the \n\ttrailing \"/\" for that. Which fails with your example.\n\n\tI actually took the time to research in Wikipedia what should be \n\tthe correct way to find out if the current item is a directory...\n\n contrib/fast-import/import-tars.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl\nindex 1e6fa5a..23aeb25 100755\n--- a/contrib/fast-import/import-tars.perl\n+++ b/contrib/fast-import/import-tars.perl\n@@ -75,7 +75,7 @@ foreach my $tar_file (@ARGV)\n \t\t$mode = oct $mode;\n \t\t$size = oct $size;\n \t\t$mtime = oct $mtime;\n-\t\tnext if $mode & 0040000;\n+\t\tnext if $typeflag == 5; # directory\n \n \t\tprint FI \"blob\\n\", \"mark :$next_mark\\n\", \"data $size\\n\";\n \t\twhile ($size > 0 && read(I, $_, 512) == 512) {\n-- \n1.5.2.rc3.2506.ge455\n"},{"id":"42322","messageId":"7vsl9weie0.fsf@assigned-by-dhcp.cox.net","threadId":"8085","inReplyTo":"Pine.LNX.4.64.0705161659530.6410@racer.site","subject":"Re: [PATCH] import-tars: Use the \"Link indicator\" to identify directories","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-16T18:24:39Z","receivedAt":"2007-05-16T18:24:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Earlier, we used the mode to determine if a name was associated with\n> a directory. This fails, since some tar programs do not set the mode\n> correctly. However, the link indicator _has_ to be set correctly.\n\n> \tThe problem is -- again -- that a directory is overwritten, since \n> \tit is not recognized as a directory. Earlier, I tried to use the \n> \ttrailing \"/\" for that. Which fails with your example.\n>\n> \tI actually took the time to research in Wikipedia what should be \n> \tthe correct way to find out if the current item is a directory...\n\nThis matches my reading of GNU tar as well. The patch would not\nbreak a correctly made tar archive, would fix importing archives\ncreated by a broken tar that does not do mode right (but uses the\ncorrect typeflag), _and_ would _break_ archives created by tar\nthat is broken in a different way, sets the mode right but uses\na wrong typeflag.\n\nI do not know which breakages are more common, and this one\nbeing in contrib/ I do not think it really matters in practice,\nbut as a principle, I think we should try to adhere to the same\n\"no regression\" policy the kernel folks try to adhere to. If\nsomething used to work, even if its was by accident or a bug, we\nhad better have a pretty good reason to break it by a change\nthat fixes things for other people, _even_ when that other\npeople outnumber the people who are affected by the regression.\n\nI'd first ask GNU tar maintainer if he knows of existing\nimplementations of tar that are broken in the latter sense (iow,\nsets modes correctly but typeflag incorrectly), as the tarball\nextraction codepath would have the exact same issue.\n"},{"id":"42324","messageId":"7vlkfoeh80.fsf@assigned-by-dhcp.cox.net","threadId":"8085","inReplyTo":"7vsl9weie0.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] import-tars: Use the \"Link indicator\" to identify directories","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-16T18:49:51Z","receivedAt":"2007-05-16T18:49:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n>\n>> Earlier, we used the mode to determine if a name was associated with\n>> a directory. This fails, since some tar programs do not set the mode\n>> correctly. However, the link indicator _has_ to be set correctly.\n\nNah, what was I smoking. Even gtar seems to give mode=\"0000775\"\n(with NUL termination) for directories, so there is no way there\nwere regressions. Patch looks good.\n\n\tAcked-by: Junio C Hamano \n"}]}