All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Mazo, Andrey" <amazo@checkvideo.com>
To: "git@vger.kernel.org" <git@vger.kernel.org>
Cc: "Mazo, Andrey" <amazo@checkvideo.com>,
	"Luke Diamand" <luke@diamand.org>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"George Vanburgh" <gvanburgh@bloomberg.net>,
	"Lars Schneider" <larsxschneider@gmail.com>,
	"Miguel Torroja" <miguel.torroja@gmail.com>,
	"Romain Merland" <merlorom@yahoo.fr>,
	"Vitor Antunes" <vitor.hda@gmail.com>,
	"Andrew Oakley" <aoakley@roku.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>, Andrey <ahippo@yandex.com>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: [RFC PATCH 1/1] git-p4: inexact label detection
Date: Wed, 27 Mar 2019 23:08:11 +0000	[thread overview]
Message-ID: <f695acc99834e01f0313c0cc9cb024f960da3ab1.1553727979.git.amazo@checkvideo.com> (raw)
In-Reply-To: <20190326184327.28335-1-amazo@checkvideo.com>

Labels in Perforce are not global, but can be placed on a particular view/subdirectory.
This might pose difficulties when importing only parts of Perforce depot into a git repository.
For example:
 1. Depot layout is as follows:
    //depot/metaproject/branch1/subprojectA/...
    //depot/metaproject/branch1/subprojectB/...
    //depot/metaproject/branch2/subprojectA/...
    //depot/metaproject/branch2/subprojectB/...
 2. Labels are placed as follows:
    * label 1A on //depot/metaproject/branch1/subprojectA/...
    * label 1B on //depot/metaproject/branch1/subprojectB/...
    * label 2A on //depot/metaproject/branch2/subprojectA/...
    * label 2B on //depot/metaproject/branch2/subprojectB/...
 3. The goal is to import
    subprojectA into subprojectA.git and
    subprojectB into subprojectB.git
    preserving all the branches and labels.
 4. Importing subprojectA.
    Label 1A is imported fine because it's placed on certain commit on branch1.
    However, label 1B is not imported because it's placed on a commit in another subproject:
    git-p4 says: "importing label 1B: could not find git commit for changelist ..."
    The same is with label 2A, which is imported; and 2B, which is not.

