* [PATCH] git-p4: Fix support for symlinks.
@ 2007-08-07 8:25 Simon Hausmann
2007-08-07 8:40 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Simon Hausmann @ 2007-08-07 8:25 UTC (permalink / raw
To: git; +Cc: Han-Wen Nienhuys, Brian Swetland
[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]
Detect symlinks as file type, set the git file mode accordingly and strip off the trailing newline in the p4 print output.
Signed-off-by: Simon Hausmann <simon@lst.de>
---
contrib/fast-import/git-p4 | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 41e86e7..9c6f911 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -839,11 +839,15 @@ class P4Sync(Command):
if file["action"] == "delete":
self.gitStream.write("D %s\n" % relPath)
else:
+ data = file['data']
+
mode = 644
if file["type"].startswith("x"):
mode = 755
-
- data = file['data']
+ elif file["type"] == "symlink":
+ mode = 120000
+ # p4 print on a symlink contains "target\n", so strip it off
+ data = data[:-1]
if self.isWindows and file["type"].endswith("text"):
data = data.replace("\r\n", "\n")
--
1.5.3.rc3.91.g5c75
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] git-p4: Fix support for symlinks.
2007-08-07 8:25 [PATCH] git-p4: Fix support for symlinks Simon Hausmann
@ 2007-08-07 8:40 ` Junio C Hamano
2007-08-07 9:10 ` Brian Swetland
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2007-08-07 8:40 UTC (permalink / raw
To: Simon Hausmann; +Cc: git, Han-Wen Nienhuys, Brian Swetland
Simon Hausmann <simon@lst.de> writes:
> Detect symlinks as file type, set the git file mode accordingly and strip off the trailing newline in the p4 print output.
>
> Signed-off-by: Simon Hausmann <simon@lst.de>
> ---
> contrib/fast-import/git-p4 | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 41e86e7..9c6f911 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -839,11 +839,15 @@ class P4Sync(Command):
> if file["action"] == "delete":
> self.gitStream.write("D %s\n" % relPath)
> else:
> + data = file['data']
> +
> mode = 644
> if file["type"].startswith("x"):
> mode = 755
> -
> - data = file['data']
> + elif file["type"] == "symlink":
> + mode = 120000
> + # p4 print on a symlink contains "target\n", so strip it off
> + data = data[:-1]
>
> if self.isWindows and file["type"].endswith("text"):
> data = data.replace("\r\n", "\n")
Thanks for a quick fix.
Brian, does this resolve the issue for you? I do not have an
access to p4 myself so I won't make a good judge in this area
myself. An Ack is appreciated.
Simon, just a style nit.
Every time I see decimal integers 644 and/or 755, it interrupts
my flow of thought and forces me to read the change and its
surrounding text needlessly carefully.
If you read the code, you can see that this "mode" variable is
formatted with ("%d" % mode) for writing out, and is not used as
permission bit pattern in any bitwise operations, so you can
tell that these constants _are_ safe. But it takes extra
efforts to convince yourself that they indeed are.
I would prefer this kind of thing to be written as either:
mode = 0644
"%o" % mode
or
mode = "644"
"%s" % mode
to make it clear that the author knew what he was doing when he
wrote the code.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] git-p4: Fix support for symlinks.
2007-08-07 8:40 ` Junio C Hamano
@ 2007-08-07 9:10 ` Brian Swetland
2007-08-08 1:36 ` Scott Lamb
0 siblings, 1 reply; 5+ messages in thread
From: Brian Swetland @ 2007-08-07 9:10 UTC (permalink / raw
To: Junio C Hamano; +Cc: Simon Hausmann, git, Han-Wen Nienhuys
[Junio C Hamano <gitster@pobox.com>]
> Simon Hausmann <simon@lst.de> writes:
>
> [ patch for correct symlink handling ]
>
> Thanks for a quick fix.
>
> Brian, does this resolve the issue for you? I do not have an
> access to p4 myself so I won't make a good judge in this area
> myself. An Ack is appreciated.
Ack.
Looks good here. I can now sync from the p4 tree into git, check out
from git and do a clean build, and everything's happy.
Thanks for the quick fix, Simon!
One observation on git-p4 -- it's a little memory hungry when processing
large syncs. I haven't tried incremental syncs on top of the initial
one though -- if it's only the initial that's expensive it's not that
big a deal.
It seemed to top out around 988MB resident. The branch I was importing
is about 562MB when checked out and the resulting git repository is
about 175MB.
Brian
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] git-p4: Fix support for symlinks.
@ 2007-08-07 10:28 Simon Hausmann
0 siblings, 0 replies; 5+ messages in thread
From: Simon Hausmann @ 2007-08-07 10:28 UTC (permalink / raw
To: Junio C Hamano; +Cc: git
Detect symlinks as file type, set the git file mode accordingly and strip off the trailing newline in the p4 print output.
Make the mode handling a bit more readable at the same time.
Signed-off-by: Simon Hausmann <simon@lst.de>
Acked-by: Brian Swetland <swetland@google.com>
---
contrib/fast-import/git-p4 | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 41e86e7..3cbb2da 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -839,16 +839,20 @@ class P4Sync(Command):
if file["action"] == "delete":
self.gitStream.write("D %s\n" % relPath)
else:
- mode = 644
- if file["type"].startswith("x"):
- mode = 755
-
data = file['data']
+ mode = "644"
+ if file["type"].startswith("x"):
+ mode = "755"
+ elif file["type"] == "symlink":
+ mode = "120000"
+ # p4 print on a symlink contains "target\n", so strip it off
+ data = data[:-1]
+
if self.isWindows and file["type"].endswith("text"):
data = data.replace("\r\n", "\n")
- self.gitStream.write("M %d inline %s\n" % (mode, relPath))
+ self.gitStream.write("M %s inline %s\n" % (mode, relPath))
self.gitStream.write("data %s\n" % len(data))
self.gitStream.write(data)
self.gitStream.write("\n")
--
1.5.3.rc3.91.g5c75
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] git-p4: Fix support for symlinks.
2007-08-07 9:10 ` Brian Swetland
@ 2007-08-08 1:36 ` Scott Lamb
0 siblings, 0 replies; 5+ messages in thread
From: Scott Lamb @ 2007-08-08 1:36 UTC (permalink / raw
To: Brian Swetland; +Cc: Junio C Hamano, Simon Hausmann, git, Han-Wen Nienhuys
Brian Swetland wrote:
> One observation on git-p4 -- it's a little memory hungry when processing
> large syncs. I haven't tried incremental syncs on top of the initial
> one though -- if it's only the initial that's expensive it's not that
> big a deal.
>
> It seemed to top out around 988MB resident. The branch I was importing
> is about 562MB when checked out and the resulting git repository is
> about 175MB.
While importing each change, I think git-p4 puts into memory two copies
of the contents of all changed files, one in p4CmdList and one in
readP4Files. (That's the raw contents, not just the delta.) I don't
think there's any fundamental reason it couldn't stream them instead.
So incremental syncs may or may not take less memory. If the first
change imports a huge project and no subsequent change ever touches all
those files at once, then yeah. But if, say, you periodically change the
copyright dates in all files in the repository, you'll have this memory
usage whenever syncing such a change.
As long as we're listing git-p4 complaints, here are a couple of mine:
1) coding style. *self-nag* Simon Hausmann mentioned he was happy to
accept patches...and I made one up a while ago; I just need to do a
merge and final check that I haven't broken anything before sending it off.
2) it breaks on tempfile purges. My previous employer has these in their
repository, and I think for the moment they're working around it by
treating a "purge" as a "delete". If I read the Perforce documentation
right, though, only the latest version of a tempfile's contents is kept
in the repository anyway. Their history can't be captured accurately, so
the proper thing is probably to omit tempfiles entirely. (And deleting
files when they become tempfiles and creating files when they become a
normal type.)
Best regards,
Scott
--
Scott Lamb <http://www.slamb.org/>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-08-08 1:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-07 8:25 [PATCH] git-p4: Fix support for symlinks Simon Hausmann
2007-08-07 8:40 ` Junio C Hamano
2007-08-07 9:10 ` Brian Swetland
2007-08-08 1:36 ` Scott Lamb
-- strict thread matches above, loose matches on Subject: below --
2007-08-07 10:28 Simon Hausmann
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).