[PATCH] Support different branch layouts in git-p4

[PATCH] Support different branch layouts in git-p4

[Ian Wienand]

Hi,

I think the addition to the git-p4.txt in the diff explains the
reasoning behind the patch best.  In short, we have a repository
layout

//depot/foo/branch
//depot/moo/branch

where we require projects 'foo' and 'moo' to be alongside each other.
We can do this with p4 views, but currently have to have 'foo' and
'moo' in separate git repos.

This just munges the incoming paths to either put the branch as the
top level directory, or just remove it entirely if you don't need it.

I've tested it locally, but I don't really have a wide variety of p4
environments to expose it too.

-i

Signed-off-by: Ian Wienand <ianw@vmware.com>
---
 contrib/fast-import/git-p4     |   35 +++++++++++++++++++++++-
 contrib/fast-import/git-p4.txt |   58 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 04ce7e3..4bd40f8 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -848,6 +848,10 @@ class P4Sync(Command):
                 optparse.make_option("--max-changes", dest="maxChanges"),
                 optparse.make_option("--keep-path", dest="keepRepoPath", action='store_true',
                                      help="Keep entire BRANCH/DIR/SUBDIR prefix during import"),
+                optparse.make_option("--branch-path", dest="branchPath", type='choice',
+                                     choices=('none', 'first'),
+                                     default=None,
+                                     help="Remove the branch dir (none) or move it above project dir (first)"),
                 optparse.make_option("--use-client-spec", dest="useClientSpec", action='store_true',
                                      help="Only sync files that are included in the Perforce Client Spec")
         ]
@@ -917,6 +921,20 @@ class P4Sync(Command):
             if path.startswith(p):
                 path = path[len(p):]
 
+        # reorg to move/remove branch from the output filename -- kind
+        # of like how you can set your view in your p4 client
+        if self.keepRepoPath and self.branchPath == 'first':
+            # move the second element first, so what was was
+            # "//depot/proj/branch/file" becomes "branch/proj/file".
+            path = re.sub("^([^/]+/)([^/]+/)", r'\2\1', path)
+        elif self.keepRepoPath and self.branchPath == 'none':
+            # remove the second element, so what was
+            # "//depot/proj/branch/file" becomes "proj/file"
+            path = re.sub("^([^/]+/)([^/]+/)", r'\2', path)
+        elif self.branchPath:
+            sys.stderr.write("branchPath without keepRepoPath?")
+            sys.exit(1)
+
         return path
 
     def splitFilesIntoBranches(self, commit):
@@ -940,7 +958,6 @@ class P4Sync(Command):
             relPath = self.stripRepoPath(path, self.depotPaths)
 
             for branch in self.knownBranches.keys():
-
                 # add a trailing slash so that a commit into qt/4.2foo doesn't end up in qt/4.2
                 if relPath.startswith(branch + "/"):
                     if branch not in branches:
@@ -1283,12 +1300,24 @@ class P4Sync(Command):
         if self.keepRepoPath:
             option_keys['keepRepoPath'] = 1
 
+        # since we're just saving the dict keys, append the branchPath
+        # option to the key
+        if self.branchPath:
+            option_keys['branchPath_%s' % self.branchPath] = 1
+
         d["options"] = ' '.join(sorted(option_keys.keys()))
 
     def readOptions(self, d):
         self.keepRepoPath = (d.has_key('options')
                              and ('keepRepoPath' in d['options']))
 
+        # restore the branchpath option; is one of "none" and "first"
+        if (d.has_key('options')):
+            if ('branchPath_none' in d['options']):
+                self.branchPath = 'none'
+            elif ('branchPath_first' in d['options']):
+                self.branchPath = 'first'
+
     def gitRefForBranch(self, branch):
         if branch == "main":
             return self.refPrefix + "master"
@@ -1775,6 +1804,10 @@ class P4Clone(P4Sync):
             sys.stderr.write("Must specify destination for --keep-path\n")
             sys.exit(1)
 
+        if self.branchPath and not self.keepRepoPath:
+            sys.stderr.write("Must specify --keep-path for --branch-path\n")
+            sys.exit(1)
+
         depotPaths = args
 
         if not self.cloneDestination and len(depotPaths) > 1:
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index 49b3359..669c63c 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -191,6 +191,64 @@ git-p4.useclientspec
 
   git config [--global] git-p4.useclientspec false
 
