{"thread":{"id":"9421","subject":"[PATCH] git-p4: Fix support for symlinks.","startedAt":"2007-08-07T08:25:47Z","lastAt":"2007-08-08T01:36:25Z","messageCount":4,"participants":["Simon Hausmann","Junio C Hamano","Brian Swetland","Scott Lamb"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"50103","messageId":"200708071025.47965.simon@lst.de","threadId":"9421","inReplyTo":null,"subject":"[PATCH] git-p4: Fix support for symlinks.","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2007-08-07T08:25:47Z","receivedAt":"2007-08-07T08:25:47Z","isPatch":true,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"Detect symlinks as file type, set the git file mode accordingly and strip off the trailing newline in the p4 print output.\n\nSigned-off-by: Simon Hausmann \n---\n contrib/fast-import/git-p4 | 8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 41e86e7..9c6f911 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -839,11 +839,15 @@ class P4Sync(Command):\n if file[\"action\"] == \"delete\":\n self.gitStream.write(\"D %s\\n\" % relPath)\n else:\n+ data = file['data']\n+\n mode = 644\n if file[\"type\"].startswith(\"x\"):\n mode = 755\n-\n- data = file['data']\n+ elif file[\"type\"] == \"symlink\":\n+ mode = 120000\n+ # p4 print on a symlink contains \"target\\n\", so strip it off\n+ data = data[:-1]\n \n if self.isWindows and file[\"type\"].endswith(\"text\"):\n data = data.replace(\"\\r\\n\", \"\\n\")\n-- \n1.5.3.rc3.91.g5c75\n\n"},{"id":"50104","messageId":"7vtzrb68kq.fsf@assigned-by-dhcp.cox.net","threadId":"9421","inReplyTo":"200708071025.47965.simon@lst.de","subject":"Re: [PATCH] git-p4: Fix support for symlinks.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-07T08:40:53Z","receivedAt":"2007-08-07T08:40:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Simon Hausmann writes:\n\n> Detect symlinks as file type, set the git file mode accordingly and strip off the trailing newline in the p4 print output.\n>\n> Signed-off-by: Simon Hausmann \n> ---\n> contrib/fast-import/git-p4 | 8 ++++++--\n> 1 files changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index 41e86e7..9c6f911 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -839,11 +839,15 @@ class P4Sync(Command):\n> if file[\"action\"] == \"delete\":\n> self.gitStream.write(\"D %s\\n\" % relPath)\n> else:\n> + data = file['data']\n> +\n> mode = 644\n> if file[\"type\"].startswith(\"x\"):\n> mode = 755\n> -\n> - data = file['data']\n> + elif file[\"type\"] == \"symlink\":\n> + mode = 120000\n> + # p4 print on a symlink contains \"target\\n\", so strip it off\n> + data = data[:-1]\n> \n> if self.isWindows and file[\"type\"].endswith(\"text\"):\n> data = data.replace(\"\\r\\n\", \"\\n\")\n\nThanks for a quick fix.\n\nBrian, does this resolve the issue for you? I do not have an\naccess to p4 myself so I won't make a good judge in this area\nmyself. An Ack is appreciated.\n\nSimon, just a style nit.\n\nEvery time I see decimal integers 644 and/or 755, it interrupts\nmy flow of thought and forces me to read the change and its\nsurrounding text needlessly carefully.\n\nIf you read the code, you can see that this \"mode\" variable is\nformatted with (\"%d\" % mode) for writing out, and is not used as\npermission bit pattern in any bitwise operations, so you can\ntell that these constants _are_ safe. But it takes extra\nefforts to convince yourself that they indeed are.\n\nI would prefer this kind of thing to be written as either:\n\n\tmode = 0644\n \"%o\" % mode\n\nor\n\n\tmode = \"644\"\n \"%s\" % mode\n\nto make it clear that the author knew what he was doing when he\nwrote the code.\n"},{"id":"50108","messageId":"20070807091049.GA13308@bulgaria","threadId":"9421","inReplyTo":"7vtzrb68kq.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-p4: Fix support for symlinks.","fromName":"Brian Swetland","fromEmail":"swetland@google.com","sentAt":"2007-08-07T09:10:49Z","receivedAt":"2007-08-07T09:10:49Z","isPatch":true,"sender":{"key":"swetland@google.com","avatar":"https://gravatar.com/avatar/b26b7c772097c55d8febb0fc027dd2ef577a05994a321819f49ab3c9153ac8b1?d=mp&s=160"},"body":"[Junio C Hamano ]\n> Simon Hausmann writes:\n> \n> [ patch for correct symlink handling ]\n> \n> Thanks for a quick fix.\n> \n> Brian, does this resolve the issue for you? I do not have an\n> access to p4 myself so I won't make a good judge in this area\n> myself. An Ack is appreciated.\n\nAck.\n\nLooks good here. I can now sync from the p4 tree into git, check out\nfrom git and do a clean build, and everything's happy.\n\nThanks for the quick fix, Simon!\n\nOne observation on git-p4 -- it's a little memory hungry when processing\nlarge syncs. I haven't tried incremental syncs on top of the initial\none though -- if it's only the initial that's expensive it's not that\nbig a deal.\n\nIt seemed to top out around 988MB resident. The branch I was importing\nis about 562MB when checked out and the resulting git repository is\nabout 175MB.\n\nBrian\n"},{"id":"50160","messageId":"46B91E19.9010005@slamb.org","threadId":"9421","inReplyTo":"20070807091049.GA13308@bulgaria","subject":"Re: [PATCH] git-p4: Fix support for symlinks.","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-08-08T01:36:25Z","receivedAt":"2007-08-08T01:36:25Z","isPatch":true,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"Brian Swetland wrote:\n> One observation on git-p4 -- it's a little memory hungry when processing\n> large syncs. I haven't tried incremental syncs on top of the initial\n> one though -- if it's only the initial that's expensive it's not that\n> big a deal.\n> \n> It seemed to top out around 988MB resident. The branch I was importing\n> is about 562MB when checked out and the resulting git repository is\n> about 175MB.\n\nWhile importing each change, I think git-p4 puts into memory two copies \nof the contents of all changed files, one in p4CmdList and one in \nreadP4Files. (That's the raw contents, not just the delta.) I don't \nthink there's any fundamental reason it couldn't stream them instead.\n\nSo incremental syncs may or may not take less memory. If the first \nchange imports a huge project and no subsequent change ever touches all \nthose files at once, then yeah. But if, say, you periodically change the \ncopyright dates in all files in the repository, you'll have this memory \nusage whenever syncing such a change.\n\nAs long as we're listing git-p4 complaints, here are a couple of mine:\n\n1) coding style. *self-nag* Simon Hausmann mentioned he was happy to \naccept patches...and I made one up a while ago; I just need to do a \nmerge and final check that I haven't broken anything before sending it off.\n\n2) it breaks on tempfile purges. My previous employer has these in their \nrepository, and I think for the moment they're working around it by \ntreating a \"purge\" as a \"delete\". If I read the Perforce documentation \nright, though, only the latest version of a tempfile's contents is kept \nin the repository anyway. Their history can't be captured accurately, so \nthe proper thing is probably to omit tempfiles entirely. (And deleting \nfiles when they become tempfiles and creating files when they become a \nnormal type.)\n\nBest regards,\nScott\n\n-- \nScott Lamb \n"}]}