[PATCH v2 0/8] git-p4 fixes and enhancements

[PATCH v2 0/8] git-p4 fixes and enhancements

[Pete Wyckoff]

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.

[PATCH v2 1/8] git-p4: test script

[Pete Wyckoff]

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

[PATCH v2 2/8] git-p4: fix key error for p4 problem

[Pete Wyckoff]

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

[PATCH v2 3/8] git-p4: add missing newline in initial import message

[Pete Wyckoff]

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

[PATCH v2 4/8] git-p4: accommodate new move/delete type in p4

[Pete Wyckoff]

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

Re: [PATCH v2 4/8] git-p4: accommodate new move/delete type in p4

[Junio C Hamano]

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.

[PATCH v2 5/8] git-p4: reinterpret confusing p4 message

[Pete Wyckoff]

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

Re: [PATCH v2 5/8] git-p4: reinterpret confusing p4 message

[Tor Arvid Lund]

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
>
>

[PATCH v2 6/8] git-p4: better message for "git-p4 sync" when not cloned

[Pete Wyckoff]

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

[PATCH v2 7/8] git-p4: decode p4 wildcard characters

[Pete Wyckoff]

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

Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters

[Junio C Hamano]

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.

Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters

[Pete Wyckoff]

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

Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters

[Tor Arvid Lund]

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

Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters

[Junio C Hamano]

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?

Re: [PATCH v2 7/8] git-p4: decode p4 wildcard characters

[Pete Wyckoff]

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

[PATCH v2 8/8] git-p4: support clone --bare

[Pete Wyckoff]

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