+Dealing with different repository layouts
+=========================================
+
+Perforce clients can map views of projects and branches in different
+ways which your build system may rely on.  Say your code is organised
+as two projects "foo" and "moo" which have a common branch
+
+//depot/foo/branch/...
+//depot/moo/branch/...
+
+and you require both "foo" and "moo" projects in your git repository,
+there are several options.
+
+Firstly, you could simply clone each project as a completely separate
+git tree.  However, if the two projects are dependent on each other
+this can be annoying for both sync -- you must remember to sync both
+"foo" and "moo" to keep everything consistent -- and submit -- a
+change that should logically be a single changeset across "foo" and
+"moo" will have to be broken up (breaking bisection too).
+
+Another option is to simply specify multiple depots
+
+ git p4 sync //depot/foo/branch //depot/moo/branch
+
+which will import "foo" and "moo" into the same directory.
+
+To keep the projects separate, the --keep-path option used as
+
+ git p4 sync --keep-path --destination /tmp/boo/ //depot/foo/branch //depot/moo/branch
+
+will create a layout of
+
+ /tmp/boo/foo/branch/...
+ /tmp/boo/moo/branch/...
+
+However, some build systems may rely on p4's ability to specify
+destinations for views in your client.  The --branch-path flag, which
+requires the --keep-path flag, allows two additional layout options.
+
+ git p4 sync --keep-path --destination /tmp/boo --branch-path=none //depot/foo/branch //depot/moo/branch
+
+will remove the branch name entirely, leaving you with a directory
+that looks like
+
+ /tmp/boo/foo/...
+ /tmp/boo/moo/...
+
+and
+
+ git p4 sync --keep-path --destination /tmp/boo --branch-path=first //depot/foo/branch //depot/moo/branch
+
+will give you each of the projects under a directory named for their
+common branch
+
+ /tmp/boo/branch/foo/...
+ /tmp/boo/branch/moo/...
+
+
 Implementation Details...
 =========================
 
-- 
1.7.2.3

Re: [PATCH] Support different branch layouts in git-p4

[Tor Arvid Lund]

On Tue, Feb 1, 2011 at 11:59 PM, Ian Wienand <ianw@vmware.com> wrote:
> Hi,
>
> I think the addition to the git-p4.txt in the diff explains the
> reasoning behind the patch best.  In short, we have a repository
> layout
>
> //depot/foo/branch
> //depot/moo/branch
>
> where we require projects 'foo' and 'moo' to be alongside each other.
> We can do this with p4 views, but currently have to have 'foo' and
> 'moo' in separate git repos.
>
> This just munges the incoming paths to either put the branch as the
> top level directory, or just remove it entirely if you don't need it.

Hi, Ian! We haven't met, but thank you for the patch, and for trying
to help make git better.

Now, I have to say that I don't particularly like it, and here's why I
will vote against this patch:

For starters, I don't think that I like git-p4 being taught to solve
problems that seem to be caused by a poor/unfortunate perforce layout.
Especially since *your* type of poor perforce layout will probably be
poor in a very different way from the next guy with a poor layout :)
For instance, you have hard-coded that you replace the first and
second directory name... It is very easy to imagine people having
deeper trees than that...

But then I started thinking about it a bit more... It was me who added
the --use-client-spec option back in the day. The support for that
stuff really should be better than what I made at the time. The
client-spec format contains lines like

//depot/...   //local-root/...
-//depot/dontcare/...   //local-root/dontcare/...
//depot/foo/branch/...   //local-root/branch/foo/...
//depot/moo/branch/...   //local-root/branch/moo/...

Please observe that the two last lines look like what I think *your*
client-spec should look like. This would map the foo/branch and
moo/branch in the perforce depot to branch/foo and branch/moo on the
client side.

Of course, today this will not work with git-p4 clone. The
--use-client-spec option, as I implemented it, simply filters out all
that stuff that matches the pattern lines that starts with "-". So the
names of all files will match the patterns on the left-hand side in
the client-spec. A solution which I think would work well for
everyone, is if files would be placed according to the right-hand
patterns in the client-spec.

That should be a much more elegant and generic solution. Whatcha
think? If you want to take a whack at hacking that into place, I will
help guide the way if needed (if others are not opposed to such an
idea) :)

    -- Tor Arvid

