{"thread":{"id":"12675","subject":"[PATCH] git-cvsimport: fix initial checkout","startedAt":"2008-03-13T19:09:38Z","lastAt":"2008-03-13T23:29:07Z","messageCount":9,"participants":["Marc-Andre Lureau","Junio C Hamano","Martin Langhoff","Marc-André Lureau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"71986","messageId":"1205435378-10411-1-git-send-email-marcandre.lureau@gmail.com","threadId":"12675","inReplyTo":null,"subject":"[PATCH] git-cvsimport: fix initial checkout","fromName":"Marc-Andre Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2008-03-13T19:09:38Z","receivedAt":"2008-03-13T19:09:38Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"git-symbolic-ref HEAD returns master reference, even if the file does\nnot exists. That prevents the initial checkout and fails in\ngit-rev-parse. The patch checks the existence of the reference file\nbefore assuming an original branch exists. There might be better\nsolutions than checking file existence.\n---\n git-cvsimport.perl | 11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 95c5eec..1512fe4 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -570,12 +570,16 @@ unless (-d $git_dir) {\n \topen(F, \"git-symbolic-ref HEAD |\") or\n \t\tdie \"Cannot run git-symbolic-ref: $!\\n\";\n \tchomp ($last_branch = );\n-\t$last_branch = basename($last_branch);\n-\tclose(F);\n-\tunless ($last_branch) {\n+\tif (-f \"$git_dir/$last_branch\") {\n+\t $last_branch = basename($last_branch);\n+\t unless ($last_branch) {\n \t\twarn \"Cannot read the last branch name: $! -- assuming 'master'\\n\";\n \t\t$last_branch = \"master\";\n+\t }\n+\t} else {\n+\t $last_branch = \"\";\n \t}\n+\tclose(F);\n \t$orig_branch = $last_branch;\n \t$tip_at_start = `git-rev-parse --verify HEAD`;\n \n@@ -953,6 +957,7 @@ while () {\n \t\tprint \"* UNKNOWN LINE * $_\\n\";\n \t}\n }\n+\n commit() if $branch and $state != 11;\n \n unless ($opt_P) {\n-- \n1.5.4.4.534.gfb90c.dirty\n"},{"id":"71994","messageId":"7vve3qwczy.fsf@gitster.siamese.dyndns.org","threadId":"12675","inReplyTo":"1205435378-10411-1-git-send-email-marcandre.lureau@gmail.com","subject":"Re: [PATCH] git-cvsimport: fix initial checkout","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-13T21:05:53Z","receivedAt":"2008-03-13T21:05:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marc-Andre Lureau writes:\n\n> git-symbolic-ref HEAD returns master reference, even if the file does\n> not exists. That prevents the initial checkout and fails in\n> git-rev-parse.\n\nI had an impression that this check was deliberately done, but I do not\nrecall the details. Martin?\n\n> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> index 95c5eec..1512fe4 100755\n> --- a/git-cvsimport.perl\n> +++ b/git-cvsimport.perl\n> @@ -570,12 +570,16 @@ unless (-d $git_dir) {\n> \topen(F, \"git-symbolic-ref HEAD |\") or\n> \t\tdie \"Cannot run git-symbolic-ref: $!\\n\";\n> \tchomp ($last_branch = );\n> -\t$last_branch = basename($last_branch);\n> -\tclose(F);\n> -\tunless ($last_branch) {\n> +\tif (-f \"$git_dir/$last_branch\") {\n> +\t $last_branch = basename($last_branch);\n> +\t unless ($last_branch) {\n> \t\twarn \"Cannot read the last branch name: $! -- assuming 'master'\\n\";\n> \t\t$last_branch = \"master\";\n> +\t }\n\nIn any case, what if last_branch is a branch with hierarchical name, I\nhave to wonder. If you are on a branch whose name has a slash\nin it, like \"frotz/nitfol\" when you start cvsimport, doesn't this (before\nor after the patch) code break your repository?\n"},{"id":"71998","messageId":"47D9A836.9010601@catalyst.net.nz","threadId":"12675","inReplyTo":"1205435378-10411-1-git-send-email-marcandre.lureau@gmail.com","subject":"Re: [PATCH] git-cvsimport: fix initial checkout","fromName":"Martin Langhoff","fromEmail":"martin@catalyst.net.nz","sentAt":"2008-03-13T22:18:30Z","receivedAt":"2008-03-13T22:18:30Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"Marc-Andre Lureau wrote:\n> git-symbolic-ref HEAD returns master reference, even if the file does\n> not exists. That prevents the initial checkout and fails in\n> git-rev-parse.\n\nBut you are patching the block that gets triggered on subsequent\nimports, this code does not deal with \"initial checkout\" unless\nsomething else is wrong. The line right above the open() is an else that\nhas the block that matters.\n\n> The patch checks the existence of the reference file\n> before assuming an original branch exists. There might be better\n> solutions than checking file existence.\n\nThere are indeed. If we need this patch -- then you can call git\nref-parse right to see if you get a sha1.\n\n> -\tunless ($last_branch) {\n> +\tif (-f \"$git_dir/$last_branch\") {\n\nNote that the file won't exist there in any modern git. It will be in\n$git_dir/refs/heads/$last_branch. Did you test this patch?\n\nWhat's your workflow with cvsimport? Perhaps you are doing something\nstrange with it... :-)\n\ncheers,\n\n\nmartin\n-- \n-----------------------------------------------------------------------\nMartin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington\nWEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St\nNZ: +64(4)916-7224 MOB: +64(21)364-017 UK: 0845 868 5733 ext 7224\n Make things as simple as possible, but no simpler - Einstein\n-----------------------------------------------------------------------\n"},{"id":"72000","messageId":"e29894ca0803131604qa61adfbo22ff75d076feb899@mail.gmail.com","threadId":"12675","inReplyTo":"47D9A836.9010601@catalyst.net.nz","subject":"Re: [PATCH] git-cvsimport: fix initial checkout","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2008-03-13T23:04:13Z","receivedAt":"2008-03-13T23:04:13Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi\n\nOn Fri, Mar 14, 2008 at 12:18 AM, Martin Langhoff\n wrote:\n> Marc-Andre Lureau wrote:\n> > git-symbolic-ref HEAD returns master reference, even if the file does\n> > not exists. That prevents the initial checkout and fails in\n> > git-rev-parse.\n>\n> But you are patching the block that gets triggered on subsequent\n> imports, this code does not deal with \"initial checkout\" unless\n> something else is wrong. The line right above the open() is an else that\n> has the block that matters.\n>\n\nYeah, it failed in the middle of a ~4h import, I did not restart it.\n\ngit-cvsimport -r cvs -p b,HEAD -k -m -a -v -d\n:pserver:anoncvs@anoncvs.freedesktop.org:/cvs/gstreamer -C\ngst-plugins-good gst-plugins-good\n\nThis is a quite problematic CVS, btw. (missing patches/files in the\nend, branch merge fail ... see my previous patch)\n\n> > The patch checks the existence of the reference file\n> > before assuming an original branch exists. There might be better\n> > solutions than checking file existence.\n>\n> There are indeed. If we need this patch -- then you can call git\n> ref-parse right to see if you get a sha1.\n\nOk, which one is prefered? ref-parse I guess? I am mostly ignorant of\nall the plumbing stuff.\n\n> > - unless ($last_branch) {\n> > + if (-f \"$git_dir/$last_branch\") {\n>\n> Note that the file won't exist there in any modern git. It will be in\n> $git_dir/refs/heads/$last_branch. Did you test this patch?\n>\n\nCrap. The patch indeed worked, because the file did not exist. The\nsecond time it also worked:\n\nskip patchset 5825: 1205277122 before 1205418644\nskip patchset 5826: 1205418644 before 1205418644\nDONE.\nAlready up-to-date.\n*** Building gst-plugins-good *** [1/145]\nmake -j2\n\n\nResult is here: http://git.infradead.org/users/elmarco/gst-plugins-good.git\n\nThanks for the review!!\n\n-- \nMarc-André Lureau\n"},{"id":"72001","messageId":"e29894ca0803131607ld534fa2w318e7313913933ca@mail.gmail.com","threadId":"12675","inReplyTo":"e29894ca0803131604qa61adfbo22ff75d076feb899@mail.gmail.com","subject":"Re: [PATCH] git-cvsimport: fix initial checkout","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2008-03-13T23:07:05Z","receivedAt":"2008-03-13T23:07:05Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi again,\n\nBtw, the missing checkout file is,\n\nsys / osxvideo / Makefile.am\n\nI have a clue why (strange version number), but no idea how to fix it.\n-- \nMarc-André Lureau\n"},{"id":"72002","messageId":"7vr6eew70a.fsf@gitster.siamese.dyndns.org","threadId":"12675","inReplyTo":"47D9A836.9010601@catalyst.net.nz","subject":"Re: [PATCH] git-cvsimport: fix initial checkout","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-13T23:15:17Z","receivedAt":"2008-03-13T23:15:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> Marc-Andre Lureau wrote:\n>> git-symbolic-ref HEAD returns master reference, even if the file does\n>> not exists. That prevents the initial checkout and fails in\n>> git-rev-parse.\n>\n> But you are patching the block that gets triggered on subsequent\n> imports, this code does not deal with \"initial checkout\" unless\n> something else is wrong. The line right above the open() is an else that\n> has the block that matters.\n>\n>> The patch checks the existence of the reference file\n>> before assuming an original branch exists. There might be better\n>> solutions than checking file existence.\n>\n> There are indeed. If we need this patch -- then you can call git\n> ref-parse right to see if you get a sha1.\n>\n>> -\tunless ($last_branch) {\n>> +\tif (-f \"$git_dir/$last_branch\") {\n>\n> Note that the file won't exist there in any modern git. It will be in\n> $git_dir/refs/heads/$last_branch. Did you test this patch?\n\nMartin, it may not even be in $git_dir/refs/heads/$last_branch ;-) The\nrefs can be packed.\n\nBy the way, doesn't cvsimport fail when your HEAD is detached with this\ncode?\n\nI always have cvsimport update the pristine upstream branch and rebase my\nwork against it, so I never have the branch cvsimport updates checked\nout, and for meit seems to work wonderfully (well, at least as wonderful\nas a workflow that involves any CVS in it could be). I do not see a\nreason why it should not to work similarly well when my HEAD is detached..\n"},{"id":"72003","messageId":"e29894ca0803131622mbea0c95j2d3b4fd4ee8eca1@mail.gmail.com","threadId":"12675","inReplyTo":"47D9A836.9010601@catalyst.net.nz","subject":"Re: [PATCH] git-cvsimport: fix initial checkout","fromName":"Marc-André Lureau","fromEmail":"marcandre.lureau@gmail.com","sentAt":"2008-03-13T23:22:08Z","receivedAt":"2008-03-13T23:22:08Z","isPatch":true,"sender":{"key":"marcandre.lureau@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9381?v=4"},"body":"Hi\n\nOn Fri, Mar 14, 2008 at 12:18 AM, Martin Langhoff\n wrote:\n> > - unless ($last_branch) {\n> > + if (-f \"$git_dir/$last_branch\") {\n>\n> Note that the file won't exist there in any modern git. It will be in\n> $git_dir/refs/heads/$last_branch. Did you test this patch?\n>\n\ngit-symbolic-ref HEAD do return \"refs/heads/master\" on my initial\nin-the-middle checkout (I still have a copy),\n\nSo it seems correct for now, but i'll change it to use rev-parse\ninstead as it seems more correct.\n\nRegards\n\n-- \nMarc-André Lureau\n"},{"id":"72004","messageId":"47D9B875.50305@catalyst.net.nz","threadId":"12675","inReplyTo":"7vr6eew70a.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsimport: fix initial checkout","fromName":"Martin Langhoff","fromEmail":"martin@catalyst.net.nz","sentAt":"2008-03-13T23:27:49Z","receivedAt":"2008-03-13T23:27:49Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"Junio C Hamano wrote:\n> Martin, it may not even be in $git_dir/refs/heads/$last_branch ;-) The\n> refs can be packed.\n\nYes. I was suggesting git-ref-parse to support packed refs, but this\nslipped ;-)\n\n> By the way, doesn't cvsimport fail when your HEAD is detached with this\n> code?\n\nPerhaps. Importing on a working repo with -i is a bit messy, and\ndetached heads may well break it. I'm not that conversant on how\ndetached heads look ;-)\n\n> I always have cvsimport update the pristine upstream branch and rebase my\n> work against it, so I never have the branch cvsimport updates checked\n> out, and for meit seems to work wonderfully (well, at least as wonderful\n> as a workflow that involves any CVS in it could be). I do not see a\n> reason why it should not to work similarly well when my HEAD is detached..\n\nsounds reasonable.\n\n\n\nm\n\n-- \n-----------------------------------------------------------------------\nMartin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington\nWEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St\nNZ: +64(4)916-7224 MOB: +64(21)364-017 UK: 0845 868 5733 ext 7224\n Make things as simple as possible, but no simpler - Einstein\n-----------------------------------------------------------------------\n"},{"id":"72005","messageId":"47D9B8C3.7000808@catalyst.net.nz","threadId":"12675","inReplyTo":"e29894ca0803131604qa61adfbo22ff75d076feb899@mail.gmail.com","subject":"Re: [PATCH] git-cvsimport: fix initial checkout","fromName":"Martin Langhoff","fromEmail":"martin@catalyst.net.nz","sentAt":"2008-03-13T23:29:07Z","receivedAt":"2008-03-13T23:29:07Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"Marc-André Lureau wrote:\n> Yeah, it failed in the middle of a ~4h import, I did not restart it.\n\nthere are no guarantees then of correctness (there seldom are, but a\nfailed import can leave your repo in a number of odd states...).\n\nI'd suggest parsecvs for the initial import of a messy repo. You can\ncontinue to do incrementas with cvsimport after the initial import.\n\ncheers,\n\n\n\nm\n\n-- \n-----------------------------------------------------------------------\nMartin @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington\nWEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St\nNZ: +64(4)916-7224 MOB: +64(21)364-017 UK: 0845 868 5733 ext 7224\n Make things as simple as possible, but no simpler - Einstein\n-----------------------------------------------------------------------\n"}]}