Re: [PATCH] git-p4: Fix depot path stripping
Yes, that's where we left off. I owe you better testing of the clientspec-side path remapping case, which I hope to get to soon. Will get back to you as soon as I can. Thanks, Nuno On Fri, Mar 2, 2018 at 2:09 AM, Luke Diamand <l...@diamand.org> wrote: > On 27 February 2018 at 19:00, Nuno Subtil <sub...@gmail.com> wrote: >> I originally thought this had been designed such that the p4 client spec >> determines the paths, which would make sense. However, git-p4 still ignores >> the local path mappings in the client spec when syncing p4 depot paths >> w/--use-client-spec. Effectively, it looks as though --use-client-spec >> always implies --keep-paths, which didn't seem to me like what was intended. >> >> For the use case I'm dealing with (syncing a p4 path but ignoring some >> directories inside it), there seems to be no way today to generate a git >> tree rooted at the p4 path location instead of the root of the depot, which >> looks like a bug to me. I don't really understand the overall design well >> enough to tell whether the bug lies in stripRepoPath or somewhere else, to >> be honest, but that's where I saw the inconsistency manifest itself. > > I replied but managed to drop git-users off the thread. So trying again! > > The behavior is a touch surprising, but I _think_ it's correct. > > With --use-client-spec enabled, paths in the git repot get mapped as > if you had used the file mapping in the client spec, using "p4 where". > > So, for example, if you have a client spec which looks like: >//depot/... //my_client_spec/... > > then you're going to get the full repo structure, even if you only > clone a subdirectory. > > e.g. if you clone //depot/a/b/c then with "--use-client-spec" enabled, > you will get a/b/c in your git repo, and with it not enabled, you will > just get c/. > > If instead your client spec looks like: > //depot/a/b/... //my_client_spec/... > > then you should only get c/d. (And a quick test confirms that). > > I think Nuno's original question comes from wanting to map some files > into place which the clientspec mapping emulation in git-p4 was > possibly not handling - if we can get a test case for that (or an > example) then we can see if it's just that the client mapping code > that Pete put in is insufficient, or if there's some other way around. > Or if indeed I'm wrong, and there's a bug! There may be some weird > corner-cases where it might do the wrong thing. > > Thanks, > Luke > > >> >> Nuno >> >> >> On Tue, Feb 27, 2018 at 3:12 AM, Luke Diamand <l...@diamand.org> wrote: >>> >>> On 27 February 2018 at 08:43, Nuno Subtil <sub...@gmail.com> wrote: >>> > The issue is that passing in --use-client-spec will always cause git-p4 >>> > to >>> > preserve the full depot path in the working tree that it creates, even >>> > when >>> > --keep-path is not used. >>> > >>> > The repro case is fairly simple: cloning a given p4 path that is nested >>> > 2 or >>> > more levels below the depot root will have different results with and >>> > without --use-client-spec (even when the client spec is just a >>> > straightforward map of the entire depot). >>> > >>> > E.g., 'git p4 clone //p4depot/path/to/some/project/...' will create a >>> > working tree rooted at project. However, 'git p4 clone --use-client-spec >>> > //p4depot/path/to/some/project/...' will create a working tree rooted at >>> > the >>> > root of the depot. >>> >>> I think it _might_ be by design. >>> >>> At least, the test ./t9809-git-p4-client-view.sh seems to fail for me >>> with your change, although I haven't investigated what's going on: >>> >>> $ ./t9809-git-p4-client-view.sh >>> ... >>> ... >>> Doing initial import of //depot/dir1/ from revision #head into >>> refs/remotes/p4/master >>> ./t9809-git-p4-client-view.sh: 10: eval: cannot create dir1/file13: >>> Directory nonexistent >>> not ok 23 - subdir clone, submit add >>> >>> I think originally the logic came from this change: >>> >>>21ef5df43 git p4: make branch detection work with --use-client-spec >>> >>> which was fixing what seems like the same problem but with branch >>> detection enabled. >>> >>> >>> > >>> > Thanks, >>> > Nuno >>> > >>> > >>> > On Tue, Feb 27, 2018 at 12:10 AM, Luke Diamand <l...@diamand.org&
[PATCH] git-p4: Fix depot path stripping
When useClientSpec=true, stripping of P4 depot paths doesn't happen
correctly on sync. Modifies stripRepoPath to handle this case better.
Signed-off-by: Nuno Subtil <sub...@gmail.com>
---
git-p4.py | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69738..3df95d0fd1d83 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2480,7 +2480,7 @@ def stripRepoPath(self, path, prefixes):
if path.startswith(b + "/"):
path = path[len(b)+1:]
-elif self.keepRepoPath:
+if self.keepRepoPath:
# Preserve everything in relative path name except leading
# //depot/; just look at first prefix as they all should
# be in the same depot.
@@ -2490,6 +2490,12 @@ def stripRepoPath(self, path, prefixes):
else:
for p in prefixes:
+ if self.useClientSpec and not self.keepRepoPath:
+# when useClientSpec is false, the prefix will contain the
depot name but the path will not
+# extract the depot name and add it to the path so the
match below will do the right thing
+depot = re.sub("^(//[^/]+/).*", r'\1', p)
+path = depot + path
+
if p4PathStartsWith(path, p):
path = path[len(p):]
break
@@ -2526,8 +2532,8 @@ def splitFilesIntoBranches(self, commit):
# go in a p4 client
if self.useClientSpec:
relPath = self.clientSpecDirs.map_in_client(path)
-else:
-relPath = self.stripRepoPath(path, self.depotPaths)
+
+relPath = self.stripRepoPath(path, self.depotPaths)
for branch in self.knownBranches.keys():
# add a trailing slash so that a commit into qt/4.2foo
--
https://github.com/git/git/pull/463
[PATCH] git-p4: add p4 shelf support
Extends the submit command to support shelving a commit instead of
submitting it to p4 (similar to --prepare-p4-only).
Signed-off-by: Nuno Subtil <sub...@gmail.com>
---
git-p4.py | 36 ++--
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index fd5ca52..3c4be22 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1286,6 +1286,8 @@ def __init__(self):
optparse.make_option("--export-labels", dest="exportLabels",
action="store_true"),
optparse.make_option("--dry-run", "-n", dest="dry_run",
action="store_true"),
optparse.make_option("--prepare-p4-only",
dest="prepare_p4_only", action="store_true"),
+optparse.make_option("--shelve-only", dest="shelve_only",
action="store_true", help="Create P4 shelf for first change that would be
submitted (using a new CL)"),
+optparse.make_option("--shelve-cl", dest="shelve_cl",
help="Replace shelf under existing CL number (previously shelved files will be
deleted)"),
optparse.make_option("--conflict", dest="conflict_behavior",
choices=self.conflict_behavior_choices),
optparse.make_option("--branch", dest="branch"),
@@ -1297,6 +1299,8 @@ def __init__(self):
self.preserveUser = gitConfigBool("git-p4.preserveUser")
self.dry_run = False
self.prepare_p4_only = False
+self.shelve_only = False
+self.shelve_cl = None
self.conflict_behavior = None
self.isWindows = (platform.system() == "Windows")
self.exportLabels = False
@@ -1496,6 +1500,12 @@ def prepareSubmitTemplate(self):
else:
inFilesSection = False
else:
+if self.shelve_only and self.shelve_cl:
+if line.startswith("Change:"):
+line = "Change: %s\n" % self.shelve_cl
+if line.startswith("Status:"):
+line = "Status: pending\n"
+
if line.startswith("Files:"):
inFilesSection = True
@@ -1785,7 +1795,11 @@ def applyCommit(self, id):
if self.isWindows:
message = message.replace("\r\n", "\n")
submitTemplate = message[:message.index(separatorLine)]
-p4_write_pipe(['submit', '-i'], submitTemplate)
+
+if self.shelve_only:
+p4_write_pipe(['shelve', '-i', '-r'], submitTemplate)
+else:
+p4_write_pipe(['submit', '-i'], submitTemplate)
if self.preserveUser:
if p4User:
@@ -1799,12 +1813,17 @@ def applyCommit(self, id):
# new file. This leaves it writable, which confuses p4.
for f in pureRenameCopy:
p4_sync(f, "-f")
-submitted = True
+
+if not self.shelve_only:
+submitted = True
finally:
# skip this patch
if not submitted:
-print "Submission cancelled, undoing p4 changes."
+if not self.shelve_only:
+print "Submission cancelled, undoing p4 changes."
+else:
+print "Change shelved, undoing p4 changes."
for f in editedFiles:
p4_revert(f)
for f in filesToAdd:
@@ -2034,9 +2053,13 @@ def run(self, args):
if ok:
applied.append(commit)
else:
-if self.prepare_p4_only and i < last:
-print "Processing only the first commit due to option" \
- " --prepare-p4-only"
+if (self.prepare_p4_only or self.shelve_only) and i < last:
+if self.prepare_p4_only:
+print "Processing only the first commit due to option"
\
+ " --prepare-p4-only"
+else:
+print "Processing only the first commit due to option"
\
+ " --shelve-only"
break
if i < last:
quit = False
@@ -3638,6 +3661,7 @@ def printUsage(commands):
"debug" : P4Debug,
"submit" : P4Submit,
"commit" : P4Submit,
+"shelve" : P4Submit,
"sync" : P4Sync,
"rebase" : P4Rebase,
"clone" : P4Clone,
--
https://github.com/git/git/pull/309