> I've tested it locally, but I don't really have a wide variety of p4
> environments to expose it too.
>
> -i
>
> Signed-off-by: Ian Wienand <ianw@vmware.com>
> ---
>  contrib/fast-import/git-p4     |   35 +++++++++++++++++++++++-
>  contrib/fast-import/git-p4.txt |   58 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 92 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 04ce7e3..4bd40f8 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -848,6 +848,10 @@ class P4Sync(Command):
>                 optparse.make_option("--max-changes", dest="maxChanges"),
>                 optparse.make_option("--keep-path", dest="keepRepoPath", action='store_true',
>                                      help="Keep entire BRANCH/DIR/SUBDIR prefix during import"),
> +                optparse.make_option("--branch-path", dest="branchPath", type='choice',
> +                                     choices=('none', 'first'),
> +                                     default=None,
> +                                     help="Remove the branch dir (none) or move it above project dir (first)"),
>                 optparse.make_option("--use-client-spec", dest="useClientSpec", action='store_true',
>                                      help="Only sync files that are included in the Perforce Client Spec")
>         ]
> @@ -917,6 +921,20 @@ class P4Sync(Command):
>             if path.startswith(p):
>                 path = path[len(p):]
>
> +        # reorg to move/remove branch from the output filename -- kind
> +        # of like how you can set your view in your p4 client
> +        if self.keepRepoPath and self.branchPath == 'first':
> +            # move the second element first, so what was was
> +            # "//depot/proj/branch/file" becomes "branch/proj/file".
> +            path = re.sub("^([^/]+/)([^/]+/)", r'\2\1', path)
> +        elif self.keepRepoPath and self.branchPath == 'none':
> +            # remove the second element, so what was
> +            # "//depot/proj/branch/file" becomes "proj/file"
> +            path = re.sub("^([^/]+/)([^/]+/)", r'\2', path)
> +        elif self.branchPath:
> +            sys.stderr.write("branchPath without keepRepoPath?")
> +            sys.exit(1)
> +
>         return path
>
>     def splitFilesIntoBranches(self, commit):
> @@ -940,7 +958,6 @@ class P4Sync(Command):
>             relPath = self.stripRepoPath(path, self.depotPaths)
>
>             for branch in self.knownBranches.keys():
> -
>                 # add a trailing slash so that a commit into qt/4.2foo doesn't end up in qt/4.2
>                 if relPath.startswith(branch + "/"):
>                     if branch not in branches:
> @@ -1283,12 +1300,24 @@ class P4Sync(Command):
>         if self.keepRepoPath:
>             option_keys['keepRepoPath'] = 1
>
> +        # since we're just saving the dict keys, append the branchPath
> +        # option to the key
> +        if self.branchPath:
> +            option_keys['branchPath_%s' % self.branchPath] = 1
> +
>         d["options"] = ' '.join(sorted(option_keys.keys()))
>
>     def readOptions(self, d):
>         self.keepRepoPath = (d.has_key('options')
>                              and ('keepRepoPath' in d['options']))
>
> +        # restore the branchpath option; is one of "none" and "first"
> +        if (d.has_key('options')):
> +            if ('branchPath_none' in d['options']):
> +                self.branchPath = 'none'
> +            elif ('branchPath_first' in d['options']):
> +                self.branchPath = 'first'
> +
>     def gitRefForBranch(self, branch):
>         if branch == "main":
>             return self.refPrefix + "master"
> @@ -1775,6 +1804,10 @@ class P4Clone(P4Sync):
>             sys.stderr.write("Must specify destination for --keep-path\n")
>             sys.exit(1)
>
> +        if self.branchPath and not self.keepRepoPath:
> +            sys.stderr.write("Must specify --keep-path for --branch-path\n")
> +            sys.exit(1)
> +
>         depotPaths = args
>
>         if not self.cloneDestination and len(depotPaths) > 1:
> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
> index 49b3359..669c63c 100644
> --- a/contrib/fast-import/git-p4.txt
> +++ b/contrib/fast-import/git-p4.txt
> @@ -191,6 +191,64 @@ git-p4.useclientspec
>
>   git config [--global] git-p4.useclientspec false
>
> +Dealing with different repository layouts
> +=========================================
> +
> +Perforce clients can map views of projects and branches in different
> +ways which your build system may rely on.  Say your code is organised
> +as two projects "foo" and "moo" which have a common branch
> +
> +//depot/foo/branch/...
> +//depot/moo/branch/...
> +
> +and you require both "foo" and "moo" projects in your git repository,
> +there are several options.
> +
> +Firstly, you could simply clone each project as a completely separate
> +git tree.  However, if the two projects are dependent on each other
> +this can be annoying for both sync -- you must remember to sync both
> +"foo" and "moo" to keep everything consistent -- and submit -- a
> +change that should logically be a single changeset across "foo" and
> +"moo" will have to be broken up (breaking bisection too).
> +
> +Another option is to simply specify multiple depots
> +
> + git p4 sync //depot/foo/branch //depot/moo/branch
> +
> +which will import "foo" and "moo" into the same directory.
> +
> +To keep the projects separate, the --keep-path option used as
> +
> + git p4 sync --keep-path --destination /tmp/boo/ //depot/foo/branch //depot/moo/branch
> +
> +will create a layout of
> +
> + /tmp/boo/foo/branch/...
> + /tmp/boo/moo/branch/...
> +
> +However, some build systems may rely on p4's ability to specify
> +destinations for views in your client.  The --branch-path flag, which
> +requires the --keep-path flag, allows two additional layout options.
> +
> + git p4 sync --keep-path --destination /tmp/boo --branch-path=none //depot/foo/branch //depot/moo/branch
> +
> +will remove the branch name entirely, leaving you with a directory
> +that looks like
> +
> + /tmp/boo/foo/...
> + /tmp/boo/moo/...
> +
> +and
> +
> + git p4 sync --keep-path --destination /tmp/boo --branch-path=first //depot/foo/branch //depot/moo/branch
> +
> +will give you each of the projects under a directory named for their
> +common branch
> +
> + /tmp/boo/branch/foo/...
> + /tmp/boo/branch/moo/...
> +
> +
>  Implementation Details...
>  =========================
>
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] Support different branch layouts in git-p4

