* [PATCH 0/2] git-p4: fix submit regression with --use-client-spec
@ 2012-02-26 1:06 Pete Wyckoff
2012-02-26 1:06 ` [PATCH 1/2] git-p4: set useClientSpec variable on initial clone Pete Wyckoff
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Pete Wyckoff @ 2012-02-26 1:06 UTC (permalink / raw)
To: git; +Cc: Laurent Charrière
This pair of patches fixes a regression that happened with ecb7cf9
(git-p4: rewrite view handling, 2012-01-02). There are two factors that
affect where files go in the client when submitting: the cilent spec,
and the depot path. When the depot path was not exactly the root of
the client, submit fails.
Fix this by always using the true client root. And also notice that
somebody has to tell the submit path that it should be looking at the
cilent spec. Save useClientSpec in a configuration variable if it
was specified as an option on the command line.
Junio: can you put this on maint to go out with the next 1.9.x?
Pete Wyckoff (2):
git-p4: set useClientSpec variable on initial clone
git-p4: fix submit regression with clientSpec and subdir clone
Documentation/git-p4.txt | 10 ++-
contrib/fast-import/git-p4 | 97 ++++++++++++++++---------
t/t9809-git-p4-client-view.sh | 159 ++++++++++++++++++++++++++++++++++++++---
3 files changed, 219 insertions(+), 47 deletions(-)
--
1.7.9.219.g1d56c.dirty
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] git-p4: set useClientSpec variable on initial clone
2012-02-26 1:06 [PATCH 0/2] git-p4: fix submit regression with --use-client-spec Pete Wyckoff
@ 2012-02-26 1:06 ` Pete Wyckoff
2012-02-26 1:06 ` [PATCH 2/2] git-p4: fix submit regression with clientSpec and subdir clone Pete Wyckoff
2012-02-27 0:18 ` [PATCH 0/2] git-p4: fix submit regression with --use-client-spec Junio C Hamano
2 siblings, 0 replies; 5+ messages in thread
From: Pete Wyckoff @ 2012-02-26 1:06 UTC (permalink / raw)
To: git; +Cc: Laurent Charrière
If --use-client-spec was given, set the matching configuration
variable. This is necessary to ensure that future submits
work properly.
The alternatives of requiring the user to set it, or providing
a command-line option on every submit, are error prone.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
Documentation/git-p4.txt | 10 +++++++---
contrib/fast-import/git-p4 | 11 ++++++++++-
t/t9809-git-p4-client-view.sh | 17 +++++++++++++++++
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 78938b2..ed82790 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -303,9 +303,13 @@ CLIENT SPEC
-----------
The p4 client specification is maintained with the 'p4 client' command
and contains among other fields, a View that specifies how the depot
-is mapped into the client repository. Git-p4 can consult the client
-spec when given the '--use-client-spec' option or useClientSpec
-variable.
+is mapped into the client repository. The 'clone' and 'sync' commands
+can consult the client spec when given the '--use-client-spec' option or
+when the useClientSpec variable is true. After 'git p4 clone', the
+useClientSpec variable is automatically set in the repository
+configuration file. This allows future 'git p4 submit' commands to
+work properly; the submit command looks only at the variable and does
+not have a command-line option.
The full syntax for a p4 view is documented in 'p4 help views'. Git-p4
knows only a subset of the view syntax. It understands multi-line
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 3e1aa27..94f0a12 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1428,6 +1428,7 @@ class P4Sync(Command, P4UserMap):
self.p4BranchesInGit = []
self.cloneExclude = []
self.useClientSpec = False
+ self.useClientSpec_from_options = False
self.clientSpecDirs = None
if gitConfig("git-p4.syncFromOrigin") == "false":
@@ -2136,7 +2137,11 @@ class P4Sync(Command, P4UserMap):
if not gitBranchExists(self.refPrefix + "HEAD") and self.importIntoRemotes and gitBranchExists(self.branch):
system("git symbolic-ref %sHEAD %s" % (self.refPrefix, self.branch))
- if not self.useClientSpec:
+ # accept either the command-line option, or the configuration variable
+ if self.useClientSpec:
+ # will use this after clone to set the variable
+ self.useClientSpec_from_options = True
+ else:
if gitConfig("git-p4.useclientspec", "--bool") == "true":
self.useClientSpec = True
if self.useClientSpec:
@@ -2455,6 +2460,10 @@ class P4Clone(P4Sync):
else:
print "Could not detect main branch. No checkout/master branch created."
+ # auto-set this variable if invoked with --use-client-spec
+ if self.useClientSpec_from_options:
+ system("git config --bool git-p4.useclientspec true")
+
return True
class P4Branches(Command):
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index c9471d5..25e01a4 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -241,6 +241,23 @@ test_expect_success 'quotes on rhs only' '
'
#
+# Submit tests
+#
+
+# clone sets variable
+test_expect_success 'clone --use-client-spec sets useClientSpec' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config --bool git-p4.useClientSpec >actual &&
+ echo true >true &&
+ test_cmp actual true
+ )
+'
+
+#
# Rename directories to test quoting in depot-side mappings
# //depot
# - "dir 1"
--
1.7.9.219.g1d56c.dirty
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] git-p4: fix submit regression with clientSpec and subdir clone
2012-02-26 1:06 [PATCH 0/2] git-p4: fix submit regression with --use-client-spec Pete Wyckoff
2012-02-26 1:06 ` [PATCH 1/2] git-p4: set useClientSpec variable on initial clone Pete Wyckoff
@ 2012-02-26 1:06 ` Pete Wyckoff
2012-02-27 0:18 ` [PATCH 0/2] git-p4: fix submit regression with --use-client-spec Junio C Hamano
2 siblings, 0 replies; 5+ messages in thread
From: Pete Wyckoff @ 2012-02-26 1:06 UTC (permalink / raw)
To: git; +Cc: Laurent Charrière
When the --use-client-spec is given to clone, and the clone
path is a subset of the full tree as specified in the client,
future submits will go to the wrong place.
Factor out getClientSpec() so both clone/sync and submit can
use it. Introduce getClientRoot() that is needed for the client
spec case, and use it instead of p4Where().
Test the five possible submit behaviors (add, modify, rename,
copy, delete).
Reported-by: Laurent Charrière <lcharriere@promptu.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
contrib/fast-import/git-p4 | 86 ++++++++++++++++---------
t/t9809-git-p4-client-view.sh | 142 +++++++++++++++++++++++++++++++++++++---
2 files changed, 185 insertions(+), 43 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 94f0a12..9ccc87b 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -555,6 +555,46 @@ def p4PathStartsWith(path, prefix):
return path.lower().startswith(prefix.lower())
return path.startswith(prefix)
+def getClientSpec():
+ """Look at the p4 client spec, create a View() object that contains
+ all the mappings, and return it."""
+
+ specList = p4CmdList("client -o")
+ if len(specList) != 1:
+ die('Output from "client -o" is %d lines, expecting 1' %
+ len(specList))
+
+ # dictionary of all client parameters
+ entry = specList[0]
+
+ # just the keys that start with "View"
+ view_keys = [ k for k in entry.keys() if k.startswith("View") ]
+
+ # hold this new View
+ view = View()
+
+ # append the lines, in order, to the view
+ for view_num in range(len(view_keys)):
+ k = "View%d" % view_num
+ if k not in view_keys:
+ die("Expected view key %s missing" % k)
+ view.append(entry[k])
+
+ return view
+
+def getClientRoot():
+ """Grab the client directory."""
+
+ output = p4CmdList("client -o")
+ if len(output) != 1:
+ die('Output from "client -o" is %d lines, expecting 1' % len(output))
+
+ entry = output[0]
+ if "Root" not in entry:
+ die('Client has no "Root"')
+
+ return entry["Root"]
+
class Command:
def __init__(self):
self.usage = "usage: %prog [options]"
@@ -1119,11 +1159,20 @@ class P4Submit(Command, P4UserMap):
print "Internal error: cannot locate perforce depot path from existing branches"
sys.exit(128)
- self.clientPath = p4Where(self.depotPath)
+ self.useClientSpec = False
+ if gitConfig("git-p4.useclientspec", "--bool") == "true":
+ self.useClientSpec = True
+ if self.useClientSpec:
+ self.clientSpecDirs = getClientSpec()
+
+ if self.useClientSpec:
+ # all files are relative to the client spec
+ self.clientPath = getClientRoot()
+ else:
+ self.clientPath = p4Where(self.depotPath)
- if len(self.clientPath) == 0:
- print "Error: Cannot locate perforce checkout of %s in client view" % self.depotPath
- sys.exit(128)
+ if self.clientPath == "":
+ die("Error: Cannot locate perforce checkout of %s in client view" % self.depotPath)
print "Perforce checkout for depot path %s located at %s" % (self.depotPath, self.clientPath)
self.oldWorkingDirectory = os.getcwd()
@@ -2078,33 +2127,6 @@ class P4Sync(Command, P4UserMap):
print self.gitError.read()
- def getClientSpec(self):
- specList = p4CmdList("client -o")
- if len(specList) != 1:
- die('Output from "client -o" is %d lines, expecting 1' %
- len(specList))
-
- # dictionary of all client parameters
- entry = specList[0]
-
- # just the keys that start with "View"
- view_keys = [ k for k in entry.keys() if k.startswith("View") ]
-
- # hold this new View
- view = View()
-
- # append the lines, in order, to the view
- for view_num in range(len(view_keys)):
- k = "View%d" % view_num
- if k not in view_keys:
- die("Expected view key %s missing" % k)
- view.append(entry[k])
-
- self.clientSpecDirs = view
- if self.verbose:
- for i, m in enumerate(self.clientSpecDirs.mappings):
- print "clientSpecDirs %d: %s" % (i, str(m))
-
def run(self, args):
self.depotPaths = []
self.changeRange = ""
@@ -2145,7 +2167,7 @@ class P4Sync(Command, P4UserMap):
if gitConfig("git-p4.useclientspec", "--bool") == "true":
self.useClientSpec = True
if self.useClientSpec:
- self.getClientSpec()
+ self.clientSpecDirs = getClientSpec()
# TODO: should always look at previous commits,
# merge with previous imports, if possible.
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 25e01a4..9642641 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -71,20 +71,24 @@ git_verify() {
# - dir2
# - file21
# - file22
+init_depot() {
+ for d in 1 2 ; do
+ mkdir -p dir$d &&
+ for f in 1 2 ; do
+ echo dir$d/file$d$f >dir$d/file$d$f &&
+ p4 add dir$d/file$d$f &&
+ p4 submit -d "dir$d/file$d$f"
+ done
+ done &&
+ find . -type f ! -name files >files &&
+ check_files_exist dir1/file11 dir1/file12 \
+ dir2/file21 dir2/file22
+}
+
test_expect_success 'init depot' '
(
cd "$cli" &&
- for d in 1 2 ; do
- mkdir -p dir$d &&
- for f in 1 2 ; do
- echo dir$d/file$d$f >dir$d/file$d$f &&
- p4 add dir$d/file$d$f &&
- p4 submit -d "dir$d/file$d$f"
- done
- done &&
- find . -type f ! -name files >files &&
- check_files_exist dir1/file11 dir1/file12 \
- dir2/file21 dir2/file22
+ init_depot
)
'
@@ -257,6 +261,122 @@ test_expect_success 'clone --use-client-spec sets useClientSpec' '
)
'
+# clone just a subdir of the client spec
+test_expect_success 'subdir clone' '
+ client_view "//depot/... //client/..." &&
+ files="dir1/file11 dir1/file12 dir2/file21 dir2/file22" &&
+ client_verify $files &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+ git_verify dir1/file11 dir1/file12
+'
+
+#
+# submit back, see what happens: five cases
+#
+test_expect_success 'subdir clone, submit modify' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ echo line >>dir1/file12 &&
+ git add dir1/file12 &&
+ git commit -m dir1/file12 &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ test_path_is_file dir1/file12 &&
+ test_line_count = 2 dir1/file12
+ )
+'
+
+test_expect_success 'subdir clone, submit add' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ echo file13 >dir1/file13 &&
+ git add dir1/file13 &&
+ git commit -m dir1/file13 &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ test_path_is_file dir1/file13
+ )
+'
+
+test_expect_success 'subdir clone, submit delete' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git rm dir1/file12 &&
+ git commit -m "delete dir1/file12" &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ test_path_is_missing dir1/file12
+ )
+'
+
+test_expect_success 'subdir clone, submit copy' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.detectCopies true &&
+ cp dir1/file11 dir1/file11a &&
+ git add dir1/file11a &&
+ git commit -m "copy to dir1/file11a" &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ test_path_is_file dir1/file11a
+ )
+'
+
+test_expect_success 'subdir clone, submit rename' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot/dir1 &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.detectRenames true &&
+ git mv dir1/file13 dir1/file13a &&
+ git commit -m "rename dir1/file13 to dir1/file13a" &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ test_path_is_missing dir1/file13 &&
+ test_path_is_file dir1/file13a
+ )
+'
+
+test_expect_success 'reinit depot' '
+ (
+ cd "$cli" &&
+ p4 sync -f &&
+ rm files &&
+ p4 delete */* &&
+ p4 submit -d "delete all files" &&
+ init_depot
+ )
+'
+
#
# Rename directories to test quoting in depot-side mappings
# //depot
--
1.7.9.219.g1d56c.dirty
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] git-p4: fix submit regression with --use-client-spec
2012-02-26 1:06 [PATCH 0/2] git-p4: fix submit regression with --use-client-spec Pete Wyckoff
2012-02-26 1:06 ` [PATCH 1/2] git-p4: set useClientSpec variable on initial clone Pete Wyckoff
2012-02-26 1:06 ` [PATCH 2/2] git-p4: fix submit regression with clientSpec and subdir clone Pete Wyckoff
@ 2012-02-27 0:18 ` Junio C Hamano
2012-02-27 0:25 ` Pete Wyckoff
2 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2012-02-27 0:18 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git, Laurent Charrière
Pete Wyckoff <pw@padd.com> writes:
> This pair of patches fixes a regression that happened with ecb7cf9
> (git-p4: rewrite view handling, 2012-01-02). There are two factors that
> affect where files go in the client when submitting: the cilent spec,
> and the depot path. When the depot path was not exactly the root of
> the client, submit fails.
>
> Fix this by always using the true client root. And also notice that
> somebody has to tell the submit path that it should be looking at the
> cilent spec. Save useClientSpec in a configuration variable if it
> was specified as an option on the command line.
>
> Junio: can you put this on maint to go out with the next 1.9.x?
Surely and thanks.
Your "p4: submit with wildcards" seems to conflict with this change and
may need to be rebased, though.
>
> Pete Wyckoff (2):
> git-p4: set useClientSpec variable on initial clone
> git-p4: fix submit regression with clientSpec and subdir clone
>
> Documentation/git-p4.txt | 10 ++-
> contrib/fast-import/git-p4 | 97 ++++++++++++++++---------
> t/t9809-git-p4-client-view.sh | 159 ++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 219 insertions(+), 47 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] git-p4: fix submit regression with --use-client-spec
2012-02-27 0:18 ` [PATCH 0/2] git-p4: fix submit regression with --use-client-spec Junio C Hamano
@ 2012-02-27 0:25 ` Pete Wyckoff
0 siblings, 0 replies; 5+ messages in thread
From: Pete Wyckoff @ 2012-02-27 0:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Laurent Charrière
gitster@pobox.com wrote on Sun, 26 Feb 2012 16:18 -0800:
> Pete Wyckoff <pw@padd.com> writes:
>
> > This pair of patches fixes a regression that happened with ecb7cf9
> > (git-p4: rewrite view handling, 2012-01-02). There are two factors that
> > affect where files go in the client when submitting: the cilent spec,
> > and the depot path. When the depot path was not exactly the root of
> > the client, submit fails.
> >
> > Fix this by always using the true client root. And also notice that
> > somebody has to tell the submit path that it should be looking at the
> > cilent spec. Save useClientSpec in a configuration variable if it
> > was specified as an option on the command line.
> >
> > Junio: can you put this on maint to go out with the next 1.9.x?
>
> Surely and thanks.
>
> Your "p4: submit with wildcards" seems to conflict with this change and
> may need to be rebased, though.
Ah, yes. The conflicting chunks can be added in any order above
class P4Command. I'll rebase and resend in a few days, after
waiting for review comments.
-- Pete
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-02-27 0:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-26 1:06 [PATCH 0/2] git-p4: fix submit regression with --use-client-spec Pete Wyckoff
2012-02-26 1:06 ` [PATCH 1/2] git-p4: set useClientSpec variable on initial clone Pete Wyckoff
2012-02-26 1:06 ` [PATCH 2/2] git-p4: fix submit regression with clientSpec and subdir clone Pete Wyckoff
2012-02-27 0:18 ` [PATCH 0/2] git-p4: fix submit regression with --use-client-spec Junio C Hamano
2012-02-27 0:25 ` Pete Wyckoff
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 NNTP newsgroup(s).