Currently, there is no easy way (that I'm aware of) to tell git-p4 to
import an empty commit into a desired branch,
so that a label placed on that changelist could be imported as well,
It might be possible to get a similar effect by importing both subprojectA and B in a single git repo,
and then running `git filter-branch --subdirectory-filter subprojectA`,
but this might produce way more irrelevant empty commits, than needed for labels.
(although imported changelists can be limited with git-p4 --changesfile option)
Also, `git filter-branch` is harder to use for incremental imports
or when changes are submitted from git back to Perforce.

As suggested by Luke,
instead of creating an empty commit for the sole purpose of being tagged later,
teach git-p4 to search harder for the next lower changelist,
corresponding to the label in question.

Do this by finding the highest changelist up to the label under all known branches,
(branches are finalized by the time importP4Labels() runs)
and using it instead of a depot-wide changelist corresponding to the label.

This new behavior may not be desired for people,
who want exact label <-> changelist relationship.
So, add a new boolean config parameter git-p4.allowInexactLabels (defaults to false)
to explicitly enable it if needed.
Also, this behavior only appears to be useful in case of multiple branches,
(otherwise, every Perforce changelist should appear in git)
so it's not engaged when running without branch detection.

Detect and report (--verbose) "inexact" tags,
i.e. tags placed on a lower changelist than was in Perforce.
Implement this by comparing a changelist for which a commit was found
with a changelist corresponding to the label on the whole depot.

Note, that the new "inexact" logic works slower
than the original code in case of numerous branches,
because p4 needs to calculate the most recent change for each branch path instead of just one.

This is an alternative solution to "alien" branches concept proposed earlier:
https://public-inbox.org/git/b02df749b9266ac8c73707617a171122156621ab.1553283214.git.amazo@checkvideo.com/

Signed-off-by: Andrey Mazo <amazo@checkvideo.com>
Suggested-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py | 45 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index b04f54b3d1..838c1b43d7 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3186,17 +3186,40 @@ def importP4Labels(self, stream, p4Labels):
             if name in ignoredP4Labels:
                 continue
 
             labelDetails = p4CmdList(['label', "-o", name])[0]
 
-            # get the most recent changelist for each file in this label
-            change = p4Cmd(["changes", "-m", "1"] + ["%s...@%s" % (p, name)
+            if self.detectBranches and gitConfigBool("git-p4.allowInexactLabels"):
+                doInexactLabels = True
+            else:
+                doInexactLabels = False
+
+            # get the most recent changelist in this label for the whole depot
+            depot_wide_changelist = p4Cmd(["changes", "-m", "1"] + ["%s...@%s" % (p, name)
                                 for p in self.depotPaths])
+            if 'change' in depot_wide_changelist:
+                depot_wide_changelist = int(depot_wide_changelist['change'])
+            else:
+                depot_wide_changelist = None
+
+            # get the most recent changelist for each file under branches of interest in this label
+            if doInexactLabels:
+                paths = ["%s...@%s" % (self.depotPaths[0] + p + '/', name) for p in self.knownBranches]
+                changes = p4CmdList(["changes", "-m", "1"] + paths)
+                changes = [int(c['change']) for c in changes if 'change' in c]
+
+                # there may be different "most recent" changelists for different paths.
+                # take the newest since some paths were just modified later than others.
+                if changes:
+                    changelist = max(changes)
+                else:
+                    changelist = None
+            else:
+                changelist = depot_wide_changelist
 
-            if 'change' in change:
+            if changelist:
                 # find the corresponding git commit; take the oldest commit
-                changelist = int(change['change'])
                 if changelist in self.committedChanges:
                     gitCommit = ":%d" % changelist       # use a fast-import mark
                     commitFound = True
                 else:
                     gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
@@ -3216,14 +3239,24 @@ def importP4Labels(self, stream, p4Labels):
                         tmwhen = 1
 
                     when = int(time.mktime(tmwhen))
                     self.streamTag(stream, name, labelDetails, gitCommit, when)
                     if verbose:
-                        print("p4 label %s mapped to git commit %s" % (name, gitCommit))
+                        if depot_wide_changelist == changelist:
+                            isExact = ""
+                        else:
+                            isExact = " inexactly"
+                        print("p4 label %s mapped%s to git commit %s" % (name, isExact, gitCommit))
             else:
                 if verbose:
-                    print("Label %s has no changelists - possibly deleted?" % name)
+                    if depot_wide_changelist:
+                        # there is a changelist corresponding to this label,
+                        # but it's not under any branches of interest.
+                        print("Label %s has no changelists under detected branches -- ignoring" % name)
+                    else:
+                        # there is no changelist corresponding to this label in the whole depot
+                        print("Label %s has no changelists - possibly deleted?" % name)
 
             if not commitFound:
                 # We can't import this label; don't try again as it will get very
                 # expensive repeatedly fetching all the files for labels that will
                 # never be imported. If the label is moved in the future, the

base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
-- 
2.19.2


  reply	other threads:[~2019-03-27 23:09 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-04 17:34 [PATCH 0/5] git-p4: a few assorted fixes for branches, excludes Mazo, Andrey
2019-03-04 17:34 ` [PATCH 1/5] git-p4: detect/prevent infinite loop in gitCommitByP4Change() Mazo, Andrey
2019-03-04 17:34 ` [PATCH 2/5] git-p4: match branches case insensitively if configured Mazo, Andrey
2019-03-04 17:34 ` [PATCH 3/5] git-p4: don't groom exclude path list on every commit Mazo, Andrey
2019-03-04 17:34 ` [PATCH 4/5] git-p4: add failing test for "don't exclude other files with same prefix" Mazo, Andrey
2019-03-04 17:34 ` [PATCH 5/5] git-p4: don't exclude other files with same prefix Mazo, Andrey
2019-03-21 22:32 ` [PATCH v2 0/7] git-p4: a few assorted fixes for branches, excludes Mazo, Andrey
2019-03-21 22:32   ` [PATCH v2 1/7] git-p4: detect/prevent infinite loop in gitCommitByP4Change() Mazo, Andrey
2019-03-21 22:32   ` [PATCH v2 2/7] git-p4: match branches case insensitively if configured Mazo, Andrey
2019-03-23  9:15     ` Luke Diamand
2019-03-25 17:20       ` Mazo, Andrey
2019-03-21 22:32   ` [PATCH v2 3/7] git-p4: don't groom exclude path list on every commit Mazo, Andrey
2019-03-21 22:33   ` [PATCH v2 4/7] git-p4: add failing test for "don't exclude other files with same prefix" Mazo, Andrey
2019-03-21 22:33   ` [PATCH v2 5/7] git-p4: don't exclude other files with same prefix Mazo, Andrey
2019-03-21 22:33   ` [PATCH v2 6/7] git-p4: add failing test for "git-p4: respect excluded paths when detecting branches" Mazo, Andrey
2019-03-21 22:33   ` [PATCH v2 7/7] git-p4: respect excluded paths when detecting branches Mazo, Andrey
2019-03-22 19:54   ` [RFC PATCH 0/2] git-p4: "alien" branches and load changelist info from file Mazo, Andrey
2019-03-22 19:54     ` [RFC PATCH 1/2] git-p4: introduce alien branch mappings Mazo, Andrey
2019-03-23  9:08       ` Luke Diamand
2019-03-26 18:43         ` Mazo, Andrey
2019-03-27 23:08           ` Mazo, Andrey [this message]
2019-03-22 19:54     ` [RFC PATCH 2/2] git-p4: support loading changelist descriptions from files Mazo, Andrey
2019-03-23  8:44       ` Luke Diamand
2019-03-25 17:46         ` [RFC PATCH 2/2] git-p4: support loading changelist descriptions Mazo, Andrey
2019-04-01 18:02   ` [PATCH v3 0/8] git-p4: a few assorted fixes for branches, excludes Mazo, Andrey
2019-04-01 18:02     ` [PATCH v3 1/8] git-p4: detect/prevent infinite loop in gitCommitByP4Change() Mazo, Andrey
2019-04-01 18:02     ` [PATCH v3 2/8] git-p4: add failing test for "git-p4: match branches case insensitively if configured" Mazo, Andrey
2019-04-02 12:05       ` SZEDER Gábor
2019-04-02 17:13         ` Mazo, Andrey
2019-04-03  7:10         ` Junio C Hamano
2019-04-01 18:02     ` [PATCH v3 3/8] git-p4: match branches case insensitively if configured Mazo, Andrey
2019-04-01 18:02     ` [PATCH v3 4/8] git-p4: don't groom exclude path list on every commit Mazo, Andrey
2019-04-01 18:02     ` [PATCH v3 5/8] git-p4: add failing test for "don't exclude other files with same prefix" Mazo, Andrey
2019-04-01 18:02     ` [PATCH v3 6/8] git-p4: don't exclude other files with same prefix Mazo, Andrey
2019-04-01 18:02     ` [PATCH v3 7/8] git-p4: add failing test for "git-p4: respect excluded paths when detecting branches" Mazo, Andrey
2019-04-01 18:02     ` [PATCH v3 8/8] git-p4: respect excluded paths when detecting branches Mazo, Andrey
2019-04-01 19:54     ` [PATCH v3 0/8] git-p4: a few assorted fixes for branches, excludes Mazo, Andrey
2019-04-02  0:13     ` [RFC PATCH v2 0/2] git-p4: inexact labels and load changelist description from file Mazo, Andrey
2019-04-02  0:13       ` [RFC PATCH v2 1/2] git-p4: inexact label detection Mazo, Andrey
2019-04-02  0:13       ` [RFC PATCH v2 2/2] git-p4: support loading changelist descriptions from files Mazo, Andrey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f695acc99834e01f0313c0cc9cb024f960da3ab1.1553727979.git.amazo@checkvideo.com \
    --to=amazo@checkvideo.com \
    --cc=ahippo@yandex.com \
    --cc=aoakley@roku.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=gvanburgh@bloomberg.net \
    --cc=larsxschneider@gmail.com \
    --cc=luke@diamand.org \
    --cc=merlorom@yahoo.fr \
    --cc=miguel.torroja@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    --cc=vitor.hda@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.