[Ian Wienand]

Thanks for taking a look

On 04/02/11 16:37, Tor Arvid Lund wrote:
> For starters, I don't think that I like git-p4 being taught to solve
> problems that seem to be caused by a poor/unfortunate perforce layout.

I do think this //depot/project/branch type layout is pretty typical,
although I admit I don't have a lot of experience with alternative p4
setups.

> A solution which I think would work well for everyone, is if files
> would be placed according to the right-hand patterns in the
> client-spec.

I did consider this at first.  My only issue is that it is a bit
confusing to use the client spec for filtering (and in this case
re-writing), but not for actually selecting the depots to clone, which
I still need to replicate on the command line.  However that is a much
larger change.

What do you think of this one?

In this case, my client view is

//depot/project/branch/...  //client/branch/project/...
//depot/project2/branch/...  //client/branch/project2/...

and my git directory layout ends up as

branch/project/...
branch/project2/...

-i

---

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 04ce7e3..eb9620c 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -878,6 +878,7 @@ class P4Sync(Command):
         self.cloneExclude = []
         self.useClientSpec = False
         self.clientSpecDirs = []
+        self.clientName = None
 
         if gitConfig("git-p4.syncFromOrigin") == "false":
             self.syncWithOrigin = False
@@ -910,6 +911,22 @@ class P4Sync(Command):
         return files
 
     def stripRepoPath(self, path, prefixes):
+        if self.useClientSpec:
+
+            # if using the client spec, we use the output directory
+            # specified in the client.  For example, a view
+            #   //depot/foo/branch/... //client/branch/foo/...
+            # will end up putting all foo/branch files into
+            #  branch/foo/
+            for val in self.clientSpecDirs:
+                if path.startswith(val[0]):
+                    # replace the depot path with the client path
+                    path = path.replace(val[0], val[1][1])
+                    # now strip out the client (//client/...)
+                    path = re.sub("^(//[^/]+/)", '', path)
+                    # the rest is all path
+                    return path
+
         if self.keepRepoPath:
             prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
 
@@ -1032,7 +1049,7 @@ class P4Sync(Command):
             includeFile = True
             for val in self.clientSpecDirs:
                 if f['path'].startswith(val[0]):
-                    if val[1] <= 0:
+                    if val[1][0] <= 0:
                         includeFile = False
                     break
 
@@ -1474,20 +1491,36 @@ class P4Sync(Command):
         temp = {}
         for entry in specList:
             for k,v in entry.iteritems():
+                if k.startswith("Client"):
+                    self.clientName = v
+            
                 if k.startswith("View"):
                     if v.startswith('"'):
                         start = 1
                     else:
                         start = 0
                     index = v.find("...")
+
+                    # save the "client view"; i.e the RHS of the view
+                    # line that tells the client where to put the
+                    # files for this view.
+                    cv = v[index+4:] # +4 to remove previous '... '
+                    cv_index = cv.find("...")
+                    cv=cv[:cv_index]
+
+                    # now save the view; +index means included, -index
+                    # means it should be filtered out.
                     v = v[start:index]
                     if v.startswith("-"):
                         v = v[1:]
-                        temp[v] = -len(v)
+                        include = -len(v)
                     else:
-                        temp[v] = len(v)
+                        include = len(v)
+
+                    temp[v] = (include, cv)
+
         self.clientSpecDirs = temp.items()
-        self.clientSpecDirs.sort( lambda x, y: abs( y[1] ) - abs( x[1] ) )
+        self.clientSpecDirs.sort( lambda x, y: abs( y[1][0] ) - abs( x[1][0] ) )
 
     def run(self, args):
         self.depotPaths = []
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index 49b3359..e09da44 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -191,6 +191,11 @@ git-p4.useclientspec
 
   git config [--global] git-p4.useclientspec false
 
+The P4CLIENT environment variable should be correctly set for p4 to be
+able to find the relevant client.  This client spec will be used to
+both filter the files cloned by git and set the directory layout as
+specified in the client (this implies --keep-path style semantics).
+
 Implementation Details...
 =========================
 

