* [PATCH v2 0/8] git-p4 fixes and enhancements
@ 2011-02-19 13:17 Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 1/8] git-p4: test script Pete Wyckoff
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:17 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/8] git-p4: test script
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
@ 2011-02-19 13:17 ` Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 2/8] git-p4: fix key error for p4 problem Pete Wyckoff
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:17 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/t9800-git-p4.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 55 insertions(+), 0 deletions(-)
create mode 100755 t/t9800-git-p4.sh
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
new file mode 100755
index 0000000..2d354f8
--- /dev/null
+++ b/t/t9800-git-p4.sh
@@ -0,0 +1,55 @@
+#!/bin/sh
+
+test_description='git-p4 tests'
+
+. ./test-lib.sh
+
+( p4 -h && p4d -h ) >/dev/null 2>&1 || {
+ skip_all='skipping git-p4 tests; no p4 or p4d'
+ test_done
+}
+
+GITP4=$GIT_BUILD_DIR/contrib/fast-import/git-p4
+P4DPORT=10669
+
+db="$TRASH_DIRECTORY/db"
+cli="$TRASH_DIRECTORY/cli"
+git="$TRASH_DIRECTORY/git"
+
+test_debug 'echo p4d -q -d -r "$db" -p $P4DPORT'
+test_expect_success setup '
+ mkdir -p "$db" &&
+ p4d -q -d -r "$db" -p $P4DPORT &&
+ mkdir -p "$cli" &&
+ mkdir -p "$git" &&
+ export P4PORT=localhost:$P4DPORT
+'
+
+test_expect_success 'add p4 files' '
+ cd "$cli" &&
+ p4 client -i <<-EOF &&
+ Client: client
+ Description: client
+ Root: $cli
+ View: //depot/... //client/...
+ EOF
+ export P4CLIENT=client &&
+ echo file1 >file1 &&
+ p4 add file1 &&
+ p4 submit -d "file1" &&
+ cd "$TRASH_DIRECTORY"
+'
+
+test_expect_success 'basic git-p4 clone' '
+ "$GITP4" clone --dest="$git" //depot &&
+ rm -rf "$git" && mkdir "$git"
+'
+
+test_expect_success 'shutdown' '
+ pid=`pgrep -f p4d` &&
+ test -n "$pid" &&
+ test_debug "ps wl `echo $pid`" &&
+ kill $pid
+'
+
+test_done
--
1.7.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/8] git-p4: fix key error for p4 problem
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 1/8] git-p4: test script Pete Wyckoff
@ 2011-02-19 13:17 ` Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 3/8] git-p4: add missing newline in initial import message Pete Wyckoff
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:17 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
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 <pw@padd.com>
---
contrib/fast-import/git-p4 | 5 ++++-
t/t9800-git-p4.sh | 13 +++++++++++++
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 04ce7e3..2fefea4 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -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"])
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 2d354f8..c1ea4d4 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -45,6 +45,19 @@ test_expect_success 'basic git-p4 clone' '
rm -rf "$git" && mkdir "$git"
'
+test_expect_success 'exit when p4 fails to produce marshaled output' '
+ badp4dir="$TRASH_DIRECTORY/badp4dir" &&
+ mkdir -p "$badp4dir" &&
+ cat >"$badp4dir"/p4 <<-EOF &&
+ #!$SHELL_PATH
+ exit 1
+ EOF
+ chmod 755 "$badp4dir"/p4 &&
+ PATH="$badp4dir:$PATH" "$GITP4" clone --dest="$git" //depot >errs 2>&1 ; retval=$? &&
+ test $retval -eq 1 &&
+ test_must_fail grep -q Traceback errs
+'
+
test_expect_success 'shutdown' '
pid=`pgrep -f p4d` &&
test -n "$pid" &&
--
1.7.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/8] git-p4: add missing newline in initial import message
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 1/8] git-p4: test script Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 2/8] git-p4: fix key error for p4 problem Pete Wyckoff
@ 2011-02-19 13:17 ` Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 4/8] git-p4: accommodate new move/delete type in p4 Pete Wyckoff
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:17 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
Signed-off-by: Pete Wyckoff <pw@padd.com>
Acked-By: Tor Arvid Lund <torarvid@gmail.com>
---
contrib/fast-import/git-p4 | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 2fefea4..d2ba215 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -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
--
1.7.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/8] git-p4: accommodate new move/delete type in p4
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
` (2 preceding siblings ...)
2011-02-19 13:17 ` [PATCH v2 3/8] git-p4: add missing newline in initial import message Pete Wyckoff
@ 2011-02-19 13:17 ` Pete Wyckoff
2011-02-21 23:32 ` Junio C Hamano
2011-02-19 13:17 ` [PATCH v2 5/8] git-p4: reinterpret confusing p4 message Pete Wyckoff
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:17 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
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 <pw@padd.com>
Acked-By: Tor Arvid Lund <torarvid@gmail.com>
---
contrib/fast-import/git-p4 | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index d2ba215..db19b17 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -834,6 +834,8 @@ class P4Submit(Command):
return True
class P4Sync(Command):
+ delete_actions = ( "delete", "move/delete", "purge" )
+
def __init__(self):
Command.__init__(self)
self.options = [
@@ -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
--
1.7.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 5/8] git-p4: reinterpret confusing p4 message
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
` (3 preceding siblings ...)
2011-02-19 13:17 ` [PATCH v2 4/8] git-p4: accommodate new move/delete type in p4 Pete Wyckoff
@ 2011-02-19 13:17 ` Pete Wyckoff
2011-02-23 8:26 ` Tor Arvid Lund
2011-02-19 13:17 ` [PATCH v2 6/8] git-p4: better message for "git-p4 sync" when not cloned Pete Wyckoff
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:17 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
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 <pw@padd.com>
---
contrib/fast-import/git-p4 | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index db19b17..6b847c4 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -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'])
--
1.7.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 6/8] git-p4: better message for "git-p4 sync" when not cloned
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
` (4 preceding siblings ...)
2011-02-19 13:17 ` [PATCH v2 5/8] git-p4: reinterpret confusing p4 message Pete Wyckoff
@ 2011-02-19 13:17 ` Pete Wyckoff
2011-02-19 13:18 ` [PATCH v2 7/8] git-p4: decode p4 wildcard characters Pete Wyckoff
2011-02-19 13:18 ` [PATCH v2 8/8] git-p4: support clone --bare Pete Wyckoff
7 siblings, 0 replies; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:17 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
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 <pw@padd.com>
Acked-By: Tor Arvid Lund <torarvid@gmail.com>
---
contrib/fast-import/git-p4 | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 6b847c4..04e6c3d 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -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)
--
1.7.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 7/8] git-p4: decode p4 wildcard characters
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
` (5 preceding siblings ...)
2011-02-19 13:17 ` [PATCH v2 6/8] git-p4: better message for "git-p4 sync" when not cloned Pete Wyckoff
@ 2011-02-19 13:18 ` Pete Wyckoff
2011-02-21 23:32 ` Junio C Hamano
2011-02-19 13:18 ` [PATCH v2 8/8] git-p4: support clone --bare Pete Wyckoff
7 siblings, 1 reply; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:18 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
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 <pw@padd.com>
---
contrib/fast-import/git-p4 | 18 ++++++++++++++++++
t/t9800-git-p4.sh | 22 ++++++++++++++++++++++
2 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 04e6c3d..98597d3 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -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)
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index c1ea4d4..026277a 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -58,6 +58,28 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
test_must_fail grep -q Traceback errs
'
+test_expect_success 'add p4 files with wildcards in the names' '
+ cd "$cli" &&
+ echo file-wild-hash >file-wild#hash &&
+ echo file-wild-star >file-wild\*star &&
+ echo file-wild-at >file-wild@at &&
+ echo file-wild-percent >file-wild%percent &&
+ p4 add -f file-wild* &&
+ p4 submit -d "file wildcards" &&
+ cd "$TRASH_DIRECTORY"
+'
+
+test_expect_success 'wildcard files git-p4 clone' '
+ "$GITP4" clone --dest="$git" //depot &&
+ cd "$git" &&
+ test -f file-wild#hash &&
+ test -f file-wild\*star &&
+ test -f file-wild@at &&
+ test -f file-wild%percent &&
+ cd "$TRASH_DIRECTORY" &&
+ rm -rf "$git" && mkdir "$git"
+'
+
test_expect_success 'shutdown' '
pid=`pgrep -f p4d` &&
test -n "$pid" &&
--
1.7.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 8/8] git-p4: support clone --bare
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
` (6 preceding siblings ...)
2011-02-19 13:18 ` [PATCH v2 7/8] git-p4: decode p4 wildcard characters Pete Wyckoff
@ 2011-02-19 13:18 ` Pete Wyckoff
7 siblings, 0 replies; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-19 13:18 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
Just like git clone --bare, build a .git directory but no
checked out files.
Signed-off-by: Pete Wyckoff <pw@padd.com>
Acked-By: Tor Arvid Lund <torarvid@gmail.com>
---
contrib/fast-import/git-p4 | 17 +++++++++++++----
t/t9800-git-p4.sh | 10 ++++++++++
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 98597d3..725af75 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1776,10 +1776,13 @@ class P4Clone(P4Sync):
help="where to leave result of the clone"),
optparse.make_option("-/", dest="cloneExclude",
action="append", type="string",
- help="exclude depot path")
+ help="exclude depot path"),
+ optparse.make_option("--bare", dest="cloneBare",
+ action="store_true", default=False),
]
self.cloneDestination = None
self.needsGit = False
+ self.cloneBare = False
# This is required for the "append" cloneExclude action
def ensure_value(self, attr, value):
@@ -1819,11 +1822,16 @@ class P4Clone(P4Sync):
self.cloneDestination = self.defaultDestination(args)
print "Importing from %s into %s" % (', '.join(depotPaths), self.cloneDestination)
+
if not os.path.exists(self.cloneDestination):
os.makedirs(self.cloneDestination)
chdir(self.cloneDestination)
- system("git init")
- self.gitdir = os.getcwd() + "/.git"
+
+ init_cmd = [ "git", "init" ]
+ if self.cloneBare:
+ init_cmd.append("--bare")
+ subprocess.check_call(init_cmd)
+
if not P4Sync.run(self, depotPaths):
return False
if self.branch != "master":
@@ -1833,7 +1841,8 @@ class P4Clone(P4Sync):
masterbranch = "refs/heads/p4/master"
if gitBranchExists(masterbranch):
system("git branch master %s" % masterbranch)
- system("git checkout -f")
+ if not self.cloneBare:
+ system("git checkout -f")
else:
print "Could not detect main branch. No checkout/master branch created."
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 026277a..1969e6b 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -80,6 +80,16 @@ test_expect_success 'wildcard files git-p4 clone' '
rm -rf "$git" && mkdir "$git"
'
+test_expect_success 'clone bare' '
+ "$GITP4" clone --dest="$git" --bare //depot &&
+ cd "$git" &&
+ test ! -d .git &&
+ bare=`git config --get core.bare` &&
+ test "$bare" = true &&
+ cd "$TRASH_DIRECTORY" &&
+ rm -rf "$git" && mkdir "$git"
+'
+
test_expect_success 'shutdown' '
pid=`pgrep -f p4d` &&
test -n "$pid" &&
--
1.7.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/8] git-p4: accommodate new move/delete type in p4
2011-02-19 13:17 ` [PATCH v2 4/8] git-p4: accommodate new move/delete type in p4 Pete Wyckoff
@ 2011-02-21 23:32 ` Junio C Hamano
0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2011-02-21 23:32 UTC (permalink / raw
To: Pete Wyckoff; +Cc: Tor Arvid Lund, Vitor Antunes, git
Pete Wyckoff <pw@padd.com> 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters
2011-02-19 13:18 ` [PATCH v2 7/8] git-p4: decode p4 wildcard characters Pete Wyckoff
@ 2011-02-21 23:32 ` Junio C Hamano
2011-02-24 12:12 ` Pete Wyckoff
0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2011-02-21 23:32 UTC (permalink / raw
To: Pete Wyckoff; +Cc: Tor Arvid Lund, Vitor Antunes, git
Pete Wyckoff <pw@padd.com> 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 5/8] git-p4: reinterpret confusing p4 message
2011-02-19 13:17 ` [PATCH v2 5/8] git-p4: reinterpret confusing p4 message Pete Wyckoff
@ 2011-02-23 8:26 ` Tor Arvid Lund
0 siblings, 0 replies; 16+ messages in thread
From: Tor Arvid Lund @ 2011-02-23 8:26 UTC (permalink / raw
To: Pete Wyckoff; +Cc: Junio C Hamano, Vitor Antunes, git
On Sat, Feb 19, 2011 at 2:17 PM, Pete Wyckoff <pw@padd.com> 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 <pw@padd.com>
Acked-by: Tor Arvid Lund <torarvid@gmail.com>
:-)
> ---
> contrib/fast-import/git-p4 | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index db19b17..6b847c4 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -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'])
> --
> 1.7.4.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters
2011-02-21 23:32 ` Junio C Hamano
@ 2011-02-24 12:12 ` Pete Wyckoff
2011-02-24 13:54 ` Tor Arvid Lund
0 siblings, 1 reply; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-24 12:12 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
gitster@pobox.com wrote on Mon, 21 Feb 2011 15:32 -0800:
> Pete Wyckoff <pw@padd.com> 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.
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters
2011-02-24 12:12 ` Pete Wyckoff
@ 2011-02-24 13:54 ` Tor Arvid Lund
2011-02-24 16:40 ` Junio C Hamano
0 siblings, 1 reply; 16+ messages in thread
From: Tor Arvid Lund @ 2011-02-24 13:54 UTC (permalink / raw
To: Pete Wyckoff; +Cc: Junio C Hamano, Vitor Antunes, git
On Thu, Feb 24, 2011 at 1:12 PM, Pete Wyckoff <pw@padd.com> wrote:
> gitster@pobox.com wrote on Mon, 21 Feb 2011 15:32 -0800:
>> Pete Wyckoff <pw@padd.com> 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.
>
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters
2011-02-24 13:54 ` Tor Arvid Lund
@ 2011-02-24 16:40 ` Junio C Hamano
2011-02-27 21:16 ` Pete Wyckoff
0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2011-02-24 16:40 UTC (permalink / raw
To: Tor Arvid Lund; +Cc: Pete Wyckoff, Vitor Antunes, git
Tor Arvid Lund <torarvid@gmail.com> writes:
> On Thu, Feb 24, 2011 at 1:12 PM, Pete Wyckoff <pw@padd.com> wrote:
> ...
> 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 :-/ )
This shares the same issue as "checking files out on case insensitive
filesystems" topic in the other thread. "my*file" may not be usable by
the project when renamed to "my%2Afile", so "git-p4 sync" may want to warn
the user about the path when this happens.
And you need to reverse this quoting upon "git-p4 submit". Does that
happen already?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters
2011-02-24 16:40 ` Junio C Hamano
@ 2011-02-27 21:16 ` Pete Wyckoff
0 siblings, 0 replies; 16+ messages in thread
From: Pete Wyckoff @ 2011-02-27 21:16 UTC (permalink / raw
To: Junio C Hamano; +Cc: Tor Arvid Lund, Vitor Antunes, git
gitster@pobox.com wrote on Thu, 24 Feb 2011 08:40 -0800:
> Tor Arvid Lund <torarvid@gmail.com> writes:
>
> > On Thu, Feb 24, 2011 at 1:12 PM, Pete Wyckoff <pw@padd.com> wrote:
> > ...
> > 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 :-/ )
>
> This shares the same issue as "checking files out on case insensitive
> filesystems" topic in the other thread. "my*file" may not be usable by
> the project when renamed to "my%2Afile", so "git-p4 sync" may want to warn
> the user about the path when this happens.
>
> And you need to reverse this quoting upon "git-p4 submit". Does that
> happen already?
I have not found any testers to try these things on windows.
We have a separate bug in git-p4 for submitting files with
wildcards, windows or unix. This could be fixed anytime in
a separate patch; any takers?
Another unrelated p4-linux-only bug is this:
arf$ echo hello > my%file
arf$ p4 add -f my%file
//depot/my%25file#1 - opened for add
arf$ p4 submit -d 'add my file'
Submitting change 1.
Locking 1 files ...
add //depot/my%25file#1
Change 1 submitted.
arf$ p4 sync
File(s) up-to-date.
arf$ p4 open my%file
my%file - file(s) not on client.
arf$ p4 open my%25file
//depot/my%25file#1 - opened for edit
I can create and add my%file, but cannot edit or delete it. The
"-f" option does not help. It can be edited/deleted using the %25
expansion. I found this same problem on Windows.
Conclusion: p4 is buggy and incomplete with respect to wildcard
characters in filenames already. This particular change does not make
anything worse, and fixes a problem seen in the wild for a filename with
"@". I'd like to hope p4 gets fixed at which point this % issue goes
away, and we can decide what to do with * on windows, following their
lead.
-- Pete
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2011-02-27 21:16 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-19 13:17 [PATCH v2 0/8] git-p4 fixes and enhancements Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 1/8] git-p4: test script Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 2/8] git-p4: fix key error for p4 problem Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 3/8] git-p4: add missing newline in initial import message Pete Wyckoff
2011-02-19 13:17 ` [PATCH v2 4/8] git-p4: accommodate new move/delete type in p4 Pete Wyckoff
2011-02-21 23:32 ` Junio C Hamano
2011-02-19 13:17 ` [PATCH v2 5/8] git-p4: reinterpret confusing p4 message Pete Wyckoff
2011-02-23 8:26 ` Tor Arvid Lund
2011-02-19 13:17 ` [PATCH v2 6/8] git-p4: better message for "git-p4 sync" when not cloned Pete Wyckoff
2011-02-19 13:18 ` [PATCH v2 7/8] git-p4: decode p4 wildcard characters Pete Wyckoff
2011-02-21 23:32 ` Junio C Hamano
2011-02-24 12:12 ` Pete Wyckoff
2011-02-24 13:54 ` Tor Arvid Lund
2011-02-24 16:40 ` Junio C Hamano
2011-02-27 21:16 ` Pete Wyckoff
2011-02-19 13:18 ` [PATCH v2 8/8] git-p4: support clone --bare Pete Wyckoff
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.