From: Pete Wyckoff <hidden> Date: 2016-06-15 22:50:36
This is the second submission of git-p4 fixes and enhancements.
The first of these 8 patches adds a test script. The rest
fix problems with git-p4 and add small featurest-p4: test script
git-p4: fix key error for p4 problem
git-p4: add missing newline in initial import message
git-p4: accommodate new move/delete type in p4
git-p4: reinterpret confusing p4 message
git-p4: better message for "git-p4 sync" when not cloned
git-p4: decode p4 wildcard characters
git-p4: support clone --bare
contrib/fast-import/git-p4 | 60 ++++++++++++++++++++++-----
t/t9800-git-p4.sh | 100 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 149 insertions(+), 11 deletions(-)
Changes from v1.
Includes acks from Tor Arvid for 3, 4, 6, 8.
Includes review changes from Junio on 1, and from Tor Arvid
on 7.
1/8: test script
Review comments from Junio.
1. Beautify icky p4 and p4d existence check.
2. Remove p4d initialization sleep. It binds and listens
to the port nicely before daemonizing.
7/8: decode p4 wildcard characters
Tor Arvid points out that windows cannot have * in filenames.
This is not something I can test, but at least avoid the situation
on that platform.
@@ -1429,7 +1429,7 @@ class P4Sync(Command): print "Doing initial import of %s from revision %s into %s" % (' '.join(self.depotPaths), revision, self.branch) details = { "user" : "git perforce import user", "time" : int(time.time()) }- details["desc"] = ("Initial import of %s from the state at revision %s"+ details["desc"] = ("Initial import of %s from the state at revision %s\n" % (' '.join(self.depotPaths), revision)) details["change"] = revision newestRevision = 0
From: Pete Wyckoff <hidden> Date: 2016-06-15 22:50:36
Some p4 failures result in an error, but the info['code'] is not
set. These include a bad p4 executable, or a core dump from p4,
and other odd internal errors where p4 fails to generate proper
marshaled output.
Make sure the info key exists before using it to avoid a python
traceback.
Signed-off-by: Pete Wyckoff <redacted>
---
contrib/fast-import/git-p4 | 5 ++++-
t/t9800-git-p4.sh | 13 +++++++++++++
2 files changed, 17 insertions(+), 1 deletions(-)
@@ -1440,10 +1440,13 @@ class P4Sync(Command): % (p, revision) for p in self.depotPaths])):- if info['code'] == 'error':+ if 'code' in info and info['code'] == 'error': sys.stderr.write("p4 returned an error: %s\n" % info['data']) sys.exit(1)+ if 'p4ExitCode' in info:+ sys.stderr.write("p4 exitcode: %s\n" % info['p4ExitCode'])+ sys.exit(1) change = int(info["change"])
From: Pete Wyckoff <hidden> Date: 2016-06-15 22:50:36
Change 562d53f (2010-11-21) recognized the new move/delete type
for git-p4 sync, but it can also show up in an initial clone and
labels output. Instead of replicating this in three places,
hoist the definition somewhere global.
Signed-off-by: Pete Wyckoff <redacted>
Acked-By: Tor Arvid Lund <redacted>
---
contrib/fast-import/git-p4 | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
@@ -1038,10 +1040,10 @@ class P4Sync(Command): if includeFile: filesForCommit.append(f)- if f['action'] not in ('delete', 'move/delete', 'purge'):- filesToRead.append(f)- else:+ if f['action'] in self.delete_actions: filesToDelete.append(f)+ else:+ filesToRead.append(f) # deleted files... for f in filesToDelete:
@@ -1127,7 +1129,7 @@ class P4Sync(Command): cleanedFiles = {} for info in files:- if info["action"] in ("delete", "purge"):+ if info["action"] in self.delete_actions: continue cleanedFiles[info["depotFile"]] = info["rev"]
@@ -1453,7 +1455,7 @@ class P4Sync(Command): if change > newestRevision: newestRevision = change- if info["action"] in ("delete", "purge"):+ if info["action"] in self.delete_actions: # don't increase the file cnt, otherwise details["depotFile123"] will have gaps! #fileCnt = fileCnt + 1 continue
From: Pete Wyckoff <hidden> Date: 2016-06-15 22:50:36
There are four wildcard characters in p4. Files with these
characters can be added to p4 repos using the "-f" option.
They are stored in %xx notation, and when checked out, p4
converts them back to normal.
This patch does the same thing when importing into git,
converting the four special characters. Without this change,
the files appear with literal %xx in their names.
Be careful not to produce "*" in filenames on windows. That
will fail.
Signed-off-by: Pete Wyckoff <redacted>
---
contrib/fast-import/git-p4 | 18 ++++++++++++++++++
t/t9800-git-p4.sh | 22 ++++++++++++++++++++++
2 files changed, 40 insertions(+), 0 deletions(-)
@@ -884,6 +884,23 @@ class P4Sync(Command): if gitConfig("git-p4.syncFromOrigin") == "false": self.syncWithOrigin = False+ #+ # P4 wildcards are not allowed in filenames. P4 complains+ # if you simply add them, but you can force it with "-f", in+ # which case it translates them into %xx encoding internally.+ # Search for and fix just these four characters. Do % last so+ # that fixing it does not inadvertently create new %-escapes.+ #+ def wildcard_decode(self, path):+ # Cannot have * in a filename in windows; untested as to+ # what p4 would do in such a case.+ if not self.isWindows:+ path = path.replace("%2A", "*")+ path = path.replace("%23", "#") \+ .replace("%40", "@") \+ .replace("%25", "%")+ return path+ def extractFilesFromCommit(self, commit): self.cloneExclude = [re.sub(r"\.\.\.$", "", path) for path in self.cloneExclude]
@@ -962,6 +979,7 @@ class P4Sync(Command): return relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)+ relPath = self.wildcard_decode(relPath) if verbose: sys.stderr.write("%s\n" % relPath)
@@ -58,6 +58,28 @@ test_expect_success 'exit when p4 fails to produce marshaled output' 'test_must_failgrep-qTracebackerrs'+test_expect_success'add p4 files with wildcards in the names''+cd"$cli"&&+echofile-wild-hash>file-wild#hash&&+echofile-wild-star>file-wild\*star&&+echofile-wild-at>file-wild@at&&+echofile-wild-percent>file-wild%percent&&+p4add-ffile-wild*&&+p4submit-d"file wildcards"&&+cd"$TRASH_DIRECTORY"+'++test_expect_success'wildcard files git-p4 clone''+"$GITP4"clone--dest="$git"//depot&&+cd"$git"&&+test-ffile-wild#hash&&+test-ffile-wild\*star&&+test-ffile-wild@at&&+test-ffile-wild%percent&&+cd"$TRASH_DIRECTORY"&&+rm-rf"$git"&&mkdir"$git"+'+ test_expect_success'shutdown''pid=`pgrep-fp4d`&&test-n"$pid"&&
From: Pete Wyckoff <hidden> Date: 2016-06-15 22:50:36
Just like git clone --bare, build a .git directory but no
checked out files.
Signed-off-by: Pete Wyckoff <redacted>
Acked-By: Tor Arvid Lund <redacted>
---
contrib/fast-import/git-p4 | 17 +++++++++++++----
t/t9800-git-p4.sh | 10 ++++++++++
2 files changed, 23 insertions(+), 4 deletions(-)
From: Pete Wyckoff <hidden> Date: 2016-06-15 22:50:36
Error output will look like this:
glom$ git p4 clone //deopt
Importing from //deopt into .
Reinitialized existing Git repository in /tmp/x/.git/
Doing initial import of //deopt from revision #head into refs/remotes/p4/master
p4 returned an error: //deopt/... - must refer to client glom.
This particular p4 error is misleading.
Perhaps the depot path was misspelled.
Depot path: //deopt
Signed-off-by: Pete Wyckoff <redacted>
---
contrib/fast-import/git-p4 | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
@@ -1445,6 +1445,10 @@ class P4Sync(Command): if 'code' in info and info['code'] == 'error': sys.stderr.write("p4 returned an error: %s\n" % info['data'])+ if info['data'].find("must refer to client") >= 0:+ sys.stderr.write("This particular p4 error is misleading.\n")+ sys.stderr.write("Perhaps the depot path was misspelled.\n");+ sys.stderr.write("Depot path: %s\n" % " ".join(self.depotPaths)) sys.exit(1) if 'p4ExitCode' in info: sys.stderr.write("p4 exitcode: %s\n" % info['p4ExitCode'])
From: Pete Wyckoff <hidden> Date: 2016-06-15 22:50:36
A common error is to do "git-p4 sync" in a repository that
was not initialized by "git-p4 clone". There will be no
p4 refs. The error message in this case is a traceback
for an assertion, which is confusing.
Change it instead to explain the likely problem.
Signed-off-by: Pete Wyckoff <redacted>
Acked-By: Tor Arvid Lund <redacted>
---
contrib/fast-import/git-p4 | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
@@ -1676,6 +1676,8 @@ class P4Sync(Command): changes.sort() else:+ if not self.p4BranchesInGit:+ die("No remote p4 branches. Perhaps you never did \"git p4 clone\" in here."); if self.verbose: print "Getting p4 changes for %s...%s" % (', '.join(self.depotPaths), self.changeRange)
From: Junio C Hamano <hidden> Date: 2016-06-15 22:50:37
Pete Wyckoff [off-list ref] writes:
Change 562d53f (2010-11-21) recognized the new move/delete type
for git-p4 sync, but it can also show up in an initial clone and
labels output. Instead of replicating this in three places,
hoist the definition somewhere global.
I think the said commit is from January not November, though.
No need to resend---I'll fix the comment up in place.
Thanks.
From: Junio C Hamano <hidden> Date: 2016-06-15 22:50:37
Pete Wyckoff [off-list ref] writes:
There are four wildcard characters in p4. Files with these
characters can be added to p4 repos using the "-f" option.
They are stored in %xx notation, and when checked out, p4
converts them back to normal.
This patch does the same thing when importing into git,
converting the four special characters. Without this change,
the files appear with literal %xx in their names.
Be careful not to produce "*" in filenames on windows. That
will fail.
+ # P4 wildcards are not allowed in filenames. P4 complains
+ # if you simply add them, but you can force it with "-f", in
+ # which case it translates them into %xx encoding internally.
+ # Search for and fix just these four characters. Do % last so
+ # that fixing it does not inadvertently create new %-escapes.
+ #
+ def wildcard_decode(self, path):
+ # Cannot have * in a filename in windows; untested as to
+ # what p4 would do in such a case.
+ if not self.isWindows:
+ path = path.replace("%2A", "*")
I'll queue the patch as-is, but perhaps we can ask for help from people
who have access to P4 on both non-Windows and Windows to run a small test
to determine what happens in the native client?
1. On a non-Windows client, add a path with '*' in it to the depot;
perhaps "p4 add" might fail at this point, in which case we don't
need to worry about this issue at all.
2. Create a p4 client on Windows against that depot, and sync it; unless
the previous step failed, we will see what happens (I would imagine it
either dies or mangles the pathname and warns), so that we have
something to emulate.
and then the quoted part can be further refined in a separate patch later.
Thanks.
From: Tor Arvid Lund <hidden> Date: 2016-06-15 22:50:38
On Sat, Feb 19, 2011 at 2:17 PM, Pete Wyckoff [off-list ref] wrote:
Error output will look like this:
glom$ git p4 clone //deopt
Importing from //deopt into .
Reinitialized existing Git repository in /tmp/x/.git/
Doing initial import of //deopt from revision #head into refs/remotes/p4/master
p4 returned an error: //deopt/... - must refer to client glom.
This particular p4 error is misleading.
Perhaps the depot path was misspelled.
Depot path: //deopt
Signed-off-by: Pete Wyckoff <redacted>
if 'code' in info and info['code'] == 'error':
sys.stderr.write("p4 returned an error: %s\n"
% info['data'])
+ if info['data'].find("must refer to client") >= 0:
+ sys.stderr.write("This particular p4 error is misleading.\n")
+ sys.stderr.write("Perhaps the depot path was misspelled.\n");
+ sys.stderr.write("Depot path: %s\n" % " ".join(self.depotPaths))
sys.exit(1)
if 'p4ExitCode' in info:
sys.stderr.write("p4 exitcode: %s\n" % info['p4ExitCode'])
--
1.7.4.1
From: Pete Wyckoff <hidden> Date: 2016-06-15 22:50:39
gitster@pobox.com wrote on Mon, 21 Feb 2011 15:32 -0800:
Pete Wyckoff [off-list ref] writes:
quoted
There are four wildcard characters in p4. Files with these
characters can be added to p4 repos using the "-f" option.
They are stored in %xx notation, and when checked out, p4
converts them back to normal.
This patch does the same thing when importing into git,
converting the four special characters. Without this change,
the files appear with literal %xx in their names.
Be careful not to produce "*" in filenames on windows. That
will fail.
quoted
+ # P4 wildcards are not allowed in filenames. P4 complains
+ # if you simply add them, but you can force it with "-f", in
+ # which case it translates them into %xx encoding internally.
+ # Search for and fix just these four characters. Do % last so
+ # that fixing it does not inadvertently create new %-escapes.
+ #
+ def wildcard_decode(self, path):
+ # Cannot have * in a filename in windows; untested as to
+ # what p4 would do in such a case.
+ if not self.isWindows:
+ path = path.replace("%2A", "*")
I'll queue the patch as-is, but perhaps we can ask for help from people
who have access to P4 on both non-Windows and Windows to run a small test
to determine what happens in the native client?
1. On a non-Windows client, add a path with '*' in it to the depot;
perhaps "p4 add" might fail at this point, in which case we don't
need to worry about this issue at all.
2. Create a p4 client on Windows against that depot, and sync it; unless
the previous step failed, we will see what happens (I would imagine it
either dies or mangles the pathname and warns), so that we have
something to emulate.
and then the quoted part can be further refined in a separate patch later.
I tried this myself in a VM when Tor Arvid pointed out the
problem with Windows:
http://article.gmane.org/gmane.comp.version-control.git/166374
1. "*" is acceptable in filenames, but users must use "p4 add -f"
to indicate that they really want that wildcard character in
the filename.
2. Windows clients fail to create the file in "p4 sync". The
error is:
open for write: c:\Documents and Settings\Administrator\Desktop\file*star:
The filename, directory name, or volume label syntax is incorrect.
The behavior for git-p4 I chose is to sync the file but leave
the name with its encoded %2A. If we think it is better to
duplicate p4's failure, we can simply try to create the file
and let the OS produce the same error message.
-- Pete
From: Tor Arvid Lund <hidden> Date: 2016-06-15 22:50:39
On Thu, Feb 24, 2011 at 1:12 PM, Pete Wyckoff [off-list ref] wrote:
gitster@pobox.com wrote on Mon, 21 Feb 2011 15:32 -0800:
quoted
Pete Wyckoff [off-list ref] writes:
quoted
There are four wildcard characters in p4. Files with these
characters can be added to p4 repos using the "-f" option.
They are stored in %xx notation, and when checked out, p4
converts them back to normal.
This patch does the same thing when importing into git,
converting the four special characters. Without this change,
the files appear with literal %xx in their names.
Be careful not to produce "*" in filenames on windows. That
will fail.
quoted
+ # P4 wildcards are not allowed in filenames. P4 complains
+ # if you simply add them, but you can force it with "-f", in
+ # which case it translates them into %xx encoding internally.
+ # Search for and fix just these four characters. Do % last so
+ # that fixing it does not inadvertently create new %-escapes.
+ #
+ def wildcard_decode(self, path):
+ # Cannot have * in a filename in windows; untested as to
+ # what p4 would do in such a case.
+ if not self.isWindows:
+ path = path.replace("%2A", "*")
I'll queue the patch as-is, but perhaps we can ask for help from people
who have access to P4 on both non-Windows and Windows to run a small test
to determine what happens in the native client?
1. On a non-Windows client, add a path with '*' in it to the depot;
perhaps "p4 add" might fail at this point, in which case we don't
need to worry about this issue at all.
2. Create a p4 client on Windows against that depot, and sync it; unless
the previous step failed, we will see what happens (I would imagine it
either dies or mangles the pathname and warns), so that we have
something to emulate.
and then the quoted part can be further refined in a separate patch later.
I tried this myself in a VM when Tor Arvid pointed out the
problem with Windows:
http://article.gmane.org/gmane.comp.version-control.git/166374
1. "*" is acceptable in filenames, but users must use "p4 add -f"
to indicate that they really want that wildcard character in
the filename.
2. Windows clients fail to create the file in "p4 sync". The
error is:
open for write: c:\Documents and Settings\Administrator\Desktop\file*star:
The filename, directory name, or volume label syntax is incorrect.
The behavior for git-p4 I chose is to sync the file but leave
the name with its encoded %2A. If we think it is better to
duplicate p4's failure, we can simply try to create the file
and let the OS produce the same error message.
Yeah, I was thinking... what happens now if we do:
1) Create "my*file" in linux, and submit.
2) git-p4 sync from windows, and get my%2Afile on windows.
3) modify my%2Afile and do git commit.
4) git-p4 submit
I haven't had time to test right now, but maybe p4 will not recognise
my%2Afile (or try to check it in as my%252Afile (replacing the '%'
character) or something like that? (Or maybe I just haven't had enough
coffee today :-/ )
-- Tor Arvid