Re: [PATCH] Support different branch layouts in git-p4

[Tor Arvid Lund]

On Mon, Feb 7, 2011 at 1:05 AM, Ian Wienand <ianw@vmware.com> wrote:
> On 04/02/11 16:37, Tor Arvid Lund wrote:
>> For starters, I don't think that I like git-p4 being taught to solve
>> problems that seem to be caused by a poor/unfortunate perforce layout.
>
> I do think this //depot/project/branch type layout is pretty typical,
> although I admit I don't have a lot of experience with alternative p4
> setups.

You may be right, although I suspect that
//depot/department/project/branch may be equally typical. At my
$dayjob, we have gone through several "reorganizations" of the
perforce layouts. I'm the guy that never likes any of them ;)

>> A solution which I think would work well for everyone, is if files
>> would be placed according to the right-hand patterns in the
>> client-spec.
>
> I did consider this at first.  My only issue is that it is a bit
> confusing to use the client spec for filtering (and in this case
> re-writing), but not for actually selecting the depots to clone, which
> I still need to replicate on the command line.  However that is a much
> larger change.
>
> What do you think of this one?

In general, me thinks me likes it :-)

(and it turned out much smaller than I would have originally guessed)

I should probably mention that I haven't tested your patch at all. I
will have a pretty rough week at work, so it would be great if anyone
else feels like chiming in on this one... But I have some quick
observations below:

> ---
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 04ce7e3..eb9620c 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -878,6 +878,7 @@ class P4Sync(Command):
>         self.cloneExclude = []
>         self.useClientSpec = False
>         self.clientSpecDirs = []
> +        self.clientName = None
>
>         if gitConfig("git-p4.syncFromOrigin") == "false":
>             self.syncWithOrigin = False
> @@ -910,6 +911,22 @@ class P4Sync(Command):
>         return files
>
>     def stripRepoPath(self, path, prefixes):
> +        if self.useClientSpec:
> +
> +            # if using the client spec, we use the output directory
> +            # specified in the client.  For example, a view
> +            #   //depot/foo/branch/... //client/branch/foo/...
> +            # will end up putting all foo/branch files into
> +            #  branch/foo/
> +            for val in self.clientSpecDirs:
> +                if path.startswith(val[0]):
> +                    # replace the depot path with the client path
> +                    path = path.replace(val[0], val[1][1])
> +                    # now strip out the client (//client/...)
> +                    path = re.sub("^(//[^/]+/)", '', path)
> +                    # the rest is all path
> +                    return path
> +
>         if self.keepRepoPath:
>             prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
>
> @@ -1032,7 +1049,7 @@ class P4Sync(Command):
>             includeFile = True
>             for val in self.clientSpecDirs:
>                 if f['path'].startswith(val[0]):
> -                    if val[1] <= 0:
> +                    if val[1][0] <= 0:
>                         includeFile = False
>                     break
>
> @@ -1474,20 +1491,36 @@ class P4Sync(Command):
>         temp = {}
>         for entry in specList:
>             for k,v in entry.iteritems():
> +                if k.startswith("Client"):
> +                    self.clientName = v
> +
>                 if k.startswith("View"):
>                     if v.startswith('"'):
>                         start = 1
>                     else:
>                         start = 0
>                     index = v.find("...")
> +
> +                    # save the "client view"; i.e the RHS of the view
> +                    # line that tells the client where to put the
> +                    # files for this view.
> +                    cv = v[index+4:] # +4 to remove previous '... '

This feels less robust than what we might want. Isn't the format of a
client-spec line either:

-?//depot/path[/...]\s+//client/path[/...]\n

or

-?"//depot/path with spaces/path[/...]"\s+"//client/path with spaces/path[/...]"

.. where -? means an optional '-' char, and \s+ is
'whatever-length-and-kind-of-whitespace'. I'm just guessing from
memory regarding these patterns, but assuming that the section
separator is exactly the string '... ' seems risky, no? :)

> +                    cv_index = cv.find("...")
> +                    cv=cv[:cv_index]

What if a line doesn't end with "..." ? Maybe add an "if cv_index >= 0"

> +
> +                    # now save the view; +index means included, -index
> +                    # means it should be filtered out.
>                     v = v[start:index]
>                     if v.startswith("-"):
>                         v = v[1:]
> -                        temp[v] = -len(v)
> +                        include = -len(v)
>                     else:
> -                        temp[v] = len(v)
> +                        include = len(v)
> +
> +                    temp[v] = (include, cv)
> +
>         self.clientSpecDirs = temp.items()
> -        self.clientSpecDirs.sort( lambda x, y: abs( y[1] ) - abs( x[1] ) )
> +        self.clientSpecDirs.sort( lambda x, y: abs( y[1][0] ) - abs( x[1][0] ) )
>
>     def run(self, args):
>         self.depotPaths = []
> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
> index 49b3359..e09da44 100644
> --- a/contrib/fast-import/git-p4.txt
> +++ b/contrib/fast-import/git-p4.txt
> @@ -191,6 +191,11 @@ git-p4.useclientspec
>
>   git config [--global] git-p4.useclientspec false
>
> +The P4CLIENT environment variable should be correctly set for p4 to be
> +able to find the relevant client.  This client spec will be used to
> +both filter the files cloned by git and set the directory layout as
> +specified in the client (this implies --keep-path style semantics).
> +
>  Implementation Details...
>  =========================
>
>

Aight. I need some sleep now. Nice work so far, Ian! :)

    -- Tor Arvid

Re: [PATCH] Support different branch layouts in git-p4

[Pete Wyckoff]

ianw@vmware.com wrote on Sun, 06 Feb 2011 16:05 -0800:
> I did consider this at first.  My only issue is that it is a bit
> confusing to use the client spec for filtering (and in this case
> re-writing), but not for actually selecting the depots to clone, which
> I still need to replicate on the command line.  However that is a much
> larger change.
> 
> What do you think of this one?
> 
> In this case, my client view is
> 
> //depot/project/branch/...  //client/branch/project/...
> //depot/project2/branch/...  //client/branch/project2/...
> 
> and my git directory layout ends up as
> 
> branch/project/...
> branch/project2/...

We had such terrible p4 mappings too, before the last
rearrangement put us into a single-line view spec.  I think
it would help others to include such support, though.

Back then, I hacked together similar code to deal with the
annoyance.  My code was not pretty and not complete, either.

If you look at "p4 help views", they have lots of oddities
that in theory should be accounted for here.  It doesn't
even mention the thing about quotes, but obviously that is
supported.  Wildcards ... and * can appear multiple
times.  And %%[1-9] can be used to reorder the path.  Also the
order of lines matters, and "+" can be used to merge entries.
Whew.

In practice, I think you get most everything we care about.  A
few comments below, beyond the bits that Tor Arvid caught.

		-- Pete

> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 04ce7e3..eb9620c 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -878,6 +878,7 @@ class P4Sync(Command):
>          self.cloneExclude = []
>          self.useClientSpec = False
>          self.clientSpecDirs = []
> +        self.clientName = None

Unused.

>          if gitConfig("git-p4.syncFromOrigin") == "false":
>              self.syncWithOrigin = False
> @@ -910,6 +911,22 @@ class P4Sync(Command):
>          return files
>  
>      def stripRepoPath(self, path, prefixes):
> +        if self.useClientSpec:
> +
> +            # if using the client spec, we use the output directory
> +            # specified in the client.  For example, a view
> +            #   //depot/foo/branch/... //client/branch/foo/...
> +            # will end up putting all foo/branch files into
> +            #  branch/foo/
> +            for val in self.clientSpecDirs:
> +                if path.startswith(val[0]):
> +                    # replace the depot path with the client path
> +                    path = path.replace(val[0], val[1][1])
> +                    # now strip out the client (//client/...)
> +                    path = re.sub("^(//[^/]+/)", '', path)
> +                    # the rest is all path
> +                    return path

That's clever.  Better than having to remember Client: and build
//client/ out of it.  You could do this down in getClientSpec()
so that val[1] starts with the git-relative path.

>          if self.keepRepoPath:
>              prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
>  
> @@ -1032,7 +1049,7 @@ class P4Sync(Command):
>              includeFile = True
>              for val in self.clientSpecDirs:
>                  if f['path'].startswith(val[0]):
> -                    if val[1] <= 0:
> +                    if val[1][0] <= 0:
>                          includeFile = False
>                      break
>  
> @@ -1474,20 +1491,36 @@ class P4Sync(Command):
>          temp = {}
>          for entry in specList:
>              for k,v in entry.iteritems():
> +                if k.startswith("Client"):
> +                    self.clientName = v

Oh maybe here is where you thought you would need client, but
don't.

>                  if k.startswith("View"):
>                      if v.startswith('"'):
>                          start = 1
>                      else:
>                          start = 0
>                      index = v.find("...")
> +
> +                    # save the "client view"; i.e the RHS of the view
> +                    # line that tells the client where to put the
> +                    # files for this view.
> +                    cv = v[index+4:] # +4 to remove previous '... '
> +                    cv_index = cv.find("...")
> +                    cv=cv[:cv_index]
> +
> +                    # now save the view; +index means included, -index
> +                    # means it should be filtered out.
>                      v = v[start:index]
>                      if v.startswith("-"):
>                          v = v[1:]
> -                        temp[v] = -len(v)
> +                        include = -len(v)
>                      else:
> -                        temp[v] = len(v)
> +                        include = len(v)
> +
> +                    temp[v] = (include, cv)
> +
>          self.clientSpecDirs = temp.items()
> -        self.clientSpecDirs.sort( lambda x, y: abs( y[1] ) - abs( x[1] ) )
> +        self.clientSpecDirs.sort( lambda x, y: abs( y[1][0] ) - abs( x[1][0] ) )

Re: [PATCH] Support different branch layouts in git-p4

[Ian Wienand]

Thanks for the review

On 07/02/11 15:27, Tor Arvid Lund wrote:
> I'm just guessing from memory regarding these patterns, but assuming
> that the section separator is exactly the string '... ' seems risky,
> no? :)

Good point.  If we go past the end of the depot ...'s, then the rest
of the line strip()ed should just be the client I guess.
 
> What if a line doesn't end with "..." ? Maybe add an "if cv_index>= 0"

I think it would mess us up.  I put in a failure case for this.

On 07/02/11 17:22, Pete Wyckoff wrote:
> If you look at "p4 help views", they have lots of oddities
> that in theory should be accounted for here.  It doesn't
> even mention the thing about quotes, but obviously that is
> supported.  Wildcards ... and * can appear multiple
> times.  And %%[1-9] can be used to reorder the path.  Also the
> order of lines matters, and "+" can be used to merge entries.
> Whew.

Those %%'s would also mess us up, I put in an escape hatch for that
too.

I'm pretty sure this covers the majority of cases; if people have
really weird clients I guess they're going to have to do some more
work to get git-p4 to recognise it properly.  Personally, I struggle
to see why it is a feature to have every user re-organsing their views
of the tree -- it seems to move a lot of uncaptured state to the
client side.  anyway...

So here's another version; I agree some testing would be good as I've
only run it locally on //depot/proj/branch clients

-i

---

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 04ce7e3..3304f36 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -910,6 +910,22 @@ class P4Sync(Command):
         return files
 
     def stripRepoPath(self, path, prefixes):
+        if self.useClientSpec:
+
+            # if using the client spec, we use the output directory
+            # specified in the client.  For example, a view
+            #   //depot/foo/branch/... //client/branch/foo/...
+            # will end up putting all foo/branch files into
+            #  branch/foo/
+            for val in self.clientSpecDirs:
+                if path.startswith(val[0]):
+                    # replace the depot path with the client path
+                    path = path.replace(val[0], val[1][1])
+                    # now strip out the client (//client/...)
+                    path = re.sub("^(//[^/]+/)", '', path)
+                    # the rest is all path
+                    return path
+
         if self.keepRepoPath:
             prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
 
@@ -1032,7 +1048,7 @@ class P4Sync(Command):
             includeFile = True
             for val in self.clientSpecDirs:
                 if f['path'].startswith(val[0]):
-                    if val[1] <= 0:
+                    if val[1][0] <= 0:
                         includeFile = False
                     break
 
@@ -1475,19 +1491,45 @@ class P4Sync(Command):
         for entry in specList:
             for k,v in entry.iteritems():
                 if k.startswith("View"):
+
+                    # p4 has these %%1 to %%9 arguments in specs to
+                    # reorder paths; which we can't handle (yet :)
+                    if re.match('\%\%d', v) != None:
+                        print "Sorry, can't handle %%n arguments in client specs"
+                        sys.exit(1)
+
                     if v.startswith('"'):
                         start = 1
                     else:
                         start = 0
                     index = v.find("...")
+
+                    # save the "client view"; i.e the RHS of the view
+                    # line that tells the client where to put the
+                    # files for this view.
+                    cv = v[index+3:].strip() # +3 to remove previous '...'
+
+                    # if the client view doesn't end with a
+                    # ... wildcard, then we're going to mess up the
+                    # output directory, so fail gracefully.
+                    if not cv.endswith('...'):
+                        print 'Sorry, client view in "%s" needs to end with wildcard' % (k)
+                        sys.exit(1)
+                    cv=cv[:-3]
+
+                    # now save the view; +index means included, -index
+                    # means it should be filtered out.
                     v = v[start:index]
                     if v.startswith("-"):
                         v = v[1:]
-                        temp[v] = -len(v)
+                        include = -len(v)
                     else:
-                        temp[v] = len(v)
+                        include = len(v)
+
+                    temp[v] = (include, cv)
+
         self.clientSpecDirs = temp.items()
-        self.clientSpecDirs.sort( lambda x, y: abs( y[1] ) - abs( x[1] ) )
+        self.clientSpecDirs.sort( lambda x, y: abs( y[1][0] ) - abs( x[1][0] ) )
 
     def run(self, args):
         self.depotPaths = []
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index 49b3359..e09da44 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -191,6 +191,11 @@ git-p4.useclientspec
 
   git config [--global] git-p4.useclientspec false
 
+The P4CLIENT environment variable should be correctly set for p4 to be
+able to find the relevant client.  This client spec will be used to
+both filter the files cloned by git and set the directory layout as
+specified in the client (this implies --keep-path style semantics).
+
 Implementation Details...
 =========================
 

Re: [PATCH] Support different branch layouts in git-p4

[Pete Wyckoff]

ianw@vmware.com wrote on Tue, 08 Feb 2011 19:46 -0800:
> So here's another version; I agree some testing would be good as I've
> only run it locally on //depot/proj/branch clients

This is good.  Thanks for fixing it up.  One last pedantic whine
from me.  Fix the regex for the error case:

    arf$ python
    >>> import re
    >>> re.match('\%\%d', "%%3")
    >>> re.match(r'%%\d', "%%3")
    >>> <_sre.SRE_Match object at 0x1ec4168>


		-- Pete

> ---
> 
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 04ce7e3..3304f36 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -910,6 +910,22 @@ class P4Sync(Command):
>          return files
>  
>      def stripRepoPath(self, path, prefixes):
> +        if self.useClientSpec:
> +
> +            # if using the client spec, we use the output directory
> +            # specified in the client.  For example, a view
> +            #   //depot/foo/branch/... //client/branch/foo/...
> +            # will end up putting all foo/branch files into
> +            #  branch/foo/
> +            for val in self.clientSpecDirs:
> +                if path.startswith(val[0]):
> +                    # replace the depot path with the client path
> +                    path = path.replace(val[0], val[1][1])
> +                    # now strip out the client (//client/...)
> +                    path = re.sub("^(//[^/]+/)", '', path)
> +                    # the rest is all path
> +                    return path
> +
>          if self.keepRepoPath:
>              prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])]
>  
> @@ -1032,7 +1048,7 @@ class P4Sync(Command):
>              includeFile = True
>              for val in self.clientSpecDirs:
>                  if f['path'].startswith(val[0]):
> -                    if val[1] <= 0:
> +                    if val[1][0] <= 0:
>                          includeFile = False
>                      break
>  
> @@ -1475,19 +1491,45 @@ class P4Sync(Command):
>          for entry in specList:
>              for k,v in entry.iteritems():
>                  if k.startswith("View"):
> +
> +                    # p4 has these %%1 to %%9 arguments in specs to
> +                    # reorder paths; which we can't handle (yet :)
> +                    if re.match('\%\%d', v) != None:
> +                        print "Sorry, can't handle %%n arguments in client specs"
> +                        sys.exit(1)
> +
>                      if v.startswith('"'):
>                          start = 1
>                      else:
>                          start = 0
>                      index = v.find("...")
> +
> +                    # save the "client view"; i.e the RHS of the view
> +                    # line that tells the client where to put the
> +                    # files for this view.
> +                    cv = v[index+3:].strip() # +3 to remove previous '...'
> +
> +                    # if the client view doesn't end with a
> +                    # ... wildcard, then we're going to mess up the
> +                    # output directory, so fail gracefully.
> +                    if not cv.endswith('...'):
> +                        print 'Sorry, client view in "%s" needs to end with wildcard' % (k)
> +                        sys.exit(1)
> +                    cv=cv[:-3]
> +
> +                    # now save the view; +index means included, -index
> +                    # means it should be filtered out.
>                      v = v[start:index]
>                      if v.startswith("-"):
>                          v = v[1:]
> -                        temp[v] = -len(v)
> +                        include = -len(v)
>                      else:
> -                        temp[v] = len(v)
> +                        include = len(v)
> +
> +                    temp[v] = (include, cv)
> +
>          self.clientSpecDirs = temp.items()
> -        self.clientSpecDirs.sort( lambda x, y: abs( y[1] ) - abs( x[1] ) )
> +        self.clientSpecDirs.sort( lambda x, y: abs( y[1][0] ) - abs( x[1][0] ) )
>  
>      def run(self, args):
>          self.depotPaths = []
> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
> index 49b3359..e09da44 100644
> --- a/contrib/fast-import/git-p4.txt
> +++ b/contrib/fast-import/git-p4.txt
> @@ -191,6 +191,11 @@ git-p4.useclientspec
>  
>    git config [--global] git-p4.useclientspec false
>  
> +The P4CLIENT environment variable should be correctly set for p4 to be
> +able to find the relevant client.  This client spec will be used to
> +both filter the files cloned by git and set the directory layout as
> +specified in the client (this implies --keep-path style semantics).
> +
>  Implementation Details...
>  =========================
>  
>