Re: [ANNOUNCE] Git Merge Contributor's Summit Jan 31, 2019, Brussels
On Fri, 9 Nov 2018 at 10:48, Jeff King wrote: > > On Fri, Nov 09, 2018 at 10:44:10AM +, Luca Milanesio wrote: > > > > On 9 Nov 2018, at 10:42, Jeff King wrote: > > > > > > Git Merge 2019 is happening on February 1st. There will be a > > > Contributor's Summit the day before. Here are the details: > > > > > > When: Thursday, January 31, 2019. 10am-5pm. > > > Where: The Egg[1], Brussels, Belgium > > > What: Round-table discussion about Git > > > Who: All contributors to Git or related projects in the Git ecosystem > > > are invited; if you're not sure if you qualify, please ask! > > > > Hi Jeff, > > is Gerrit included in the "Git ecosystem"? > > Yeah, I think so. At least the Git parts of it. :) git-p4? (The git parts obviously!) > > -Peff
Re: [PATCH 7/9] tests: include detailed trace logs with --write-junit-xml upon failure
On 4 September 2018 at 12:09, Johannes Schindelin
wrote:
> Hi Eric,
>
> On Tue, 4 Sep 2018, Eric Sunshine wrote:
>
>> On Mon, Sep 3, 2018 at 5:10 PM Johannes Schindelin via GitGitGadget
>> wrote:
>> > So let's do something different in VSTS: let's run all the tests with
>> > `--quiet` first, and only if a failure is encountered, try to trace the
>> > commands as they are executed. [...]
Is this re-running just an individual test on its own or all the tests
within a single file?
The latter shouldn't need this at all. And the former, I'm not sure
will actually work - most of the tests assume some particular p4
state. But perhaps I'm missing something?
I also think it does look kind of ugly. And if there's one thing I've
learned, it's that the ugly hack you write today with the words "we'll
tidy this up later" goes on to live with you forever!
>> >
>> > Signed-off-by: Johannes Schindelin
>> > ---
>> > diff --git a/t/test-lib.sh b/t/test-lib.sh
>> > @@ -445,10 +452,37 @@ test_ok_ () {
>> > test_failure_ () {
>> > if test -n "$write_junit_xml"
>> > then
>> > + if test -z "$GIT_TEST_TEE_OUTPUT_FILE"
>> > + then
>> > + case "$(type kill_p4d 2>/dev/null | head -n 1)" in
>> > + *function*) kill_p4d;;
>> > + esac
>> > +
>> > + case "$(type stop_git_daemon 2>/dev/null |
>> > + head -n 1)" in
>> > + *function*) stop_git_daemon;;
>> > + esac
>>
>> In the long run, it might make more sense, and be more scalable, to
>> have those scripts define a "prepare_for_rerun" variable or function
>> which this code then runs generically rather than having special
>> knowledge of those facilities.
>>
>> I could imagine, for instance, test-lib.sh defining a no-op:
>>
>> test_failure_prepare_rerun () {}
>>
>> and then each of those scripts overriding the function:
>>
>> # in lib-git-p4.sh
>> test_failure_prepare_rerun () {
>> kill_p4d
>> }
>>
>> # in lib-git-daemon.sh
>> test_failure_prepare_rerun () {
>> stop_git_daemon
>> }
>
> Or we could implement `test_atexit` (similar to `test_when_finished`, but
> to be executed at `test_done` time). I guess that's what the p4 and daemon
> tests really needed to begin with (and probably also the apache2-using
> tests).
>
>>
>> > + # re-run with --verbose-log
>> > + echo "# Re-running: $junit_rerun_options_sq" >&2
>> > +
>> > + cd "$TEST_DIRECTORY" &&
>> > + eval "${TEST_SHELL_PATH}"
>> > "$junit_rerun_options_sq" \
>> > + >/dev/null 2>&1
>> > + status=$?
>> > +
>> > + say_color "" "$(test 0 = $status ||
>> > + echo "not ")ok $test_count - (re-ran with
>> > trace)"
>> > + say "1..$test_count"
>> > + GIT_EXIT_OK=t
>> > + exit $status
>> > + fi
>> > +
>> > junit_insert="> > junit_insert="$junit_insert $(xml_attr_encode "$1")\">"
>> > junit_insert="$junit_insert $(xml_attr_encode \
>> > - "$(printf '%s\n' "$@" | sed 1d)")"
>> > + "$(cat "$GIT_TEST_TEE_OUTPUT_FILE")")"
>> > + >"$GIT_TEST_TEE_OUTPUT_FILE"
>> > junit_insert="$junit_insert"
>> > write_junit_xml_testcase "$1" " $junit_insert"
>> > fi
>>
>> This junit-related stuff is getting pretty lengthy. I wonder if it
>> would make sense to pull it out to its own function at some point
>> (again, in the long run).
>
> Now that you mention it... I agree. This is getting long.
>
> In the short run, I have two things to consider, though: I want to make
> this work first, then think about introducing a layer of abstraction, and
> I want to go on vacation tomorrow.
>
> So I agree that this is something to be considered in the long run, i.e.
> not right now ;-)
>
> Thanks,
> Dscho
Re: [PATCH 7/9] tests: include detailed trace logs with --write-junit-xml upon failure
On 5 September 2018 at 13:39, Johannes Schindelin wrote: > Hi Luke, > > On Wed, 5 Sep 2018, Luke Diamand wrote: > >> On 4 September 2018 at 12:09, Johannes Schindelin >> wrote: >> > >> > On Tue, 4 Sep 2018, Eric Sunshine wrote: >> > >> >> On Mon, Sep 3, 2018 at 5:10 PM Johannes Schindelin via GitGitGadget >> >> wrote: >> >> > So let's do something different in VSTS: let's run all the tests with >> >> > `--quiet` first, and only if a failure is encountered, try to trace the >> >> > commands as they are executed. [...] >> >> Is this re-running just an individual test on its own or all the tests >> within a single file? > > Upon encountering a failed test, it is re-running the entire test script > afresh. > >> The latter shouldn't need this at all. > > Please do not let me die dumb. In other words, I would love for you to > explain what exactly you mean by that sentence. Just re-run the script. You shouldn't need to kill p4d, as each script starts up its own instance of p4d, and shuts it down when it exits. $ cd t $ ./t9800-git-p4-basic.sh Ctrl^C $ ./t9800-git-p4-basic.sh -v There's a cleanup() function in lib-git-p4.sh which kills the p4d server, and that's invoked via: trap cleanup EXIT That's the only cleanup that each of the scripts require AFAIK. > >> And the former, I'm not sure will actually work - most of the tests >> assume some particular p4 state. But perhaps I'm missing something? > > No, the former would not work at all. Not only for the p4 tests: Git's > tests frequently commit the deadly sin of relying on output of one another > (wreaking havoc e.g. when test cases are skipped due to missing > prerequisites, and latter test cases relying on their output). It is not > the only thing that is wrong with the test suite, of course. > >> I also think it does look kind of ugly. And if there's one thing I've >> learned, it's that the ugly hack you write today with the words "we'll >> tidy this up later" goes on to live with you forever! > > Okay. > > (And having read lib-git-p4.sh, I kind of see where you learned that.) > > But maybe you also have some splendid idea what to do instead? Because > doing something about it, that we need. We can't just say "oh, the only > solution we found is ugly, so let's not do it at all". > > I am even going so far as to say: unless you have a better idea, it is > pretty detrimental to criticize the current approach. It is the opposite > of constructive. > > So let's hear some ideas how to improve the situation, m'kay? > > Just as a reminder, this is the problem I want to solve: I want to run the > tests in a light-weight manner, with minimal output, and only in case of > an error do I want to crank up the verbosity. Instead of wasting most of the > effort to log everything and then throwing it away in most of the common > cases, I suggest to re-run the entire test. > > What do you suggest to solve this? > I don't know about any other tests, but the git-p4 tests don't take any longer in verbose mode. So one simple solution is to just run it in verbose mode - unless there are other tests which have more overhead. The trap/exit/cleanup method that the git-p4 tests already use would seem to be ideally suited to cleaning up everything on exit. There might be some specific tests where this doesn't quite work out, if you let me know what they are I can have a look at fixing them for you. Thanks, Luke
[PATCHv1 1/3] git-p4: do not fail in verbose mode for missing 'fileSize' key
If deleting or moving a file, sometimes P4 doesn't report the file size.
The code handles this just fine but some logging crashes. Stop this
happening.
There was some earlier discussion on the list about this:
https://public-inbox.org/git/xmqq1sqpp1vv@gitster.mtv.corp.google.com/
Signed-off-by: Luke Diamand
---
git-p4.py | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 7fab255584..5701bad06a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2775,7 +2775,10 @@ def streamOneP4File(self, file, contents):
relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
relPath = self.encodeWithUTF8(relPath)
if verbose:
-size = int(self.stream_file['fileSize'])
+if 'fileSize' in self.stream_file:
+size = int(self.stream_file['fileSize'])
+else:
+size = 0 # deleted files don't get a fileSize apparently
sys.stdout.write('\r%s --> %s (%i MB)\n' % (file['depotFile'],
relPath, size/1024/1024))
sys.stdout.flush()
--
2.19.1.272.gf84b9b09d4
[PATCHv1 2/3] git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved
The branch detection code looks for branches under refs/remotes/p4/... and can end up getting confused if there are unshelved changes in there as well. This happens in the function p4BranchesInGit(). Instead, put the unshelved changes into refs/remotes/p4-unshelved/. Signed-off-by: Luke Diamand --- Documentation/git-p4.txt | 6 +++--- git-p4.py| 3 ++- t/t9832-unshelve.sh | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index 41780a5aa9..c7705ae6e7 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -174,7 +174,7 @@ $ git p4 submit --update-shelve 1234 --update-shelve 2345 Unshelve Unshelving will take a shelved P4 changelist, and produce the equivalent git commit -in the branch refs/remotes/p4/unshelved/. +in the branch refs/remotes/p4-unshelved/. The git commit is created relative to the current origin revision (HEAD by default). If the shelved changelist's parent revisions differ, git-p4 will refuse to unshelve; @@ -182,13 +182,13 @@ you need to be unshelving onto an equivalent tree. The origin revision can be changed with the "--origin" option. -If the target branch in refs/remotes/p4/unshelved already exists, the old one will +If the target branch in refs/remotes/p4-unshelved already exists, the old one will be renamed. $ git p4 sync $ git p4 unshelve 12345 -$ git show refs/remotes/p4/unshelved/12345 +$ git show p4/unshelved/12345 $ git p4 unshelve 12345 diff --git a/git-p4.py b/git-p4.py index 5701bad06a..76c18a22e9 100755 --- a/git-p4.py +++ b/git-p4.py @@ -3956,7 +3956,8 @@ def __init__(self): ] self.verbose = False self.noCommit = False -self.destbranch = "refs/remotes/p4/unshelved" +self.destbranch = "refs/remotes/p4-unshelved" +self.origin = "p4/master" def renameBranch(self, branch_name): """ Rename the existing branch to branch_name.N diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh index 48ec7679b8..c3d15ceea8 100755 --- a/t/t9832-unshelve.sh +++ b/t/t9832-unshelve.sh @@ -54,8 +54,8 @@ EOF cd "$git" && change=$(last_shelved_change) && git p4 unshelve $change && - git show refs/remotes/p4/unshelved/$change | grep -q "Further description" && - git cherry-pick refs/remotes/p4/unshelved/$change && + git show refs/remotes/p4-unshelved/$change | grep -q "Further description" && + git cherry-pick refs/remotes/p4-unshelved/$change && test_path_is_file file2 && test_cmp file1 "$cli"/file1 && test_cmp file2 "$cli"/file2 && @@ -88,7 +88,7 @@ EOF cd "$git" && change=$(last_shelved_change) && git p4 unshelve $change && - git diff refs/remotes/p4/unshelved/$change.0 refs/remotes/p4/unshelved/$change | grep -q file3 + git diff refs/remotes/p4-unshelved/$change.0 refs/remotes/p4-unshelved/$change | grep -q file3 ) ' -- 2.19.1.272.gf84b9b09d4
[PATCHv1 3/3] git-p4: fully support unshelving changelists
The previous git-p4 unshelve support would check for changes
in Perforce to the files being unshelved since the original
shelve, and would complain if any were found.
This was to ensure that the user wouldn't end up with both the
shelved change delta, and some deltas from other changes in their
git commit.
e.g. given fileA:
the
quick
brown
fox
change1: s/the/The/ <- p4 shelve this change
change2: s/fox/Fox/ <- p4 submit this change
git p4 unshelve 1 <- FAIL
This change teaches the P4Unshelve class to always create a parent
commit which matches the P4 tree (for the files being unshelved) at
the point prior to the P4 shelve being created (which is reported
in the p4 description for a shelved changelist).
That then means git-p4 can always create a git commit matching the
P4 shelve that was originally created, without any extra deltas.
The user might still need to use the --origin option though - there
is no way for git-p4 to work out the versions of all of the other
*unchanged* files in the shelve, since this information is not recorded
by Perforce.
Additionally this fixes handling of shelved 'move' operations.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 4 +-
git-p4.py| 84 +++-
t/t9832-unshelve.sh | 69 ++---
3 files changed, 106 insertions(+), 51 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index c7705ae6e7..d8332e99c1 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -177,8 +177,8 @@ Unshelving will take a shelved P4 changelist, and produce
the equivalent git com
in the branch refs/remotes/p4-unshelved/.
The git commit is created relative to the current origin revision (HEAD by
default).
-If the shelved changelist's parent revisions differ, git-p4 will refuse to
unshelve;
-you need to be unshelving onto an equivalent tree.
+A parent commit is created based on the origin, and then the unshelve commit is
+created based on that.
The origin revision can be changed with the "--origin" option.
diff --git a/git-p4.py b/git-p4.py
index 76c18a22e9..1998c3e141 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1306,6 +1306,9 @@ def processContent(self, git_mode, relPath, contents):
return LargeFileSystem.processContent(self, git_mode, relPath,
contents)
class Command:
+delete_actions = ( "delete", "move/delete", "purge" )
+add_actions = ( "add", "move/add" )
+
def __init__(self):
self.usage = "usage: %prog [options]"
self.needsGit = True
@@ -2524,7 +2527,6 @@ def map_in_client(self, depot_path):
return ""
class P4Sync(Command, P4UserMap):
-delete_actions = ( "delete", "move/delete", "purge" )
def __init__(self):
Command.__init__(self)
@@ -2612,20 +2614,7 @@ def checkpoint(self):
if self.verbose:
print("checkpoint finished: " + out)
-def cmp_shelved(self, path, filerev, revision):
-""" Determine if a path at revision #filerev is the same as the file
-at revision @revision for a shelved changelist. If they don't
match,
-unshelving won't be safe (we will get other changes mixed in).
-
-This is comparing the revision that the shelved changelist is
*based* on, not
-the shelved changelist itself.
-"""
-ret = p4Cmd(["diff2", "{0}#{1}".format(path, filerev),
"{0}@{1}".format(path, revision)])
-if verbose:
-print("p4 diff2 path %s filerev %s revision %s => %s" % (path,
filerev, revision, ret))
-return ret["status"] == "identical"
-
-def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0,
origin_revision = 0):
+def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):
self.cloneExclude = [re.sub(r"\.\.\.$", "", path)
for path in self.cloneExclude]
files = []
@@ -2650,17 +2639,6 @@ def extractFilesFromCommit(self, commit, shelved=False,
shelved_cl = 0, origin_r
file["type"] = commit["type%s" % fnum]
if shelved:
file["shelved_cl"] = int(shelved_cl)
-
-# For shelved changelists, check that the revision of each
file that the
-# shelve was based on matches the revision that we are using
for the
-# starting point for git-fast-import (self.initialParent).
Otherwise
-# the resulting diff will contain deltas from multiple commits.
-
-if file["action"] != "add" and \
-not
[PATCHv1 0/3] git-p4: improved unshelving
This patch series teaches the git-p4 unshelve command to handle intervening changes to the Perforce files. At the moment if you try to unshelve a file, and that file has been modified since the shelving, git-p4 refuses. That is so that it doesn't end up generating a commit containing deltas from several P4 changes. This gets to be more annoying as time goes on and the files you are interested in get updated by other people. However, what we can do is to create a parent commit matching the state of the tree when the shelve happened, which then lets git-p4 create a git commit containing just the changes that are wanted. It's still impossible to determine the true state of the complete tree when the P4 shelve was created, since this information is not recorded by Perforce. Manual intervention is required to fix that. There are also a few other smaller fixes, the main one being that it no longer unshelves into refs/remotes/p4/master/unshelved, but instead into refs/remotes/p4-unshelved. That's because the git-p4 branch detection gets confused by branches appearing in refs/remotes/p4. Luke Diamand (3): git-p4: do not fail in verbose mode for missing 'fileSize' key git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved git-p4: fully support unshelving changelists Documentation/git-p4.txt | 10 ++--- git-p4.py| 90 +++- t/t9832-unshelve.sh | 75 ++--- 3 files changed, 117 insertions(+), 58 deletions(-) -- 2.19.1.272.gf84b9b09d4
Re: [PATCHv1 2/3] git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved
On Fri, 12 Oct 2018 at 14:45, Junio C Hamano wrote: > > Luke Diamand writes: > > > The branch detection code looks for branches under refs/remotes/p4/... > > and can end up getting confused if there are unshelved changes in > > there as well. This happens in the function p4BranchesInGit(). > > > > Instead, put the unshelved changes into refs/remotes/p4-unshelved/. > > I am not a p4 user (and not a git-p4 user), so it is a bit hard for > me to assess if this is a backward incompatibile change and if so > how serious potential breakage to existing users would be. I don't think it's a particularly serious breakage - it reports the branch it unshelves to, so it should be fairly obvious. However, maybe it would make sense to pull this into a separate commit to make it more obvious? I should have thought of that before submitting. > > > > > -If the target branch in refs/remotes/p4/unshelved already exists, the old > > one will > > +If the target branch in refs/remotes/p4-unshelved already exists, the old > > one will > > be renamed. > > > > > > $ git p4 sync > > $ git p4 unshelve 12345 > > -$ git show refs/remotes/p4/unshelved/12345 > > +$ git show p4/unshelved/12345 > > Isn't this "p4-unshelved/12345" now? Yes, I think another reason to pull into a separate commit. Luke >
Re: [PATCH v2 09/13] git-p4: use `test_atexit` to kill the daemon
On Mon, 15 Oct 2018 at 11:12, Johannes Schindelin via GitGitGadget wrote: > > From: Johannes Schindelin > > This should be more reliable than the current method, and prepares the > test suite for a consistent way to clean up before re-running the tests > with different options. > I'm finding that it's leaving p4d processes lying around. e.g. $ ./t9820-git-p4-editor-handling.sh $ ./t9820-git-p4-editor-handling.sh And also $ ./t9800-git-p4-basic.sh Ctrl-C $ ./t9800-git-p4-basic.sh $ ps | grep p4d 21392 pts/100:00:00 p4d < Luke
Re: [PATCHv1 2/3] git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved
On Fri, 12 Oct 2018 at 19:19, Luke Diamand wrote: > > On Fri, 12 Oct 2018 at 14:45, Junio C Hamano wrote: > > > > Luke Diamand writes: > > > > > The branch detection code looks for branches under refs/remotes/p4/... > > > and can end up getting confused if there are unshelved changes in > > > there as well. This happens in the function p4BranchesInGit(). > > > > > > Instead, put the unshelved changes into refs/remotes/p4-unshelved/. > > > > I am not a p4 user (and not a git-p4 user), so it is a bit hard for > > me to assess if this is a backward incompatibile change and if so > > how serious potential breakage to existing users would be. > > I don't think it's a particularly serious breakage - it reports the > branch it unshelves to, so it should be fairly obvious. > > However, maybe it would make sense to pull this into a separate commit > to make it more obvious? I should have thought of that before > submitting. > > > > > > > > > -If the target branch in refs/remotes/p4/unshelved already exists, the > > > old one will > > > +If the target branch in refs/remotes/p4-unshelved already exists, the > > > old one will > > > be renamed. > > > > > > > > > $ git p4 sync > > > $ git p4 unshelve 12345 > > > -$ git show refs/remotes/p4/unshelved/12345 > > > +$ git show p4/unshelved/12345 > > > > Isn't this "p4-unshelved/12345" now? > > Yes, I think another reason to pull into a separate commit. D'oh. It's already in a separate commit. I'll re-roll fixing that documentation. I think it will be fine to change the branch that the unshelving happens into - I think it's very unlikely anyone is basing some automated scripts on this, because of the way that unshelving is used anyway. Luke
[PATCHv2 3/3] git-p4: fully support unshelving changelists
The previous git-p4 unshelve support would check for changes
in Perforce to the files being unshelved since the original
shelve, and would complain if any were found.
This was to ensure that the user wouldn't end up with both the
shelved change delta, and some deltas from other changes in their
git commit.
e.g. given fileA:
the
quick
brown
fox
change1: s/the/The/ <- p4 shelve this change
change2: s/fox/Fox/ <- p4 submit this change
git p4 unshelve 1 <- FAIL
This change teaches the P4Unshelve class to always create a parent
commit which matches the P4 tree (for the files being unshelved) at
the point prior to the P4 shelve being created (which is reported
in the p4 description for a shelved changelist).
That then means git-p4 can always create a git commit matching the
P4 shelve that was originally created, without any extra deltas.
The user might still need to use the --origin option though - there
is no way for git-p4 to work out the versions of all of the other
*unchanged* files in the shelve, since this information is not recorded
by Perforce.
Additionally this fixes handling of shelved 'move' operations.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 4 +-
git-p4.py| 84 +++-
t/t9832-unshelve.sh | 69 ++---
3 files changed, 106 insertions(+), 51 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 6c0017e36e..f0a0280954 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -177,8 +177,8 @@ Unshelving will take a shelved P4 changelist, and produce
the equivalent git com
in the branch refs/remotes/p4-unshelved/.
The git commit is created relative to the current origin revision (HEAD by
default).
-If the shelved changelist's parent revisions differ, git-p4 will refuse to
unshelve;
-you need to be unshelving onto an equivalent tree.
+A parent commit is created based on the origin, and then the unshelve commit is
+created based on that.
The origin revision can be changed with the "--origin" option.
diff --git a/git-p4.py b/git-p4.py
index 76c18a22e9..1998c3e141 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1306,6 +1306,9 @@ def processContent(self, git_mode, relPath, contents):
return LargeFileSystem.processContent(self, git_mode, relPath,
contents)
class Command:
+delete_actions = ( "delete", "move/delete", "purge" )
+add_actions = ( "add", "move/add" )
+
def __init__(self):
self.usage = "usage: %prog [options]"
self.needsGit = True
@@ -2524,7 +2527,6 @@ def map_in_client(self, depot_path):
return ""
class P4Sync(Command, P4UserMap):
-delete_actions = ( "delete", "move/delete", "purge" )
def __init__(self):
Command.__init__(self)
@@ -2612,20 +2614,7 @@ def checkpoint(self):
if self.verbose:
print("checkpoint finished: " + out)
-def cmp_shelved(self, path, filerev, revision):
-""" Determine if a path at revision #filerev is the same as the file
-at revision @revision for a shelved changelist. If they don't
match,
-unshelving won't be safe (we will get other changes mixed in).
-
-This is comparing the revision that the shelved changelist is
*based* on, not
-the shelved changelist itself.
-"""
-ret = p4Cmd(["diff2", "{0}#{1}".format(path, filerev),
"{0}@{1}".format(path, revision)])
-if verbose:
-print("p4 diff2 path %s filerev %s revision %s => %s" % (path,
filerev, revision, ret))
-return ret["status"] == "identical"
-
-def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0,
origin_revision = 0):
+def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):
self.cloneExclude = [re.sub(r"\.\.\.$", "", path)
for path in self.cloneExclude]
files = []
@@ -2650,17 +2639,6 @@ def extractFilesFromCommit(self, commit, shelved=False,
shelved_cl = 0, origin_r
file["type"] = commit["type%s" % fnum]
if shelved:
file["shelved_cl"] = int(shelved_cl)
-
-# For shelved changelists, check that the revision of each
file that the
-# shelve was based on matches the revision that we are using
for the
-# starting point for git-fast-import (self.initialParent).
Otherwise
-# the resulting diff will contain deltas from multiple commits.
-
-if file["action"] != "add" and \
-not
[PATCHv2 1/3] git-p4: do not fail in verbose mode for missing 'fileSize' key
If deleting or moving a file, sometimes P4 doesn't report the file size.
The code handles this just fine but some logging crashes. Stop this
happening.
There was some earlier discussion on the list about this:
https://public-inbox.org/git/xmqq1sqpp1vv@gitster.mtv.corp.google.com/
Signed-off-by: Luke Diamand
---
git-p4.py | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 7fab255584..5701bad06a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2775,7 +2775,10 @@ def streamOneP4File(self, file, contents):
relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
relPath = self.encodeWithUTF8(relPath)
if verbose:
-size = int(self.stream_file['fileSize'])
+if 'fileSize' in self.stream_file:
+size = int(self.stream_file['fileSize'])
+else:
+size = 0 # deleted files don't get a fileSize apparently
sys.stdout.write('\r%s --> %s (%i MB)\n' % (file['depotFile'],
relPath, size/1024/1024))
sys.stdout.flush()
--
2.19.1.331.gae0ed827e6
[PATCHv2 2/3] git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved
The branch detection code looks for branches under refs/remotes/p4/... and can end up getting confused if there are unshelved changes in there as well. This happens in the function p4BranchesInGit(). Instead, put the unshelved changes into refs/remotes/p4-unshelved/. Signed-off-by: Luke Diamand --- Documentation/git-p4.txt | 6 +++--- git-p4.py| 3 ++- t/t9832-unshelve.sh | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index 41780a5aa9..6c0017e36e 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -174,7 +174,7 @@ $ git p4 submit --update-shelve 1234 --update-shelve 2345 Unshelve Unshelving will take a shelved P4 changelist, and produce the equivalent git commit -in the branch refs/remotes/p4/unshelved/. +in the branch refs/remotes/p4-unshelved/. The git commit is created relative to the current origin revision (HEAD by default). If the shelved changelist's parent revisions differ, git-p4 will refuse to unshelve; @@ -182,13 +182,13 @@ you need to be unshelving onto an equivalent tree. The origin revision can be changed with the "--origin" option. -If the target branch in refs/remotes/p4/unshelved already exists, the old one will +If the target branch in refs/remotes/p4-unshelved already exists, the old one will be renamed. $ git p4 sync $ git p4 unshelve 12345 -$ git show refs/remotes/p4/unshelved/12345 +$ git show p4-unshelved/12345 $ git p4 unshelve 12345 diff --git a/git-p4.py b/git-p4.py index 5701bad06a..76c18a22e9 100755 --- a/git-p4.py +++ b/git-p4.py @@ -3956,7 +3956,8 @@ def __init__(self): ] self.verbose = False self.noCommit = False -self.destbranch = "refs/remotes/p4/unshelved" +self.destbranch = "refs/remotes/p4-unshelved" +self.origin = "p4/master" def renameBranch(self, branch_name): """ Rename the existing branch to branch_name.N diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh index 48ec7679b8..c3d15ceea8 100755 --- a/t/t9832-unshelve.sh +++ b/t/t9832-unshelve.sh @@ -54,8 +54,8 @@ EOF cd "$git" && change=$(last_shelved_change) && git p4 unshelve $change && - git show refs/remotes/p4/unshelved/$change | grep -q "Further description" && - git cherry-pick refs/remotes/p4/unshelved/$change && + git show refs/remotes/p4-unshelved/$change | grep -q "Further description" && + git cherry-pick refs/remotes/p4-unshelved/$change && test_path_is_file file2 && test_cmp file1 "$cli"/file1 && test_cmp file2 "$cli"/file2 && @@ -88,7 +88,7 @@ EOF cd "$git" && change=$(last_shelved_change) && git p4 unshelve $change && - git diff refs/remotes/p4/unshelved/$change.0 refs/remotes/p4/unshelved/$change | grep -q file3 + git diff refs/remotes/p4-unshelved/$change.0 refs/remotes/p4-unshelved/$change | grep -q file3 ) ' -- 2.19.1.331.gae0ed827e6
[PATCHv2 0/3] git-p4: improved unshelving - small fixes
This is the same as my earlier patch other than fixing the documentation to reflect the change to the destination branch, as noticed by Junio. Luke Diamand (3): git-p4: do not fail in verbose mode for missing 'fileSize' key git-p4: unshelve into refs/remotes/p4-unshelved, not refs/remotes/p4/unshelved git-p4: fully support unshelving changelists Documentation/git-p4.txt | 10 ++--- git-p4.py| 90 +++- t/t9832-unshelve.sh | 75 ++--- 3 files changed, 117 insertions(+), 58 deletions(-) -- 2.19.1.331.gae0ed827e6
Re: [PATCH v2 09/13] git-p4: use `test_atexit` to kill the daemon
On Mon, 15 Oct 2018 at 16:02, Johannes Schindelin
wrote:
>
> Hi Luke,
>
> On Mon, 15 Oct 2018, Luke Diamand wrote:
>
> > On Mon, 15 Oct 2018 at 11:12, Johannes Schindelin via GitGitGadget
> > wrote:
> > >
> > > From: Johannes Schindelin
> > >
> > > This should be more reliable than the current method, and prepares the
> > > test suite for a consistent way to clean up before re-running the tests
> > > with different options.
> > >
> >
> > I'm finding that it's leaving p4d processes lying around.
>
> That's a bummer!
>
> > e.g.
> >
> > $ ./t9820-git-p4-editor-handling.sh
> >
> > $ ./t9820-git-p4-editor-handling.sh
> >
>
> Since I do not currently have a setup with p4d installed, can you run that
> with `sh -x ...` and see whether this code path is hit?
All you need to do is to put p4 and p4d in your PATH.
https://www.perforce.com/downloads/helix-core-p4d
https://www.perforce.com/downloads/helix-command-line-client-p4
The server is free to use for a small number of users, you don't need
to do anything to make it go.
>
> test_done () {
> GIT_EXIT_OK=t
>
> + test -n "$immediate" || test_atexit_handler
> +
+ test -n
+ test_atexit_handler
./t9820-git-p4-editor-handling.sh: 764:
./t9820-git-p4-editor-handling.sh: test_atexit_handler: not found
Is that expected?
> if test -n "$write_junit_xml" && test -n "$junit_xml_path"
> then
>
> > And also
> >
> > $ ./t9800-git-p4-basic.sh
> >
> > Ctrl-C
>
> Oh, you're right. I think I need to do something in this line:
>
> trap 'exit $?' INT
>
> in t/test-lib.sh, something like
>
> trap 'exit_code=$?; test_atexit_handler; exit $exit_code' INT
>
> would you agree? (And: could you test?)
Not sure.
Send me a patch and I can try it out.
Thanks,
Luke
Re: What's cooking in git.git (Jun 2018, #02; Mon, 4)
On 4 June 2018 at 14:57, Junio C Hamano wrote: > Here are the topics that have been cooking. Commits prefixed with > '-' are only in 'pu' (proposed updates) while commits prefixed with > '+' are in 'next'. The ones marked with '.' do not appear in any of > the integration branches, but I am still holding onto them. > > Generally I try to avoid sending issues of "What's cooking" report > in close succession, but as I plan to go offline for most of the > remainder of the week, here is how the tree looks like as of > tonight, just after tagging 2.18-rc1. Hopefully we'll have another > rc next week and then the real thing a week after that. > > You can find the changes described here in the integration branches > of the repositories listed at > > http://git-blame.blogspot.com/p/git-public-repositories.html > > > * rm/p4-submit-with-commit-option (2018-05-21) 1 commit > - git-p4: add options --commit and --disable-rebase > > Needs sign-off. I think the signed-off version was sent in just last week: http://lists-archives.com/git/925692-git-p4-add-options-commit-and-disable-rebase.html Thanks, Luke
[PATCHv1 0/2] git-p4: disable sync after submit
This is a small patch to git-p4 to disable the automatic sync after submit. In my day-to-day work, I have a central git-p4 repo which is automatically kept up-to-date, so the repos where I actually work and submit from don't even need the sync. I usually end up hitting Ctrl-C partway through the sync. I imagine other people with large git-p4 projects do something similar and have the same problem. This also updates Merland's recent submit-selection change so that both options can be set via configuration rather than being set on the command line. Luke Diamand (2): git-p4: disable-rebase: allow setting this via configuration git-p4: add option to disable syncing of p4/master with p4 Documentation/git-p4.txt | 13 - git-p4.py| 33 + 2 files changed, 33 insertions(+), 13 deletions(-) -- 2.17.0.392.gdeb1a6e9b7
[PATCHv1 2/2] git-p4: add option to disable syncing of p4/master with p4
Add an option to the git-p4 submit command to disable syncing
with Perforce.
This is useful for the case where a git-p4 mirror has been setup
on a server somewhere, running from (e.g.) cron, and developers
then clone from this. Having the local cloned copy also sync
from Perforce just isn't useful.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 8
git-p4.py| 31 ---
2 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 3d83842e47..8f6a7543fd 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -369,6 +369,11 @@ These options can be used to modify 'git p4 submit'
behavior.
Disable the automatic rebase after all commits have been successfully
submitted. Can also be set with git-p4.disableRebase.
+--disable-p4sync::
+Disable the automatic sync of p4/master from Perforce after commit have
+been submitted. Implies --disable-rebase. Can also be set with
+git-p4.disableP4Sync. Sync with origin/master still goes ahead if possible.
+
Rebase options
~~
These options can be used to modify 'git p4 rebase' behavior.
@@ -693,6 +698,9 @@ git-p4.conflict::
git-p4.disableRebase::
Do not rebase the tree against p4/master following a submit.
+git-p4.disableP4Sync::
+Do not sync p4/master with Perforce following a submit. Implies
git-p4.disableRebase.
+
IMPLEMENTATION DETAILS
--
* Changesets from p4 are imported using Git fast-import.
diff --git a/git-p4.py b/git-p4.py
index 5ab9421af8..b9e79f1d8b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1368,7 +1368,9 @@ def __init__(self):
help="submit only the specified
commit(s), one commit or xxx..xxx"),
optparse.make_option("--disable-rebase",
dest="disable_rebase", action="store_true",
help="Disable rebase after submit is
completed. Can be useful if you "
- "work from a local git branch that is not
master")
+ "work from a local git branch that is not
master"),
+optparse.make_option("--disable-p4sync",
dest="disable_p4sync", action="store_true",
+ help="Skip perforce sync of p4/master
after submit or shelve"),
]
self.description = "Submit changes from git to the perforce depot."
self.usage += " [name of git branch to submit into perforce depot]"
@@ -1380,6 +1382,7 @@ def __init__(self):
self.update_shelve = list()
self.commit = ""
self.disable_rebase = gitConfigBool("git-p4.disableRebase")
+self.disable_p4sync = gitConfigBool("git-p4.disableP4Sync")
self.prepare_p4_only = False
self.conflict_behavior = None
self.isWindows = (platform.system() == "Windows")
@@ -2240,11 +2243,14 @@ def run(self, args):
sync = P4Sync()
if self.branch:
sync.branch = self.branch
-sync.run([])
+if self.disable_p4sync:
+sync.sync_origin_only()
+else:
+sync.run([])
-if self.disable_rebase is False:
-rebase = P4Rebase()
-rebase.rebase()
+if not self.disable_rebase:
+rebase = P4Rebase()
+rebase.rebase()
else:
if len(applied) == 0:
@@ -3324,6 +3330,14 @@ def importChanges(self, changes, shelved=False,
origin_revision=0):
print self.gitError.read()
sys.exit(1)
+def sync_origin_only(self):
+if self.syncWithOrigin:
+self.hasOrigin = originP4BranchesExist()
+if self.hasOrigin:
+if not self.silent:
+print 'Syncing with origin first, using "git fetch origin"'
+system("git fetch origin")
+
def importHeadRevision(self, revision):
print "Doing initial import of %s from revision %s into %s" % ('
'.join(self.depotPaths), revision, self.branch)
@@ -3402,12 +3416,7 @@ def run(self, args):
else:
self.refPrefix = "refs/heads/p4/"
-if self.syncWithOrigin:
-self.hasOrigin = originP4BranchesExist()
-if self.hasOrigin:
-if not self.silent:
-print 'Syncing with origin first, using "git fetch origin"'
-system("git fetch origin")
+self.sync_origin_only()
branch_arg_given = bool(self.branch)
if len(self.branch) == 0:
--
2.17.0.392.gdeb1a6e9b7
[PATCHv1 1/2] git-p4: disable-rebase: allow setting this via configuration
This just lets you set the --disable-rebase option with the
git configuration options git-p4.disableRebase. If you're
using this option, you probably want to set it all the time
for a given repo.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 5 -
git-p4.py| 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index e8452528fc..3d83842e47 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -367,7 +367,7 @@ These options can be used to modify 'git p4 submit'
behavior.
--disable-rebase::
Disable the automatic rebase after all commits have been successfully
-submitted.
+submitted. Can also be set with git-p4.disableRebase.
Rebase options
~~
@@ -690,6 +690,9 @@ git-p4.conflict::
Specify submit behavior when a conflict with p4 is found, as per
--conflict. The default behavior is 'ask'.
+git-p4.disableRebase::
+Do not rebase the tree against p4/master following a submit.
+
IMPLEMENTATION DETAILS
--
* Changesets from p4 are imported using Git fast-import.
diff --git a/git-p4.py b/git-p4.py
index c4581d20a6..5ab9421af8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1379,7 +1379,7 @@ def __init__(self):
self.shelve = False
self.update_shelve = list()
self.commit = ""
-self.disable_rebase = False
+self.disable_rebase = gitConfigBool("git-p4.disableRebase")
self.prepare_p4_only = False
self.conflict_behavior = None
self.isWindows = (platform.system() == "Windows")
--
2.17.0.392.gdeb1a6e9b7
[PATCHv1 1/1] git-p4: better error reporting when p4 fails
Currently when p4 fails to run, git-p4 just crashes with an obscure
error message.
For example, if the P4 ticket has expired, you get:
Error: Cannot locate perforce checkout of in client view
This change checks whether git-p4 can talk to the Perforce server when
the first P4 operation is attempted, and tries to print a meaningful
error message if it fails.
Signed-off-by: Luke Diamand
---
git-p4.py | 55 +
t/t9833-errors.sh | 78 +++
2 files changed, 133 insertions(+)
create mode 100755 t/t9833-errors.sh
diff --git a/git-p4.py b/git-p4.py
index b9e79f1d8b..66652f8280 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -50,6 +50,8 @@ def __str__(self):
# Grab changes in blocks of this many revisions, unless otherwise requested
defaultBlockSize = 512
+p4_access_checked = False
+
def p4_build_cmd(cmd):
"""Build a suitable p4 command line.
@@ -91,6 +93,13 @@ def p4_build_cmd(cmd):
real_cmd = ' '.join(real_cmd) + ' ' + cmd
else:
real_cmd += cmd
+
+# now check that we can actually talk to the server
+global p4_access_checked
+if not p4_access_checked:
+p4_access_checked = True
+p4_check_access()
+
return real_cmd
def git_dir(path):
@@ -264,6 +273,52 @@ def p4_system(cmd):
if retcode:
raise CalledProcessError(retcode, real_cmd)
+def die_bad_access(s):
+die("failure accessing depot: {0}".format(s.rstrip()))
+
+def p4_check_access(min_expiration=1):
+""" Check if we can access Perforce - account still logged in
+"""
+results = p4CmdList(["login", "-s"])
+
+if len(results) == 0:
+# should never get here: always get either some results, or a
p4ExitCode
+assert("could not parse response from perforce")
+
+result = results[0]
+
+if 'p4ExitCode' in result:
+# p4 returned non-zero status, e.g. P4PORT invalid, or p4 not in path
+die_bad_access("could not run p4")
+
+code = result.get("code")
+if not code:
+# we get here if we couldn't connect and there was nothing to unmarshal
+die_bad_access("could not connect")
+
+elif code == "stat":
+expiry = result.get("TicketExpiration")
+if expiry:
+expiry = int(expiry)
+if expiry > min_expiration:
+# ok to carry on
+return
+else:
+die_bad_access("perforce ticket expires in {0}
seconds".format(expiry))
+
+else:
+# account without a timeout - all ok
+return
+
+elif code == "error":
+data = result.get("data")
+if data:
+die_bad_access("p4 error: {0}".format(data))
+else:
+die_bad_access("unknown error")
+else:
+die_bad_access("unknown error code {0}".format(code))
+
_p4_version_string = None
def p4_version_string():
"""Read the version string, showing just the last line, which
diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
new file mode 100755
index 00..9ba892de7a
--- /dev/null
+++ b/t/t9833-errors.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='git p4 errors'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'add p4 files' '
+ (
+ cd "$cli" &&
+ echo file1 >file1 &&
+ p4 add file1 &&
+ p4 submit -d "file1"
+ )
+'
+
+# after this test, the default user requires a password
+test_expect_success 'error handling' '
+ git p4 clone --dest="$git" //depot@all &&
+ (
+ cd "$git" &&
+ P4PORT=: test_must_fail git p4 submit 2>errmsg
+ ) &&
+ p4 passwd -P newpassword &&
+ (
+ P4PASSWD=badpassword test_must_fail git p4 clone //depot/foo
2>errmsg &&
+ grep -q "failure accessing depot.*P4PASSWD" errmsg
+ )
+'
+
+test_expect_success 'ticket logged out' '
+ P4TICKETS="$cli/tickets" &&
+ echo "newpassword" | p4 login &&
+ (
+ cd "$git" &&
+ test_commit "ticket-auth-check" &&
+ p4 logout &&
+ test_must_fail git p4 submit 2>errmsg &&
+ grep -q "failure accessing depot" errmsg
+ )
+'
+
+test_expect_success 'create group with short ticket expiry' '
[PATCHv1 0/1] git-p4: better error reporting
If git-p4 cannot talk to the Perforce server, it will either give a confusing error message, or just crash. Even I get tripped up by this. This change just checks that the client can talk to the server, and in particular that the user is logged in (the default login timeout is 12 hours). It would be nice to also check for ticket expiration during long Perforce operations, but this change does not attempt to do that. Luke Diamand (1): git-p4: better error reporting when p4 fails git-p4.py | 55 + t/t9833-errors.sh | 78 +++ 2 files changed, 133 insertions(+) create mode 100755 t/t9833-errors.sh -- 2.17.0.392.gdeb1a6e9b7
[PATCHv1 3/3] git-p4: auto-size the block
git-p4 originally would fetch changes in one query. On large repos this
could fail because of the limits that Perforce imposes on the number of
items returned and the number of queries in the database.
To fix this, git-p4 learned to query changes in blocks of 512 changes,
However, this can be very slow - if you have a few million changes,
with each chunk taking about a second, it can be an hour or so.
Although it's possible to tune this value manually with the
"--changes-block-size" option, it's far from obvious to ordinary users
that this is what needs doing.
This change alters the block size dynamically by looking for the
specific error messages returned from the Perforce server, and reducing
the block size if the error is seen, either to the limit reported by the
server, or to half the current block size.
That means we can start out with a very large block size, and then let
it automatically drop down to a value that works without error, while
still failing correctly if some other error occurs.
Signed-off-by: Luke Diamand
---
git-p4.py | 26 +-
t/t9818-git-p4-block.sh | 11 +++
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index b337562b39..6736f81b60 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -48,7 +48,8 @@ def __str__(self):
defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$'
# Grab changes in blocks of this many revisions, unless otherwise requested
-defaultBlockSize = 512
+# The code should now automatically reduce the block size if it is required
+defaultBlockSize = 1<<20
p4_access_checked = False
@@ -969,7 +970,8 @@ def p4ChangesForPaths(depotPaths, changeRange,
requestedBlockSize):
changes = set()
# Retrieve changes a block at a time, to prevent running
-# into a MaxResults/MaxScanRows error from the server.
+# into a MaxResults/MaxScanRows error from the server. If
+# we _do_ hit one of those errors, turn down the block size
while True:
cmd = ['changes']
@@ -983,10 +985,24 @@ def p4ChangesForPaths(depotPaths, changeRange,
requestedBlockSize):
for p in depotPaths:
cmd += ["%s...@%s" % (p, revisionRange)]
+# fetch the changes
+try:
+result = p4CmdList(cmd, errors_as_exceptions=True)
+except P4RequestSizeException as e:
+if not block_size:
+block_size = e.limit
+elif block_size > e.limit:
+block_size = e.limit
+else:
+block_size = max(2, block_size // 2)
+
+if verbose: print("block size error, retry with block size
{0}".format(block_size))
+continue
+except P4Exception as e:
+die('Error retrieving changes description
({0})'.format(e.p4ExitCode))
+
# Insert changes in chronological order
-for entry in reversed(p4CmdList(cmd)):
-if entry.has_key('p4ExitCode'):
-die('Error retrieving changes descriptions
({})'.format(entry['p4ExitCode']))
+for entry in reversed(result):
if not entry.has_key('change'):
continue
changes.add(int(entry['change']))
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 8840a183ac..e5ec9cdec8 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -129,6 +129,7 @@ test_expect_success 'Create a repo with multiple depot
paths' '
'
test_expect_success 'Clone repo with multiple depot paths' '
+ test_when_finished cleanup_git &&
(
cd "$git" &&
git p4 clone --changes-block-size=4 //depot/pathA@all
//depot/pathB@all \
@@ -138,6 +139,16 @@ test_expect_success 'Clone repo with multiple depot paths'
'
)
'
+test_expect_success 'Clone repo with self-sizing block size' '
+ test_when_finished cleanup_git &&
+ git p4 clone --changes-block-size=100 //depot@all
--destination="$git" &&
+ (
+ cd "$git" &&
+ git log --oneline > log &&
+ test_line_count \> 10 log
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
2.17.0.392.gdeb1a6e9b7
[PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
This change lays some groundwork for better handling of rowcount errors
from the server, where it fails to send us results because we requested
too many.
It adds an option to p4CmdList() to return errors as a Python exception.
The exceptions are derived from P4Exception (something went wrong),
P4ServerException (the server sent us an error code) and
P4RequestSizeException (we requested too many rows/results from the
server database).
This makes makes the code that handles the errors a bit easier.
The default behavior is unchanged; the new code is enabled with a flag.
Signed-off-by: Luke Diamand
---
git-p4.py | 44
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 66652f8280..d856078ec8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -566,10 +566,30 @@ def isModeExec(mode):
# otherwise False.
return mode[-3:] == "755"
+class P4Exception(Exception):
+""" Base class for exceptions from the p4 client """
+def __init__(self, exit_code):
+self.p4ExitCode = exit_code
+
+class P4ServerException(Exception):
+""" Base class for exceptions where we get some kind of marshalled up
result from the server """
+def __init__(self, exit_code, p4_result):
+super(P4ServerException, self).__init__(exit_code)
+self.p4_result = p4_result
+self.code = p4_result[0]['code']
+self.data = p4_result[0]['data']
+
+class P4RequestSizeException(P4ServerException):
+""" One of the maxresults or maxscanrows errors """
+def __init__(self, exit_code, p4_result, limit):
+super(P4RequestSizeException, self).__init__(exit_code, p4_result)
+self.limit = limit
+
def isModeExecChanged(src_mode, dst_mode):
return isModeExec(src_mode) != isModeExec(dst_mode)
-def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False):
+def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
+errors_as_exceptions=False):
if isinstance(cmd,basestring):
cmd = "-G " + cmd
@@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None,
skip_info=False):
pass
exitCode = p4.wait()
if exitCode != 0:
-entry = {}
-entry["p4ExitCode"] = exitCode
-result.append(entry)
+if errors_as_exceptions:
+if len(result) > 0:
+data = result[0].get('data')
+if data:
+m = re.search('Too many rows scanned \(over (\d+)\)', data)
+if not m:
+m = re.search('Request too large \(over (\d+)\)', data)
+
+if m:
+limit = int(m.group(1))
+raise P4RequestSizeException(exitCode, result, limit)
+
+raise P4ServerException(exitCode, result)
+else:
+raise P4Exception(exitCode)
+else:
+entry = {}
+entry["p4ExitCode"] = exitCode
+result.append(entry)
return result
--
2.17.0.392.gdeb1a6e9b7
[PATCHv1 2/3] git-p4: narrow the scope of exceptions caught when parsing an int
The current code traps all exceptions around some code which parses an integer, and then talks to Perforce. That can result in errors from Perforce being ignored. Change the code to only catch the integer conversion exceptions. Signed-off-by: Luke Diamand --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index d856078ec8..b337562b39 100755 --- a/git-p4.py +++ b/git-p4.py @@ -959,7 +959,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize): try: (changeStart, changeEnd) = p4ParseNumericChangeRange(parts) block_size = chooseBlockSize(requestedBlockSize) -except: +except ValueError: changeStart = parts[0][1:] changeEnd = parts[1] if requestedBlockSize: -- 2.17.0.392.gdeb1a6e9b7
[PATCHv1 0/3] git-p4: improvements to p4 "blocking"
This patchset improves the way that git-p4 sends requests in "blocks".
The problem comes about because the Perforce server limits the maximum
number of results it will send back (there are actually two different
limits).
This causes git-p4 failures if you ask for too much data.
In commit 96b2d54aee ("git-p4: use -m when running p4 changes",
2015-04-20), git-p4 learned to make requests in blocks, by default 512
in size.
The problem with this, however, is that it can be very slow on large
repositories - you might have millions of commits, although only a
handful actually relate to the code you are trying to clone. Each 512
block request takes a sizeable fraction of a second to complete.
There's a command-line option to increase the block size, but unless you
know about it, it won't be at all obvious that you could use this to
speed things up.
This change
~~~
The function p4CmdList() has been taught to raise an exception when it
gets an error, and in particular, to notice and report the two "too many
results" errors.
The code that does the blocking can now start out with a nice large
block size, and then reduce it if it sees an error.
Luke Diamand (3):
git-p4: raise exceptions from p4CmdList based on error from p4 server
git-p4: narrow the scope of exceptions caught when parsing an int
git-p4: auto-size the block
git-p4.py | 72 +++--
t/t9818-git-p4-block.sh | 11 +++
2 files changed, 73 insertions(+), 10 deletions(-)
--
2.17.0.392.gdeb1a6e9b7
Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
On 5 June 2018 at 10:54, Eric Sunshine wrote:
> On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:
>> This change lays some groundwork for better handling of rowcount errors
>> from the server, where it fails to send us results because we requested
>> too many.
>>
>> It adds an option to p4CmdList() to return errors as a Python exception.
>>
>> The exceptions are derived from P4Exception (something went wrong),
>> P4ServerException (the server sent us an error code) and
>> P4RequestSizeException (we requested too many rows/results from the
>> server database).
>>
>> This makes makes the code that handles the errors a bit easier.
>>
>> The default behavior is unchanged; the new code is enabled with a flag.
>>
>> Signed-off-by: Luke Diamand
>> ---
>> diff --git a/git-p4.py b/git-p4.py
>> @@ -566,10 +566,30 @@ def isModeExec(mode):
>> +class P4ServerException(Exception):
>> +""" Base class for exceptions where we get some kind of marshalled up
>> result from the server """
>> +def __init__(self, exit_code, p4_result):
>> +super(P4ServerException, self).__init__(exit_code)
>> +self.p4_result = p4_result
>> +self.code = p4_result[0]['code']
>> +self.data = p4_result[0]['data']
>
> The subsequent patches never seem to access any of these fields, so
> it's difficult to judge whether it's worthwhile having 'code' and
> 'data' bits split out like this.
These changes don't use it, but I thought that future changes might
need them, and perhaps when I put that code in I was thinking I might
need it myself.
>
>> @@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b',
>> cb=None, skip_info=False):
>> if exitCode != 0:
>> -entry = {}
>> -entry["p4ExitCode"] = exitCode
>> -result.append(entry)
>> +if errors_as_exceptions:
>> +if len(result) > 0:
>> +data = result[0].get('data')
>> +if data:
>> +m = re.search('Too many rows scanned \(over (\d+)\)',
>> data)
>> +if not m:
>> +m = re.search('Request too large \(over (\d+)\)',
>> data)
>
> Does 'p4' localize these error messages?
That's a good question.
The marshalled-up error from Perforce looks like this:
([{'generic': 35, 'code': 'error', 'data': "Too many rows scanned
(over 40); see 'p4 help maxscanrows'.\n", 'severity': 3}])
It turns out that Perforce open-sourced the P4 client in 2014 (I only
recently found this out) so we can actually look at the code now!
https://swarm.workshop.perforce.com/projects/perforce_software-p4
Clone it like this:
mkdir p4 &&
(cd p4 && git init && git config --add git-p4.branchList p4/2018-1:2018-1) &&
P4USER=guest P4PORT=workshop.perforce.com:1666 git p4 clone
--detect-branches --destination p4 //guest/perforce_software/p4@all
Here's the code:
// ErrorId graveyard: retired/deprecated ErrorIds.
ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,
E_FAILED, EV_ADMIN, 1 ), "Request too large (over %maxResults%); see
'p4 help maxresults'." } ;//NOTRANS
ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,
E_FAILED, EV_ADMIN, 1 ), "Too many rows scanned (over %maxScanRows%);
see 'p4 help maxscanrows'." } ;//NOTRANS
I don't think there's actually a way to make it return any language
other than English though. There's a P4LANGUAGE environment variable,
but it just says "this is for system integrators":
https://www.perforce.com/perforce/r15.2/manuals/cmdref/P4LANGUAGE.html
So I think probably the language is always English.
Luke
Re: [PATCHv1 1/1] git-p4: better error reporting when p4 fails
On 5 June 2018 at 11:49, Merland Romain wrote: > >> @@ -91,6 +93,13 @@ def p4_build_cmd(cmd): >> real_cmd = ' '.join(real_cmd) + ' ' + cmd >> else: >> real_cmd += cmd >> + >> +# now check that we can actually talk to the server >> +global p4_access_checked >> +if not p4_access_checked: >> +p4_access_checked = True >> +p4_check_access() >> + > > Just switch the 2 lines 'p4_access_checked = True' and 'p4_check_access()' > It seems to me more logical Like this: +p4_check_access() +p4_access_checked = True You need to set p4_access_checked first so that it doesn't go and try to check the p4 access before running "p4 login -s", which would then get stuck forever. > > Romain
Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
On 5 June 2018 at 20:41, Eric Sunshine wrote:
> On Tue, Jun 5, 2018 at 6:56 AM Luke Diamand wrote:
>> On 5 June 2018 at 10:54, Eric Sunshine wrote:
>> > On Tue, Jun 5, 2018 at 5:14 AM Luke Diamand wrote:
>> >> +m = re.search('Too many rows scanned \(over
>> >> (\d+)\)', data)
>> >> +if not m:
>> >> +m = re.search('Request too large \(over
>> >> (\d+)\)', data)
>> >
>> > Does 'p4' localize these error messages?
>>
>> That's a good question.
>>
>> It turns out that Perforce open-sourced the P4 client in 2014 (I only
>> recently found this out) so we can actually look at the code now!
>>
>> Here's the code:
>>
>> // ErrorId graveyard: retired/deprecated ErrorIds.
>
> Hmm, the "too many rows" error you're seeing is retired/deprecated(?).
There's some code elsewhere that suggests it's being kept alive:
// Retired ErrorIds. We need to keep these so that clients
// built with newer apis can commnunicate with older servers
// still sending these.
static ErrorId MaxResults; // DEPRECATED
static ErrorId MaxScanRows; // DEPRECATED
>
>> ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,
>> E_FAILED, EV_ADMIN, 1 ), "Request too large (over %maxResults%); see
>> 'p4 help maxresults'." } ;//NOTRANS
>> ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,
>> E_FAILED, EV_ADMIN, 1 ), "Too many rows scanned (over %maxScanRows%);
>> see 'p4 help maxscanrows'." } ;//NOTRANS
>>
>> I don't think there's actually a way to make it return any language
>> other than English though. [...]
>> So I think probably the language is always English.
>
> The "NOTRANS" annotation on the error messages is reassuring.
I'll check it works OK on Windows; charset translation might cause a problem.
Re: [PATCHv1 1/3] git-p4: raise exceptions from p4CmdList based on error from p4 server
>>
>>> ErrorId MsgDb::MaxResults = { ErrorOf( ES_DB, 32,
>>> E_FAILED, EV_ADMIN, 1 ), "Request too large (over %maxResults%); see
>>> 'p4 help maxresults'." } ;//NOTRANS
>>> ErrorId MsgDb::MaxScanRows = { ErrorOf( ES_DB, 61,
>>> E_FAILED, EV_ADMIN, 1 ), "Too many rows scanned (over %maxScanRows%);
>>> see 'p4 help maxscanrows'." } ;//NOTRANS
>>>
>>> I don't think there's actually a way to make it return any language
>>> other than English though. [...]
>>> So I think probably the language is always English.
>>
>> The "NOTRANS" annotation on the error messages is reassuring.
>
> I'll check it works OK on Windows; charset translation might cause a problem.
Works OK on Windows with 2017.2 client/server.
[PATCHv2 2/6] git-p4: add option to disable syncing of p4/master with p4
Add an option to the git-p4 submit command to disable syncing
with Perforce.
This is useful for the case where a git-p4 mirror has been setup
on a server somewhere, running from (e.g.) cron, and developers
then clone from this. Having the local cloned copy also sync
from Perforce just isn't useful.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 8
git-p4.py| 31 ---
2 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 3d83842e47..f0de3b891b 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -369,6 +369,11 @@ These options can be used to modify 'git p4 submit'
behavior.
Disable the automatic rebase after all commits have been successfully
submitted. Can also be set with git-p4.disableRebase.
+--disable-p4sync::
+Disable the automatic sync of p4/master from Perforce after commits have
+been submitted. Implies --disable-rebase. Can also be set with
+git-p4.disableP4Sync. Sync with origin/master still goes ahead if possible.
+
Rebase options
~~
These options can be used to modify 'git p4 rebase' behavior.
@@ -693,6 +698,9 @@ git-p4.conflict::
git-p4.disableRebase::
Do not rebase the tree against p4/master following a submit.
+git-p4.disableP4Sync::
+Do not sync p4/master with Perforce following a submit. Implies
git-p4.disableRebase.
+
IMPLEMENTATION DETAILS
--
* Changesets from p4 are imported using Git fast-import.
diff --git a/git-p4.py b/git-p4.py
index 5ab9421af8..b61f47cc61 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1368,7 +1368,9 @@ def __init__(self):
help="submit only the specified
commit(s), one commit or xxx..xxx"),
optparse.make_option("--disable-rebase",
dest="disable_rebase", action="store_true",
help="Disable rebase after submit is
completed. Can be useful if you "
- "work from a local git branch that is not
master")
+ "work from a local git branch that is not
master"),
+optparse.make_option("--disable-p4sync",
dest="disable_p4sync", action="store_true",
+ help="Skip Perforce sync of p4/master
after submit or shelve"),
]
self.description = "Submit changes from git to the perforce depot."
self.usage += " [name of git branch to submit into perforce depot]"
@@ -1380,6 +1382,7 @@ def __init__(self):
self.update_shelve = list()
self.commit = ""
self.disable_rebase = gitConfigBool("git-p4.disableRebase")
+self.disable_p4sync = gitConfigBool("git-p4.disableP4Sync")
self.prepare_p4_only = False
self.conflict_behavior = None
self.isWindows = (platform.system() == "Windows")
@@ -2240,11 +2243,14 @@ def run(self, args):
sync = P4Sync()
if self.branch:
sync.branch = self.branch
-sync.run([])
+if self.disable_p4sync:
+sync.sync_origin_only()
+else:
+sync.run([])
-if self.disable_rebase is False:
-rebase = P4Rebase()
-rebase.rebase()
+if not self.disable_rebase:
+rebase = P4Rebase()
+rebase.rebase()
else:
if len(applied) == 0:
@@ -3324,6 +3330,14 @@ def importChanges(self, changes, shelved=False,
origin_revision=0):
print self.gitError.read()
sys.exit(1)
+def sync_origin_only(self):
+if self.syncWithOrigin:
+self.hasOrigin = originP4BranchesExist()
+if self.hasOrigin:
+if not self.silent:
+print 'Syncing with origin first, using "git fetch origin"'
+system("git fetch origin")
+
def importHeadRevision(self, revision):
print "Doing initial import of %s from revision %s into %s" % ('
'.join(self.depotPaths), revision, self.branch)
@@ -3402,12 +3416,7 @@ def run(self, args):
else:
self.refPrefix = "refs/heads/p4/"
-if self.syncWithOrigin:
-self.hasOrigin = originP4BranchesExist()
-if self.hasOrigin:
-if not self.silent:
-print 'Syncing with origin first, using "git fetch origin"'
-system("git fetch origin")
+self.sync_origin_only()
branch_arg_given = bool(self.branch)
if len(self.branch) == 0:
--
2.17.0.392.gdeb1a6e9b7
[PATCHv2 1/6] git-p4: disable-rebase: allow setting this via configuration
This just lets you set the --disable-rebase option with the
git configuration options git-p4.disableRebase. If you're
using this option, you probably want to set it all the time
for a given repo.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 5 -
git-p4.py| 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index e8452528fc..3d83842e47 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -367,7 +367,7 @@ These options can be used to modify 'git p4 submit'
behavior.
--disable-rebase::
Disable the automatic rebase after all commits have been successfully
-submitted.
+submitted. Can also be set with git-p4.disableRebase.
Rebase options
~~
@@ -690,6 +690,9 @@ git-p4.conflict::
Specify submit behavior when a conflict with p4 is found, as per
--conflict. The default behavior is 'ask'.
+git-p4.disableRebase::
+Do not rebase the tree against p4/master following a submit.
+
IMPLEMENTATION DETAILS
--
* Changesets from p4 are imported using Git fast-import.
diff --git a/git-p4.py b/git-p4.py
index c4581d20a6..5ab9421af8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1379,7 +1379,7 @@ def __init__(self):
self.shelve = False
self.update_shelve = list()
self.commit = ""
-self.disable_rebase = False
+self.disable_rebase = gitConfigBool("git-p4.disableRebase")
self.prepare_p4_only = False
self.conflict_behavior = None
self.isWindows = (platform.system() == "Windows")
--
2.17.0.392.gdeb1a6e9b7
[PATCHv2 5/6] git-p4: narrow the scope of exceptions caught when parsing an int
The current code traps all exceptions around some code which parses an integer, and then talks to Perforce. That can result in errors from Perforce being ignored. Change the code to only catch the integer conversion exceptions. Signed-off-by: Luke Diamand --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index f4b5deeb83..5f59feb5bb 100755 --- a/git-p4.py +++ b/git-p4.py @@ -959,7 +959,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize): try: (changeStart, changeEnd) = p4ParseNumericChangeRange(parts) block_size = chooseBlockSize(requestedBlockSize) -except: +except ValueError: changeStart = parts[0][1:] changeEnd = parts[1] if requestedBlockSize: -- 2.17.0.392.gdeb1a6e9b7
[PATCHv2 6/6] git-p4: auto-size the block
git-p4 originally would fetch changes in one query. On large repos this
could fail because of the limits that Perforce imposes on the number of
items returned and the number of queries in the database.
To fix this, git-p4 learned to query changes in blocks of 512 changes,
However, this can be very slow - if you have a few million changes,
with each chunk taking about a second, it can be an hour or so.
Although it's possible to tune this value manually with the
"--changes-block-size" option, it's far from obvious to ordinary users
that this is what needs doing.
This change alters the block size dynamically by looking for the
specific error messages returned from the Perforce server, and reducing
the block size if the error is seen, either to the limit reported by the
server, or to half the current block size.
That means we can start out with a very large block size, and then let
it automatically drop down to a value that works without error, while
still failing correctly if some other error occurs.
Signed-off-by: Luke Diamand
---
git-p4.py | 27 +--
t/t9818-git-p4-block.sh | 8
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 5f59feb5bb..0354d4df5c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -47,8 +47,8 @@ def __str__(self):
# Only labels/tags matching this will be imported/exported
defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$'
-# Grab changes in blocks of this many revisions, unless otherwise requested
-defaultBlockSize = 512
+# The block size is reduced automatically if required
+defaultBlockSize = 1<<20
p4_access_checked = False
@@ -969,7 +969,8 @@ def p4ChangesForPaths(depotPaths, changeRange,
requestedBlockSize):
changes = set()
# Retrieve changes a block at a time, to prevent running
-# into a MaxResults/MaxScanRows error from the server.
+# into a MaxResults/MaxScanRows error from the server. If
+# we _do_ hit one of those errors, turn down the block size
while True:
cmd = ['changes']
@@ -983,10 +984,24 @@ def p4ChangesForPaths(depotPaths, changeRange,
requestedBlockSize):
for p in depotPaths:
cmd += ["%s...@%s" % (p, revisionRange)]
+# fetch the changes
+try:
+result = p4CmdList(cmd, errors_as_exceptions=True)
+except P4RequestSizeException as e:
+if not block_size:
+block_size = e.limit
+elif block_size > e.limit:
+block_size = e.limit
+else:
+block_size = max(2, block_size // 2)
+
+if verbose: print("block size error, retrying with block size
{0}".format(block_size))
+continue
+except P4Exception as e:
+die('Error retrieving changes description
({0})'.format(e.p4ExitCode))
+
# Insert changes in chronological order
-for entry in reversed(p4CmdList(cmd)):
-if entry.has_key('p4ExitCode'):
-die('Error retrieving changes descriptions
({})'.format(entry['p4ExitCode']))
+for entry in reversed(result):
if not entry.has_key('change'):
continue
changes.add(int(entry['change']))
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 8840a183ac..ce7cb22ad3 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -129,6 +129,7 @@ test_expect_success 'Create a repo with multiple depot
paths' '
'
test_expect_success 'Clone repo with multiple depot paths' '
+ test_when_finished cleanup_git &&
(
cd "$git" &&
git p4 clone --changes-block-size=4 //depot/pathA@all
//depot/pathB@all \
@@ -138,6 +139,13 @@ test_expect_success 'Clone repo with multiple depot paths'
'
)
'
+test_expect_success 'Clone repo with self-sizing block size' '
+ test_when_finished cleanup_git &&
+ git p4 clone --changes-block-size=100 //depot@all
--destination="$git" &&
+ git -C "$git" log --oneline >log &&
+ test_line_count \> 10 log
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
2.17.0.392.gdeb1a6e9b7
[PATCHv2 3/6] git-p4: better error reporting when p4 fails
Currently when p4 fails to run, git-p4 just crashes with an obscure
error message.
For example, if the P4 ticket has expired, you get:
Error: Cannot locate perforce checkout of in client view
This change checks whether git-p4 can talk to the Perforce server when
the first P4 operation is attempted, and tries to print a meaningful
error message if it fails.
Signed-off-by: Luke Diamand
---
git-p4.py | 55 +
t/t9833-errors.sh | 78 +++
2 files changed, 133 insertions(+)
create mode 100755 t/t9833-errors.sh
diff --git a/git-p4.py b/git-p4.py
index b61f47cc61..3de12a4a0a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -50,6 +50,8 @@ def __str__(self):
# Grab changes in blocks of this many revisions, unless otherwise requested
defaultBlockSize = 512
+p4_access_checked = False
+
def p4_build_cmd(cmd):
"""Build a suitable p4 command line.
@@ -91,6 +93,13 @@ def p4_build_cmd(cmd):
real_cmd = ' '.join(real_cmd) + ' ' + cmd
else:
real_cmd += cmd
+
+# now check that we can actually talk to the server
+global p4_access_checked
+if not p4_access_checked:
+p4_access_checked = True# suppress access checks in
p4_check_access itself
+p4_check_access()
+
return real_cmd
def git_dir(path):
@@ -264,6 +273,52 @@ def p4_system(cmd):
if retcode:
raise CalledProcessError(retcode, real_cmd)
+def die_bad_access(s):
+die("failure accessing depot: {0}".format(s.rstrip()))
+
+def p4_check_access(min_expiration=1):
+""" Check if we can access Perforce - account still logged in
+"""
+results = p4CmdList(["login", "-s"])
+
+if len(results) == 0:
+# should never get here: always get either some results, or a
p4ExitCode
+assert("could not parse response from perforce")
+
+result = results[0]
+
+if 'p4ExitCode' in result:
+# p4 returned non-zero status, e.g. P4PORT invalid, or p4 not in path
+die_bad_access("could not run p4")
+
+code = result.get("code")
+if not code:
+# we get here if we couldn't connect and there was nothing to unmarshal
+die_bad_access("could not connect")
+
+elif code == "stat":
+expiry = result.get("TicketExpiration")
+if expiry:
+expiry = int(expiry)
+if expiry > min_expiration:
+# ok to carry on
+return
+else:
+die_bad_access("perforce ticket expires in {0}
seconds".format(expiry))
+
+else:
+# account without a timeout - all ok
+return
+
+elif code == "error":
+data = result.get("data")
+if data:
+die_bad_access("p4 error: {0}".format(data))
+else:
+die_bad_access("unknown error")
+else:
+die_bad_access("unknown error code {0}".format(code))
+
_p4_version_string = None
def p4_version_string():
"""Read the version string, showing just the last line, which
diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
new file mode 100755
index 00..9ba892de7a
--- /dev/null
+++ b/t/t9833-errors.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='git p4 errors'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'add p4 files' '
+ (
+ cd "$cli" &&
+ echo file1 >file1 &&
+ p4 add file1 &&
+ p4 submit -d "file1"
+ )
+'
+
+# after this test, the default user requires a password
+test_expect_success 'error handling' '
+ git p4 clone --dest="$git" //depot@all &&
+ (
+ cd "$git" &&
+ P4PORT=: test_must_fail git p4 submit 2>errmsg
+ ) &&
+ p4 passwd -P newpassword &&
+ (
+ P4PASSWD=badpassword test_must_fail git p4 clone //depot/foo
2>errmsg &&
+ grep -q "failure accessing depot.*P4PASSWD" errmsg
+ )
+'
+
+test_expect_success 'ticket logged out' '
+ P4TICKETS="$cli/tickets" &&
+ echo "newpassword" | p4 login &&
+ (
+ cd "$git" &&
+ test_commit "ticket-auth-check" &&
+ p4 logout &&
+ test_must_fail git p4 submit 2>errmsg &&
+ grep -q "failure accessing depot" errmsg
+ )
+'
+
+test_expect_success 'create grou
[PATCHv2 0/6] git-p4: some small fixes updated
This is an updated version of the set of changes I posted recently, following comments on the list: disable automatic sync after git-p4 submit: https://marc.info/?l=git&m=152818734814838&w=2 better handling of being logged out by Perforce: https://marc.info/?l=git&m=152818893815326&w=2 adapt the block size automatically on git-p4 submit: https://marc.info/?l=git&m=152819004315688&w=2 - Spelling corrections (Eric) - Improved comments (Eric) - Exception class hierarchy fix (Merland) - test simplification (Eric) Luke Diamand (6): git-p4: disable-rebase: allow setting this via configuration git-p4: add option to disable syncing of p4/master with p4 git-p4: better error reporting when p4 fails git-p4: raise exceptions from p4CmdList based on error from p4 server git-p4: narrow the scope of exceptions caught when parsing an int git-p4: auto-size the block Documentation/git-p4.txt | 13 +++- git-p4.py| 161 +-- t/t9818-git-p4-block.sh | 8 ++ t/t9833-errors.sh| 78 +++ 4 files changed, 236 insertions(+), 24 deletions(-) create mode 100755 t/t9833-errors.sh -- 2.17.0.392.gdeb1a6e9b7
[PATCHv2 4/6] git-p4: raise exceptions from p4CmdList based on error from p4 server
This change lays some groundwork for better handling of rowcount errors
from the server, where it fails to send us results because we requested
too many.
It adds an option to p4CmdList() to return errors as a Python exception.
The exceptions are derived from P4Exception (something went wrong),
P4ServerException (the server sent us an error code) and
P4RequestSizeException (we requested too many rows/results from the
server database).
This makes the code that handles the errors a bit easier.
The default behavior is unchanged; the new code is enabled with a flag.
Signed-off-by: Luke Diamand
---
git-p4.py | 44
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 3de12a4a0a..f4b5deeb83 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -566,10 +566,30 @@ def isModeExec(mode):
# otherwise False.
return mode[-3:] == "755"
+class P4Exception(Exception):
+""" Base class for exceptions from the p4 client """
+def __init__(self, exit_code):
+self.p4ExitCode = exit_code
+
+class P4ServerException(P4Exception):
+""" Base class for exceptions where we get some kind of marshalled up
result from the server """
+def __init__(self, exit_code, p4_result):
+super(P4ServerException, self).__init__(exit_code)
+self.p4_result = p4_result
+self.code = p4_result[0]['code']
+self.data = p4_result[0]['data']
+
+class P4RequestSizeException(P4ServerException):
+""" One of the maxresults or maxscanrows errors """
+def __init__(self, exit_code, p4_result, limit):
+super(P4RequestSizeException, self).__init__(exit_code, p4_result)
+self.limit = limit
+
def isModeExecChanged(src_mode, dst_mode):
return isModeExec(src_mode) != isModeExec(dst_mode)
-def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False):
+def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
+errors_as_exceptions=False):
if isinstance(cmd,basestring):
cmd = "-G " + cmd
@@ -616,9 +636,25 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None,
skip_info=False):
pass
exitCode = p4.wait()
if exitCode != 0:
-entry = {}
-entry["p4ExitCode"] = exitCode
-result.append(entry)
+if errors_as_exceptions:
+if len(result) > 0:
+data = result[0].get('data')
+if data:
+m = re.search('Too many rows scanned \(over (\d+)\)', data)
+if not m:
+m = re.search('Request too large \(over (\d+)\)', data)
+
+if m:
+limit = int(m.group(1))
+raise P4RequestSizeException(exitCode, result, limit)
+
+raise P4ServerException(exitCode, result)
+else:
+raise P4Exception(exitCode)
+else:
+entry = {}
+entry["p4ExitCode"] = exitCode
+result.append(entry)
return result
--
2.17.0.392.gdeb1a6e9b7
Re: [PATCHv2 0/6] git-p4: some small fixes updated
On 12 June 2018 at 18:10, Junio C Hamano wrote:
> Luke Diamand writes:
>
>> This is an updated version of the set of changes I posted recently,
>> following comments on the list:
>>
>> disable automatic sync after git-p4 submit:
>> https://marc.info/?l=git&m=152818734814838&w=2
>>
>> better handling of being logged out by Perforce:
>>https://marc.info/?l=git&m=152818893815326&w=2
>>
>> adapt the block size automatically on git-p4 submit:
>>https://marc.info/?l=git&m=152819004315688&w=2
>>
>> - Spelling corrections (Eric)
>> - Improved comments (Eric)
>> - Exception class hierarchy fix (Merland)
>> - test simplification (Eric)
>>
>
> That reminds me of one thing.
>
> This 6-patch series depends on the rm/p4-submit-with-commit-option
> that came without and still waiting for a sign-off by the original
> author. Also I do not think the original patch reached the public
> list, so I'm attaching the patch to make sure people know which
> patch I am talking about.
>
> Romain, can we get your sign-off on the patch you sent earlier?
Wasn't it already sent in this message:
https://marc.info/?l=git&m=152783923418317&w=2
Luke
>
> Thanks.
>
> -- >8 --
> From: Romain Merland
> Date: Wed, 9 May 2018 17:32:12 +0200
> Subject: [PATCH] git-p4: add options --commit and --disable-rebase
>
> On a daily work with multiple local git branches, the usual way to
> submit only a specified commit was to cherry-pick the commit on
> master then run git-p4 submit. It can be very annoying to switch
> between local branches and master, only to submit one commit. The
> proposed new way is to select directly the commit you want to
> submit.
>
> Add option --commit to command 'git-p4 submit' in order to submit
> only specified commit(s) in p4.
>
> On a daily work developping software with big compilation time, one
> may not want to rebase on his local git tree, in order to avoid long
> recompilation.
>
> Add option --disable-rebase to command 'git-p4 submit' in order to
> disable rebase after submission.
>
> Reviewed-by: Luke Diamand
> Signed-off-by: Junio C Hamano
> ---
> Documentation/git-p4.txt | 14 ++
> git-p4.py| 29 +++--
> t/t9807-git-p4-submit.sh | 40
> 3 files changed, 77 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index d8c8f11c9f..88d109debb 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -149,6 +149,12 @@ To specify a branch other than the current one, use:
> $ git p4 submit topicbranch
>
>
> +To specify a single commit or a range of commits, use:
> +
> +$ git p4 submit --commit
> +$ git p4 submit --commit
> +
> +
> The upstream reference is generally 'refs/remotes/p4/master', but can
> be overridden using the `--origin=` command-line option.
>
> @@ -330,6 +336,14 @@ These options can be used to modify 'git p4 submit'
> behavior.
> p4/master. See the "Sync options" section above for more
> information.
>
> +--commit |::
> +Submit only the specified commit or range of commits, instead of the full
> +list of changes that are in the current Git branch.
> +
> +--disable-rebase::
> +Disable the automatic rebase after all commits have been successfully
> +submitted.
> +
> Rebase options
> ~~
> These options can be used to modify 'git p4 rebase' behavior.
> diff --git a/git-p4.py b/git-p4.py
> index 7bb9cadc69..f4a6f3b4c3 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1352,7 +1352,12 @@ def __init__(self):
> optparse.make_option("--update-shelve",
> dest="update_shelve", action="append", type="int",
> metavar="CHANGELIST",
> help="update an existing shelved
> changelist, implies --shelve, "
> - "repeat in-order for multiple
> shelved changelists")
> + "repeat in-order for multiple
> shelved changelists"),
> +optparse.make_option("--commit", dest="commit",
> metavar="COMMIT",
> + help="submit only the specified
> commit(s), one commit or xxx..xxx"),
> +optparse.make_opt
Re: [PATCHv2 0/6] git-p4: some small fixes updated
On 12 June 2018 at 22:35, Junio C Hamano wrote: > Luke Diamand writes: > >> On 12 June 2018 at 18:10, Junio C Hamano wrote: >>> Luke Diamand writes: >>> >>>> This is an updated version of the set of changes I posted recently, >>>> following comments on the list: >>>> >>>> disable automatic sync after git-p4 submit: >>>> https://marc.info/?l=git&m=152818734814838&w=2 >>>> >>>> better handling of being logged out by Perforce: >>>>https://marc.info/?l=git&m=152818893815326&w=2 >>>> >>>> adapt the block size automatically on git-p4 submit: >>>>https://marc.info/?l=git&m=152819004315688&w=2 >>>> >>>> - Spelling corrections (Eric) >>>> - Improved comments (Eric) >>>> - Exception class hierarchy fix (Merland) >>>> - test simplification (Eric) >>>> >>> >>> That reminds me of one thing. >>> >>> This 6-patch series depends on the rm/p4-submit-with-commit-option >>> that came without and still waiting for a sign-off by the original >>> author. Also I do not think the original patch reached the public >>> list, so I'm attaching the patch to make sure people know which >>> patch I am talking about. >>> >>> Romain, can we get your sign-off on the patch you sent earlier? >> >> Wasn't it already sent in this message: >> >> https://marc.info/?l=git&m=152783923418317&w=2 >> >> Luke > > > Thanks for a pointer. Will replace and requeue. Thanks. While on the subject of git-p4, I thought I should mention that I've been looking at getting git-p4 to work with Python3. I've got some low risk easy (mostly automated) changes which get it to the point where it compiles. After that I have to figure out how to fix the byte-vs-string unicode problem that Python3 brings. Having been playing around with it, I think it should be possible to make the same git-p4.py script work with 2.7, 3.6 and probably 2.6, although I'm still some way from making this last bit work. Luke
Re: [PATCHv2 0/6] git-p4: some small fixes updated
On 12 June 2018 at 22:53, Eric Sunshine wrote: > On Tue, Jun 12, 2018 at 5:49 PM, Luke Diamand wrote: >> Thanks. While on the subject of git-p4, I thought I should mention >> that I've been looking at getting git-p4 to work with Python3. >> >> I've got some low risk easy (mostly automated) changes which get it to >> the point where it compiles. After that I have to figure out how to >> fix the byte-vs-string unicode problem that Python3 brings. [...] > > See how reposurgeon handles the problem[1]. It defines polystr() and > polybytes() functions which coerce strings and byte sequences as > needed. It's not pretty, but it works. > > [1]: https://gitlab.com/esr/reposurgeon/blob/master/reposurgeon#L100 Thanks, that's got some useful tips!
Re: [PATCHv3 0/1] git-p4 unshelve
On 15 May 2018 at 11:01, Luke Diamand wrote: > This is a git-p4 unshelve command which detects when extra > changes are going to be included, and refuses to unhshelve. > > As in my earlier unshelve change, this uses git fast-import > to do the actual delta generation, but now checks to see > if the files unshelved are based on the same revisions that > fast-import will be using, using the revision numbers in > the "p4 describe -S" command output. If they're different, > it refuses to unshelve. > > That makes it safe to use, rather than potentially generating > deltas that contain bits from other changes. Having been using this, when it works it's great, and it's nice that it errors out rather than creating a garbled patch. But it would be really useful if it could somehow automatically build some kind of diff-base for fast-import to use. Doing so is probably not that hard given that it already knows the versions that it wants for the error reports, but it's not completely trivial either. I think you'd need to get the list of base revisions you want, then hijack the P4Sync class to generate a commit via fast-import, then feed that to the next stage as the base commit. I think it would make sense to see if anyone other than me actually cares! Luke
Re: Re :Re: [PATCHv3 0/1] git-p4 unshelve
On 16 June 2018 at 10:58, merlo...@yahoo.fr wrote: > Yes Luke, my colleagues and I, we care ! Our repository is p4 (choice of the > high management, sigh...). Some of us are working natively on p4, but others > like me are working on git through git-p4. We often want to share pieces of > codes to check compilation on misc platforms/configs, and shelve/unshelve > mechanism is commonly used between nativ p4 users. > For git-p4 users we have a temporary hack to unshelve, but it often fails to > apply the whole p4 diff, and we end up finishing stuff by hand, checking > line by line, sigh... Without speaking about diffs that are accross several > local workspaces. > Hopefully it is on small shelved p4 change lists for the moment, but we > cannot deploy on a larger scale. > Please continue your hard work on unshelve stuff. > > For your last remark, the members of our team often need to work on non > synchro revisions, but still need to share via shelve/unshelve, and I am > sure we will have conflits as you describe, leading unshelve to fail. > Automatic fast import would save us the need to stop our current work, sync > with p4 and launch a 1h compilation, before we could unshelve... So yes we > need it ! OK, I'll give it a go, in my copious free time :-) Luke
[PATCH 2/6] git-p4: python3: replace dict.has_key(k) with "k in dict"
Python3 does not have the dict.has_key() function, so replace all
such calls with "k in dict". This will still work with python2.6
and python2.7.
Converted using 2to3 (plus some hand-editing)
Signed-off-by: Luke Diamand
---
git-p4.py | 78 +++
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 51e9e64a73..6fcad35104 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -767,7 +767,7 @@ def gitDeleteRef(ref):
_gitConfig = {}
def gitConfig(key, typeSpecifier=None):
-if not _gitConfig.has_key(key):
+if key not in _gitConfig:
cmd = [ "git", "config" ]
if typeSpecifier:
cmd += [ typeSpecifier ]
@@ -781,12 +781,12 @@ def gitConfigBool(key):
variable is set to true, and False if set to false or not present
in the config."""
-if not _gitConfig.has_key(key):
+if key not in _gitConfig:
_gitConfig[key] = gitConfig(key, '--bool') == "true"
return _gitConfig[key]
def gitConfigInt(key):
-if not _gitConfig.has_key(key):
+if key not in _gitConfig:
cmd = [ "git", "config", "--int", key ]
s = read_pipe(cmd, ignore_error=True)
v = s.strip()
@@ -797,7 +797,7 @@ def gitConfigInt(key):
return _gitConfig[key]
def gitConfigList(key):
-if not _gitConfig.has_key(key):
+if key not in _gitConfig:
s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
_gitConfig[key] = s.strip().splitlines()
if _gitConfig[key] == ['']:
@@ -855,7 +855,7 @@ def findUpstreamBranchPoint(head = "HEAD"):
tip = branches[branch]
log = extractLogMessageFromGitCommit(tip)
settings = extractSettingsGitLog(log)
-if settings.has_key("depot-paths"):
+if "depot-paths" in settings:
paths = ",".join(settings["depot-paths"])
branchByDepotPath[paths] = "remotes/p4/" + branch
@@ -865,9 +865,9 @@ def findUpstreamBranchPoint(head = "HEAD"):
commit = head + "~%s" % parent
log = extractLogMessageFromGitCommit(commit)
settings = extractSettingsGitLog(log)
-if settings.has_key("depot-paths"):
+if "depot-paths" in settings:
paths = ",".join(settings["depot-paths"])
-if branchByDepotPath.has_key(paths):
+if paths in branchByDepotPath:
return [branchByDepotPath[paths], settings]
parent = parent + 1
@@ -891,8 +891,8 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix =
"refs/remotes/p4/", silent
originHead = line
original =
extractSettingsGitLog(extractLogMessageFromGitCommit(originHead))
-if (not original.has_key('depot-paths')
-or not original.has_key('change')):
+if ('depot-paths' not in original
+or 'change' not in original):
continue
update = False
@@ -902,7 +902,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix =
"refs/remotes/p4/", silent
update = True
else:
settings =
extractSettingsGitLog(extractLogMessageFromGitCommit(remoteHead))
-if settings.has_key('change') > 0:
+if 'change' in settings:
if settings['depot-paths'] == original['depot-paths']:
originP4Change = int(original['change'])
p4Change = int(settings['change'])
@@ -1002,7 +1002,7 @@ def p4ChangesForPaths(depotPaths, changeRange,
requestedBlockSize):
# Insert changes in chronological order
for entry in reversed(result):
-if not entry.has_key('change'):
+if 'change' not in entry:
continue
changes.add(int(entry['change']))
@@ -1312,7 +1312,7 @@ def p4UserId(self):
results = p4CmdList("user -o")
for r in results:
-if r.has_key('User'):
+if 'User' in r:
self.myP4UserId = r['User']
return r['User']
die("Could not find your p4 user id")
@@ -1336,7 +1336,7 @@ def getUserMapFromPerforceServer(self):
self.emails = {}
for output in p4CmdList("users"):
-if not output.has_key("User"):
+if "User" not in output:
continue
self.users[output["User"]] = output["FullName"] + " <" +
output["Email"] + "&g
[PATCH 4/6] git-p4: python3: basestring workaround
In Python3, basestring no longer exists, so use this workaround. Signed-off-by: Luke Diamand --- git-p4.py | 16 1 file changed, 16 insertions(+) diff --git a/git-p4.py b/git-p4.py index 67865d14aa..f127ebce27 100755 --- a/git-p4.py +++ b/git-p4.py @@ -27,6 +27,22 @@ import ctypes import errno +# support basestring in python3 +try: +unicode = unicode +except NameError: +# 'unicode' is undefined, must be Python 3 +str = str +unicode = str +bytes = bytes +basestring = (str,bytes) +else: +# 'unicode' exists, must be Python 2 +str = str +unicode = unicode +bytes = str +basestring = basestring + try: from subprocess import CalledProcessError except ImportError: -- 2.18.0.rc1.242.g61856ae69a
[PATCH 1/6] git-p4: python3: replace <> with !=
The <> string inequality operator (which doesn't seem to be even
documented) no longer exists in python3. Replace with !=.
This still works with python2.
Signed-off-by: Luke Diamand
---
git-p4.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 0354d4df5c..51e9e64a73 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3590,7 +3590,7 @@ def run(self, args):
prev_list = prev.split("/")
cur_list = cur.split("/")
for i in range(0, min(len(cur_list),
len(prev_list))):
-if cur_list[i] <> prev_list[i]:
+if cur_list[i] != prev_list[i]:
i = i - 1
break
--
2.18.0.rc1.242.g61856ae69a
[PATCH 0/6] git-p4: small step towards Python3 support
This patchset is a first small step towards Python3 support for git-p4.py. These are all the nice easy changes which can almost be done automatically using 2to3. After these changes, it compiles using Python3, but fails to run. That's because of the bytes vs string change in Python3. Fixing that is quite a bit harder (but not impossible). I have some further changes to address this, but they are quite a bit more invasive, and not actually working yet. It's based very loosely on the "polystr()" suggestion from Eric on this list. It still works fine with Python2.7 and Python2.6. Luke Diamand (6): git-p4: python3: replace <> with != git-p4: python3: replace dict.has_key(k) with "k in dict" git-p4: python3: remove backticks git-p4: python3: basestring workaround git-p4: python3: use print() function git-p4: python3: fix octal constants git-p4.py | 348 -- 1 file changed, 182 insertions(+), 166 deletions(-) -- 2.18.0.rc1.242.g61856ae69a
[PATCH 6/6] git-p4: python3: fix octal constants
See PEP3127. Works fine with python2 as well. Signed-off-by: Luke Diamand --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index 714e442d7c..b449db1cc9 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1841,7 +1841,7 @@ def applyCommit(self, id): filesToDelete.remove(path) dst_mode = int(diff['dst_mode'], 8) -if dst_mode == 012: +if dst_mode == 0o12: symlinks.add(path) elif modifier == "D": -- 2.18.0.rc1.242.g61856ae69a
[PATCH 3/6] git-p4: python3: remove backticks
Backticks around a variable are a deprecated alias for repr().
This has been removed in python3, so just use the string
representation instead, which is equivalent.
Signed-off-by: Luke Diamand
---
git-p4.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 6fcad35104..67865d14aa 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3089,7 +3089,7 @@ def getLabels(self):
l = p4CmdList(["labels"] + ["%s..." % p for p in self.depotPaths])
if len(l) > 0 and not self.silent:
-print "Finding files belonging to labels in %s" % `self.depotPaths`
+print("Finding files belonging to labels in %s" % self.depotPaths)
for output in l:
label = output["label"]
--
2.18.0.rc1.242.g61856ae69a
[PATCH 5/6] git-p4: python3: use print() function
Replace calls to print ... with the function form, print(...), to
allow use with python3 as well as python2.x.
Converted using 2to3 (and some hand-editing).
Signed-off-by: Luke Diamand
---
git-p4.py | 248 +++---
1 file changed, 124 insertions(+), 124 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index f127ebce27..714e442d7c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -892,7 +892,7 @@ def findUpstreamBranchPoint(head = "HEAD"):
def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/",
silent=True):
if not silent:
-print ("Creating/updating branch(es) in %s based on origin branch(es)"
+print("Creating/updating branch(es) in %s based on origin branch(es)"
% localRefPrefix)
originPrefix = "origin/p4/"
@@ -914,7 +914,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix =
"refs/remotes/p4/", silent
update = False
if not gitBranchExists(remoteHead):
if verbose:
-print "creating %s" % remoteHead
+print("creating %s" % remoteHead)
update = True
else:
settings =
extractSettingsGitLog(extractLogMessageFromGitCommit(remoteHead))
@@ -923,13 +923,13 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix =
"refs/remotes/p4/", silent
originP4Change = int(original['change'])
p4Change = int(settings['change'])
if originP4Change > p4Change:
-print ("%s (%s) is newer than %s (%s). "
+print("%s (%s) is newer than %s (%s). "
"Updating p4 branch from origin."
% (originHead, originP4Change,
remoteHead, p4Change))
update = True
else:
-print ("Ignoring: %s was imported from %s while "
+print("Ignoring: %s was imported from %s while "
"%s was imported from %s"
% (originHead, ','.join(original['depot-paths']),
remoteHead, ','.join(settings['depot-paths'])))
@@ -1397,9 +1397,9 @@ def __init__(self):
def run(self, args):
j = 0
for output in p4CmdList(args):
-print 'Element: %d' % j
+print('Element: %d' % j)
j += 1
-print output
+print(output)
return True
class P4RollBack(Command):
@@ -1440,14 +1440,14 @@ def run(self, args):
if len(p4Cmd("changes -m 1 " + ' '.join (['%s...@%s' % (p,
maxChange)
for p in
depotPaths]))) == 0:
-print "Branch %s did not exist at change %s, deleting." %
(ref, maxChange)
+print("Branch %s did not exist at change %s, deleting." %
(ref, maxChange))
system("git update-ref -d %s `git rev-parse %s`" % (ref,
ref))
continue
while change and int(change) > maxChange:
changed = True
if self.verbose:
-print "%s is at %s ; rewinding towards %s" % (ref,
change, maxChange)
+print("%s is at %s ; rewinding towards %s" % (ref,
change, maxChange))
system("git update-ref %s \"%s^\"" % (ref, ref))
log = extractLogMessageFromGitCommit(ref)
settings = extractSettingsGitLog(log)
@@ -1457,7 +1457,7 @@ def run(self, args):
change = settings['change']
if changed:
-print "%s rewound to %s" % (ref, change)
+print("%s rewound to %s" % (ref, change))
return True
@@ -1593,10 +1593,10 @@ def patchRCSKeywords(self, file, pattern):
except:
# cleanup our temporary file
os.unlink(outFileName)
-print "Failed to strip RCS keywords in %s" % file
+print("Failed to strip RCS keywords in %s" % file)
raise
-print "Patched up RCS keywords in %s" % file
+print("Patched up RCS keywords in %s" % file)
def p4UserForCommit(self,id):
# Return the tuple (perforce user,git email) for a given git commit id
@@ -1616,7 +1616,7 @@ def checkValidP4Users(self,commits):
if not user:
msg = "Cannot find p4 user for email
Re: [PATCH 17/29] t: use test_must_fail() instead of checking exit code manually
On 26 June 2018 at 08:29, Eric Sunshine wrote: > These tests intentionally break the &&-chain to manually check the exit > code of invoked commands which they expect to fail, and invert that > local expected failure into a successful exit code for the test overall. > Such manual exit code manipulation predates the invention of > test_must_fail(). > > An upcoming change will teach --chain-lint to check the &&-chain inside > subshells. This sort of manual exit code checking will trip up > --chain-lint due to the intentional break in the &&-chain. Therefore, > replace the manual exit code management with test_must_fail() and a > normal &&-chain. > > Signed-off-by: Eric Sunshine > --- > t/t5405-send-pack-rewind.sh | 3 +-- > t/t9814-git-p4-rename.sh| 16 ++-- > 2 files changed, 3 insertions(+), 16 deletions(-) > > diff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh > index 4bda18a662..235fb7686a 100755 > --- a/t/t5405-send-pack-rewind.sh > +++ b/t/t5405-send-pack-rewind.sh > @@ -25,8 +25,7 @@ test_expect_success 'non forced push should die not > segfault' ' > > ( > cd another && > - git push .. master:master > - test $? = 1 > + test_must_fail git push .. master:master > ) > > ' > diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh > index e7e0268e98..80aac5ab16 100755 > --- a/t/t9814-git-p4-rename.sh > +++ b/t/t9814-git-p4-rename.sh > @@ -12,20 +12,8 @@ test_expect_success 'start p4d' ' > test_expect_success 'p4 help unknown returns 1' ' > ( > cd "$cli" && > - ( > - p4 help client >errs 2>&1 > - echo $? >retval > - ) > - echo 0 >expected && > - test_cmp expected retval && > - rm retval && > - ( > - p4 help nosuchcommand >errs 2>&1 > - echo $? >retval > - ) > - echo 1 >expected && > - test_cmp expected retval && > - rm retval > + p4 help client && > + test_must_fail p4 help nosuchcommand This seems a lot more sensible. It works fine in my tests. Thanks, Luke
could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'
I've recently started seeing a lot of this message when doing a rebase: warning: could not freshen shared index '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.' (There's a repo called dev_full, and I've got a worktree where I'm working on my 3rd attempt to make it work with gcc8). That file doesn't actually exist but there are a bunch of sharedindex.XXX files in there with more convincing looking names. 2.20.1.611.gfbb209baf1
Re: Weird (seemingly flakey) p4 breakage in t9833
On Wed, 6 Feb 2019 at 11:11, SZEDER Gábor wrote: > > On Wed, Feb 06, 2019 at 11:33:21AM +0100, Johannes Schindelin wrote: > > Hi Luke, > > > > in a private Azure Pipeline (sorry...) I noticed an intermittent problem > > in the p4 tests on osx-gcc. > > > > I would point you to a public log, but the Azure Pipelines support *just* > > made it to next, so I *just* set up a public one targeting anything else > > than my `vsts-ci` branch, at > > https://dev.azure.com/gitgitgadget/git/_build/index?definitionId=6. And > > those builds do not show that problem, so it must be a flakey test. > > > > But maybe you can spot anything familiar from the log? I think you're right - it's 'git operation with expired ticket'. I wonder if we could make a faked-up Perforce client which returned the correct error data - then it wouldn't need to wait at all. Or we could just remove it (or make it optional based on an environment variable). Let me know which you think is best and I'll put together a patch. Luke
[PATCH] git-p4: remove ticket expiry test
The git-p4 login ticket expiry test causes unreliable test runs. Since the handling of ticket expiry in git-p4 is far from polished anyway, let's remove it for now. A better way to actually run the test is to create a python "fake" version of "p4" which returns whatever expiry results the test requires. Ideally git-p4 would look at the expiry time before starting any long operations, and cleanup gracefully if there is not enough time left. But that's quite hard to do. Signed-off-by: Luke Diamand --- t/t9833-errors.sh | 27 --- 1 file changed, 27 deletions(-) diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh index 277d347012..47b312e1c9 100755 --- a/t/t9833-errors.sh +++ b/t/t9833-errors.sh @@ -45,33 +45,6 @@ test_expect_success 'ticket logged out' ' ) ' -test_expect_success 'create group with short ticket expiry' ' - P4TICKETS="$cli/tickets" && - echo "newpassword" | p4 login && - p4_add_user short_expiry_user && - p4 -u short_expiry_user passwd -P password && - p4 group -i <<-EOF && - Group: testgroup - Timeout: 3 - Users: short_expiry_user - EOF - - p4 users | grep short_expiry_user -' - -test_expect_success 'git operation with expired ticket' ' - P4TICKETS="$cli/tickets" && - P4USER=short_expiry_user && - echo "password" | p4 login && - ( - cd "$git" && - git p4 sync && - sleep 5 && - test_must_fail git p4 sync 2>errmsg && - grep "failure accessing depot" errmsg - ) -' - test_expect_success 'kill p4d' ' kill_p4d ' -- 2.20.1.611.gfbb209baf1
[PATCH 0/1] git-p4: remove ticket expiration test
As per thread here, this removes the git-p4 ticket expiration test, since it isn't really that useful. https://marc.info/?l=git&m=154946136416003&w=2 Luke Diamand (1): git-p4: remove ticket expiry test t/t9833-errors.sh | 27 --- 1 file changed, 27 deletions(-) -- 2.20.1.611.gfbb209baf1
Re: [PATCH 1/1] git-p4: recover from inconsistent perforce history
ss 'init depot' ' > + ( > + cd "$cli" && > + > + touch add_file_add_dir_del_file add_file_add_dir_del_dir && > + p4 add add_file_add_dir_del_file add_file_add_dir_del_dir && > + mkdir add_dir_add_file_del_file add_dir_add_file_del_dir && > + touch add_dir_add_file_del_file/file > add_dir_add_file_del_dir/file && > + p4 add add_dir_add_file_del_file/file > add_dir_add_file_del_dir/file && > + p4 submit -d "add initial" && > + > + rm -f add_file_add_dir_del_file add_file_add_dir_del_dir && > + mkdir add_file_add_dir_del_file add_file_add_dir_del_dir && > + touch add_file_add_dir_del_file/file > add_file_add_dir_del_dir/file && > + p4 add add_file_add_dir_del_file/file > add_file_add_dir_del_dir/file && > + rm -rf add_dir_add_file_del_file add_dir_add_file_del_dir && > + touch add_dir_add_file_del_file add_dir_add_file_del_dir && > + p4 add add_dir_add_file_del_file add_dir_add_file_del_dir && > + p4 submit -d "add conflicting" && > + > + p4 delete -k add_file_add_dir_del_file && > + p4 delete -k add_file_add_dir_del_dir/file && > + p4 delete -k add_dir_add_file_del_file && > + p4 delete -k add_dir_add_file_del_dir/file && > + p4 submit -d "delete conflicting" > + ) > +' > + > +test_expect_success 'clone with git-p4' ' > + git p4 clone --dest="$git" //depot/@all > +' > + > +test_expect_success 'check final contents' ' > + test_path_is_dir "$git/add_file_add_dir_del_file" && > + test_path_is_file "$git/add_file_add_dir_del_dir" && > + test_path_is_dir "$git/add_dir_add_file_del_file" && > + test_path_is_file "$git/add_dir_add_file_del_dir" > +' > + > +test_expect_success 'kill p4d' ' > + kill_p4d > +' > + > +test_done > -- > 2.19.2 > -- Luke Diamand
Re: [PATCH 0/1] git-p4: remove ticket expiration test
On Thu, 7 Feb 2019 13:45:18 +0100 (STD) Johannes Schindelin wrote: > Hi Luke, > > On Wed, 6 Feb 2019, Luke Diamand wrote: > > > As per thread here, this removes the git-p4 ticket expiration > > test, since it isn't really that useful. > > > > https://marc.info/?l=git&m=154946136416003&w=2 > > Thank you for the prompt patch! > > However, like Gábor, my feeling is that we would want that test case in a > non-flakey form, if possible. If you think that that is only possible with > a mocked p4, so be it, let's remove the test case (because the mocked one > will likely look quite a bit different). But if there are easier ways to > work around the timing issues (such as dropping the first `sync`), then > I'd prefer to have the safety of a regression test. I've got a mocked-up p4 wrapper which returns whatever expiration time the test needs. I'll submit it tomorrow. It's just a few lines of python script to generate the marshalled data, so it's not very complicated. > > Thanks, > Dscho > > > Luke Diamand (1): > > git-p4: remove ticket expiry test > > > > t/t9833-errors.sh | 27 --- > > 1 file changed, 27 deletions(-) > > > > -- > > 2.20.1.611.gfbb209baf1 > > > > -- Luke Diamand
Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'
On Fri, 8 Feb 2019 at 10:02, Duy Nguyen wrote:
>
> On Wed, Feb 06, 2019 at 10:25:25AM +, Luke Diamand wrote:
> > I've recently started seeing a lot of this message when doing a rebase:
> >
> >warning: could not freshen shared index
> > '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.'
>
> There are only two places in the code that could print this. The one
> in read_index_from() can't happen unless is_null_oid() is broken (very
> very unlikely).
>
> The other one is in write_locked_index() which could happen in theory
> but I don't understand how it got there. If you could build git, could
> you try this patch and see if it helps?
They've gone away!
>
> -- 8< --
> diff --git a/read-cache.c b/read-cache.c
> index f68b367613..5ad71478dc 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -3165,6 +3165,7 @@ int write_locked_index(struct index_state *istate,
> struct lock_file *lock,
> fill_fsmonitor_bitmap(istate);
>
> if (!si || alternate_index_output ||
> + (si && is_null_oid(&si->base_oid)) ||
> (istate->cache_changed & ~EXTMASK)) {
> if (si)
> oidclr(&si->base_oid);
> -- 8< --
>
Luke
[PATCHv2] git-p4: ticket expiry test: use a fake p4 to avoid use of 'sleep'
From: Luke Diamand
This tests for p4 ticket expiry by creating a ticket, and then waiting
long enough for the ticket to nearly expire. However, this is
unreliable.
Instead, create a 'fake' p4 which returns expiry times under the
control of the test script, and forwards other commands to the
real p4 executable.
Signed-off-by: Luke Diamand
---
t/t9833-errors.sh | 42 ++
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
index 277d347012..9d7cb5b35b 100755
--- a/t/t9833-errors.sh
+++ b/t/t9833-errors.sh
@@ -45,30 +45,32 @@ test_expect_success 'ticket logged out' '
)
'
-test_expect_success 'create group with short ticket expiry' '
- P4TICKETS="$cli/tickets" &&
- echo "newpassword" | p4 login &&
- p4_add_user short_expiry_user &&
- p4 -u short_expiry_user passwd -P password &&
- p4 group -i <<-EOF &&
- Group: testgroup
- Timeout: 3
- Users: short_expiry_user
- EOF
-
- p4 users | grep short_expiry_user
-'
+# create a fake version of "p4" which returns a TicketExpiration based
+# on $EXPIRY, for testing login expiration
+create_fake_p4() {
+ (
+ cd "$git" && mkdir expire-p4 &&
+ cat >>expire-p4/p4 <<-EOF &&
+ #!/usr/bin/python
+ import marshal, os, subprocess, sys
+ if "login" in sys.argv:
+ marshal.dump({"foo" : "bar", "code" : "stat",
"TicketExpiration" : os.environ["EXPIRY"]}, sys.stdout)
+ else:
+ subprocess.check_call([os.environ["P4"]] + sys.argv[1:])
+ EOF
+ chmod 0755 expire-p4/p4
+ )
+}
test_expect_success 'git operation with expired ticket' '
- P4TICKETS="$cli/tickets" &&
- P4USER=short_expiry_user &&
- echo "password" | p4 login &&
+ create_fake_p4 &&
+ echo "newpassword" | p4 login &&
(
cd "$git" &&
- git p4 sync &&
- sleep 5 &&
- test_must_fail git p4 sync 2>errmsg &&
- grep "failure accessing depot" errmsg
+ P4=$(command -v p4) && export P4 &&
+ EXPIRY=3600 PATH=$PWD/expire-p4:$PATH git p4 sync &&
+ EXPIRY=1 PATH=$PWD/expire-p4:$PATH test_must_fail git p4 sync
-v 2>errmsg &&
+ grep "failure accessing depot.*expires in 1 second" errmsg
)
'
--
2.20.1.612.g17ebf93fb6.dirty
Re: could not freshen shared index ..../sharedindex.0000000000000000000000000000000000000000'
On Sat, 9 Feb 2019 at 05:01, Duy Nguyen wrote: > > On Fri, Feb 8, 2019 at 11:39 PM Luke Diamand wrote: > > > > On Fri, 8 Feb 2019 at 10:02, Duy Nguyen wrote: > > > > > > On Wed, Feb 06, 2019 at 10:25:25AM +, Luke Diamand wrote: > > > > I've recently started seeing a lot of this message when doing a rebase: > > > > > > > >warning: could not freshen shared index > > > > '/home/ldiamand/git/dev_full/.git/worktrees/gcc8-take-2/sharedindex.' > > > > > > There are only two places in the code that could print this. The one > > > in read_index_from() can't happen unless is_null_oid() is broken (very > > > very unlikely). > > > > > > The other one is in write_locked_index() which could happen in theory > > > but I don't understand how it got there. If you could build git, could > > > you try this patch and see if it helps? > > > > They've gone away! > > Great! Since you seem able to reproduce this (and can build git!) > could you try bisect to pin point the commit that causes this problem? > That would help a lot. I think you could start maybe from v2.19.0 The first bad commit was d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2. $ git show d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2 commit d658196f3c4b2478ebdff638e2c3e4fb2f9cbba2 (refs/bisect/bad) Merge: 6b0f1d9c47 7db118303a Author: Junio C Hamano Date: Wed May 23 14:38:22 2018 +0900 Merge branch 'en/unpack-trees-split-index-fix' The split-index feature had a long-standing and dormant bug in certain use of the in-core merge machinery, which has been fixed. * en/unpack-trees-split-index-fix: unpack_trees: fix breakage when o->src_index != o->dst_index The test I'm doing is just: 1. git checkout some_tag 2. git rebase -i HEAD~5 3. Swap the top and bottom commit 4. repeat I just chose "5" as my first wild guess, other numbers are also available. With "5" I get 3 lots of: warning: could not freshen shared index '.git/sharedindex.' As far as I can tell the actual rebasing is fine.
Re: [PATCH] unpack-trees.c: fix writing "link" index ext with null base oid
On Sat, 9 Feb 2019 at 11:23, Nguyễn Thái Ngọc Duy wrote:
>
> Since commit 7db118303a (unpack_trees: fix breakage when o->src_index !=
> o->dst_index - 2018-04-23) and changes in merge code to use separate
> index_state for source and destination, when doing a merge with split
> index activated, we may run into this line in unpack_trees():
>
> o->result.split_index = init_split_index(&o->result);
>
> This is by itself not wrong. But this split index information is not
> fully populated (and it's only so when move_cache_to_base_index() is
> called, aka force splitting the index, or loading index_state from a
> file). Both "base_oid" and "base" in this case remain null.
>
> So when writing the main index down, we link to this index with null
> oid (default value after init_split_index()), which also means "no split
> index" internally. This triggers an incorrect base index refresh:
>
> warning: could not freshen shared index '.../sharedindex.0{40}'
>
> This patch makes sure we will not refresh null base_oid (because the
> file is never there). It also makes sure not to write "link" extension
> with null base_oid in the first place (no point having it at
> all). Read code already has protection against null base_oid.
>
> There is also another side fix in remove_split_index() that causes a
> crash when doing "git update-index --no-split-index" when base_oid in
> the index file is null. In this case we will not load
> istate->split_index->base but we dereference it anyway and are rewarded
> with a segfault. This should not happen anymore, but it's still wrong to
> dereference a potential NULL pointer, especially when we do check for
> NULL pointer in the next code.
>
> Reported-by: Luke Diamand
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> I considered adding a test, but since the problem is a warning, not
> sure how to catch that. And a test would not be able to verify all
> changes in this patch anyway.
>
> read-cache.c | 5 +++--
> split-index.c | 34 ++
> 2 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 0e0c93edc9..d6fb09984f 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2894,7 +2894,8 @@ static int do_write_index(struct index_state *istate,
> struct tempfile *tempfile,
> return -1;
> }
>
> - if (!strip_extensions && istate->split_index) {
> + if (!strip_extensions && istate->split_index &&
> + !is_null_oid(&istate->split_index->base_oid)) {
> struct strbuf sb = STRBUF_INIT;
>
> err = write_link_extension(&sb, istate) < 0 ||
> @@ -3189,7 +3190,7 @@ int write_locked_index(struct index_state *istate,
> struct lock_file *lock,
> ret = write_split_index(istate, lock, flags);
>
> /* Freshen the shared index only if the split-index was written */
> - if (!ret && !new_shared_index) {
> + if (!ret && !new_shared_index && !is_null_oid(&si->base_oid)) {
> const char *shared_index = git_path("sharedindex.%s",
>
> oid_to_hex(&si->base_oid));
> freshen_shared_index(shared_index, 1);
> diff --git a/split-index.c b/split-index.c
> index 5820412dc5..a9d13611a4 100644
> --- a/split-index.c
> +++ b/split-index.c
> @@ -440,24 +440,26 @@ void add_split_index(struct index_state *istate)
> void remove_split_index(struct index_state *istate)
> {
> if (istate->split_index) {
> - /*
> -* When removing the split index, we need to move
> -* ownership of the mem_pool associated with the
> -* base index to the main index. There may be cache entries
> -* allocated from the base's memory pool that are shared with
> -* the_index.cache[].
> -*/
> - mem_pool_combine(istate->ce_mem_pool,
> istate->split_index->base->ce_mem_pool);
> + if (istate->split_index->base) {
> + /*
> +* When removing the split index, we need to move
> +* ownership of the mem_pool associated with the
> +* base index to the main index. There may be cache
> entries
> +* allocated from the base's memory pool that are
> shared with
> +* the_index.cache[].
> +
Re: [PATCHv2] git-p4: ticket expiry test: use a fake p4 to avoid use of 'sleep'
On Sat, 9 Feb 2019 at 18:06, Junio C Hamano wrote:
>
> SZEDER Gábor writes:
>
> > On Fri, Feb 08, 2019 at 07:02:31PM +0000, Luke Diamand wrote:
> >> +# create a fake version of "p4" which returns a TicketExpiration based
> >> +# on $EXPIRY, for testing login expiration
> >> +create_fake_p4() {
> >> +(
> >> +cd "$git" && mkdir expire-p4 &&
> >> +cat >>expire-p4/p4 <<-EOF &&
> >> +#!/usr/bin/python
> >
> > I think this should be $PYTHON_PATH.
>
> OK.
>
> Luke, I think our mails / work crossed and the tip of 'master'
> already has a removal. Please make this into a separate patch that
> adds (or, resurrects with an improved version) the test for the next
> cycle.
OK, thanks!
Re: Occasional git p4 test failures because of stray fast-import processes
On Wed, 27 Feb 2019 at 09:49, SZEDER Gábor wrote:
>
> Hi Luke,
>
> I saw rare failures in test 6 'git p4 sync uninitialized repo' in
> 't9800-git-p4-basic.sh' on Travis CI, because the 'cleanup_git'
> function failed to do its job. The (redacted) trace looks like this:
Thanks for the *very* detailed analysis!
I think you are right - git-p4 should wait() for all of its children,
and that ought to fix this.
I think I may have even added the bit of code you mention (about a
decade ago now).
I'll have a look and see what can be done.
Thanks!
Luke
>
> + cleanup_git
> + retry_until_success rm -r /home/szeder/src/git/t/trash
> directory.t9800-git-p4-basic/git
> + time_in_seconds
> + cd /
> + /usr/bin/python -c import time; print(int(time.time()))
> + timeout=1551233042
> + rm -r /home/szeder/src/git/t/trash directory.t9800-git-p4-basic/git
> + test_must_fail test -d /home/szeder/src/git/t/trash
> directory.t9800-git-p4-basic/git
> test_must_fail: command succeeded: test -d /home/szeder/src/git/t/trash
> directory.t9800-git-p4-basic/git
> + eval_ret=1
> + :
> not ok 6 - git p4 sync uninitialized repo
>
> Trying to reproduce this failure with stock Git can be tricky: I've
> seen
>
> ./t9800-git-p4-basic.sh --stress -r 1,2,6,22
>
> fail within the first 10 tries, but it also run overnight without a
> single failure... However, the following two-liner patch can reliably
> trigger this failure:
>
> diff --git a/fast-import.c b/fast-import.c
> index b7ba755c2b..54715018b3 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -3296,6 +3296,7 @@ int cmd_main(int argc, const char **argv)
> rc_free[i].next = &rc_free[i + 1];
> rc_free[cmd_save - 1].next = NULL;
>
> + sleep(1);
> start_packfile();
> set_die_routine(die_nicely);
> set_checkpoint_signal();
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> index b3be3ba011..2d2ef50bfa 100644
> --- a/t/lib-git-p4.sh
> +++ b/t/lib-git-p4.sh
> @@ -190,6 +190,7 @@ kill_p4d () {
>
> cleanup_git () {
> retry_until_success rm -r "$git"
> + sleep 2
> test_must_fail test -d "$git" &&
> retry_until_success mkdir "$git"
> }
>
>
> What's going on is this: 'git p4' invokes 'git fast-import' via
> Python's subprocess.Popen(), and then there are two chain of events
> racing with each other:
>
> - In the foreground:
>
> - 'git p4' notices that there are no p4 branches and die()s as
> expected, but leaves its child fast-import behind
> - 'test_i18ngrep' quickly verifies that 'git p4' died with the
> expected error message
> - the cleanup function removes the "$TRASH_DIRECTORY/git"
> directory, and
> - checks that the directory is indeed gone.
>
> - Meanwhile in the background:
>
> - 'git fast-import' starts up, kicks off the dashed external
> 'git-fast-import' process,
> - which opens a tmp packfile in "$TRASH_DIRECTORY/git" for writing
> (the start_packfile() call in the patch context above), creating
> any leading directories if necessary,
> - notices that its stdin is closed (because 'git p4' died) and it
> has nothing left to do, so
> - it cleans up and exits. Note that this cleanup only removes the
> tmp packfile, but leaves any newly created leading directories
> behind.
>
> While unlikely, it does apparently happen from time to time that the
> test's cleanup function first removes "$TRASH_DIRECTORY/git", but then
> 'git fast-import' re-creates it for its packfile before the cleanup
> function checks the result of the removal. The two well-placed
> sleep()s in the patch above force such a problematic scheduling.
>
> There are 4 test cases running 'test_must_fail git p4 sync': the above
> patch triggers a failure in 3 of them, and with a bit of extra modding
> I could trigger a failure in the fourth one as well.
>
> We could work this around by waiting for 'git p4' and all its child
> processes in the affected tests themselves, using the same shell
> trickery as in ef09036cf3 (t6500: wait for detached auto gc at the end
> of the test script, 2017-04-13), but this feels like, well, a
> workaround.
>
> I think the proper solution would be to ensure that 'git p4' kills and
> waits for all its child processes when die()ing. Alternatively (or in
> addition?), it could perform all the necessary sanity-checking (and
> potential die()ing) before starting the 'git fast-import' process in
> the first place.
>
> I've glanced through all subprocess.Popen() callsites in 'git p4' and
> found most of them OK, in the sense that they wait for whatever child
> process they create. Alas, there was one exception: p4CmdList() can
> invoke an optional callback function before wait()ing on its 'p4'
> child, and the streamP4FilesCb() callback function can die() without
> waiting for the 'p4' process (but it does wait() for 'git
> fast-import'!).
>
> On a related note: this check for the just-r
Re: [RFC PATCH 2/2] git-p4: support loading changelist descriptions from files
On Fri, 22 Mar 2019 at 19:54, Mazo, Andrey wrote:
>
> Our Perforce server experienced some kind of database corruption a few years
> ago.
> While the file data and revision history are mostly intact,
> some metadata for several changesets got lost.
I think it's not unheard of for P4 databases to end up being corrupt,
as in your case.
It looks like the RCS files get updated, but the database files (i.e.
the metadata) do not, after which you have a bit of a problem.
So I guess this change could be quite useful, but it really needs some
documentation and tests to support it - git-p4 is already complicated
enough!
Your example script should probably use the same magic that the git-p4
script uses to pick the path to Python.
And perhaps come up with a nicer name than "damaged" - as you say, it
could also be used for other purposes.
> For example, inspecting certain changelists produces errors.
> """
> $ p4 describe -s 12345
> Date 2019/02/26 16:46:17:
> Operation: user-describe
> Operation 'user-describe' failed.
> Change 12345 description missing!
> """
>
> While some metadata (like changeset descriptions) is obviously lost,
> most of it can be reconstructed via other commands:
> * `p4 changes -l -t //...@12345,12345` --
>to obtain date+time, author, beginning of changeset description;
> * `p4 files -a //...@12345,12345` --
>to obtain file revisions, file types, file actions;
> * `p4 diff2 -u //...@12344 //...@12345` --
>to get a unified diff of text files in a changeset;
> * `p4 print -o binary.blob@12345 //depot/binary.blob@12345` --
>to get a revision of a binary file.
>
> It might be possible to teach git-p4 to fallback to other methods if `p4
> describe` fails,
> but it's probably too special-cased (really depends on kind and scale of DB
> corruption),
> so some manual intervention is perhaps acceptable.
>
> So, with some manual work, it's possible to reconstruct `p4 -G describe ...`
> output manually.
> In our case, once git-p4 passes `p4 describe` stage,
> it can proceed further just fine.
> Thus, it's tempting to feed resurrected metadata to git-p4 when a normal `p4
> describe` would fail.
>
> This functionality may be useful to cache changelist information,
> or to make some changes to changelist info before feeding it to git-p4.
>
> A new config parameter is introduced to tell git-p4
> to load certain changelist descriptions from files instead of from a server.
> For simplicity, it's one pickled file per changelist.
> ```
> git config --add git-p4.damagedChangelists 12345.pickled
> git config --add git-p4.damagedChangelists 12346.pickled
> ```
>
> The following trivial script may be used to produce pickled `p4 -G
> describe`-compatible output.
> """
> #!/usr/bin/python2
>
> import pickle
> import time
>
> # recovered commits of interest
> changes = [
> {
> 'change': '12345',
> 'status': 'submitted',
> 'code': 'stat',
> 'user': 'username1',
> 'time': str(int(time.mktime(time.strptime('2019/02/28
> 16:00:30', '%Y/%m/%d %H:%M:%S',
> 'client': 'username1_hostname1',
> 'desc': 'A bug is fixed.\nDetails are below:\n',
> 'depotFile0': '//depot/branch1/foo.sh',
> 'action0':'edit',
> 'rev0': '28',
> 'type0': 'xtext',
> 'depotFile1': '//depot/branch1/bar.py',
> 'action1':'edit',
> 'rev1': '43',
> 'type1': 'text',
> 'depotFile2': '//depot/branch1/baz.doc',
> 'action2':'edit',
> 'rev2': '8',
> 'type2': 'binary',
> 'depotFile3': '//depot/branch1/qqq.c',
> 'action3':'edit',
> 'rev3': '6',
> 'type3': 'ktext',
> },
> ]
>
> for change in changes:
> pickle.dump(change, open('{0}.pickled'.format(change['change']), 'wb'))
> """
>
> Signed-off-by: Andrey Mazo
> ---
>
> Notes:
> Documentation changes and tests are obviously missing,
> but I hoped to get some feedback on the idea overall
> before working on those.
>
> git-p4.py | 25 -
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 40bc84573b..3133419280 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -24,10 +24,11 @@
> import stat
> import zipfile
> import zlib
> import ctypes
> import errno
> +import pickle
>
> # support basestring in python3
> try:
> unicode = unicode
> except NameError:
> @@ -2615,10 +2616,12 @@ def __init__(self):
> self.knownAlienLabelBranches = {}
>
> self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone %
> 3600) / 60))
> self.labels = {}
>
> +self.damagedChangelists = {}
> +
> # Force a checkpoint in fast-import and wait for it to finish
> def checkpoint(self):
> self.gitStream.write("checkpoint\n\n")
> self.gitStream
Re: [RFC PATCH 1/2] git-p4: introduce alien branch mappings
On Fri, 22 Mar 2019 at 19:54, Mazo, Andrey wrote: > > 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, So there is a file in subprojectA/foo.c@41. And label 1B is against //depot/metaproject/branch1/subprojectB/bar.c@42. And I suppose in Perforce you could still checkout subprojectA at change 42 and you would get change 41. But with the way git-p4 works, the label just gets discarded. You want to be able to checkout the subjectA with a tag called 1B and get the file contents as of 42. I wonder if it would be easier to teach the code in importP4Labels to go searching harder for the next lower changelist number? Where it currently says "could not find git commit"... could it do something like "p4 changes -m1 //depot/path/...@LABEL" and use that instead? I'm not sure if that would work but it would mean you wouldn't need any extra configuration to maintain. But perhaps I have misunderstood what you're trying to do here! Perhaps a failing test case might help explain it better? Thanks Luke > 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) > > So, introduce a concept of an "alien" branch. > > In the above example of importing subprojectA, > * branch1/subprojectB is an alien branch for branch1/subprojectA; > * branch2/subprojectB is an alien branch for branch2/subprojectA. > Any changelist for branch1/subprojectB will be imported into subprojectA.git > branch1 > as an empty commit for the sole purpose of being labeled with a tag later > or just to preserve the history of changes across the branches. > > This relation between branches is specified in a similar way to branchList: > `git config --add git-p4.alienLabelBranchMap alien_branch:real_branch` > > For the example of importing subprojectA above, config parameters are > ``` > git config --add git-p4.branchList branch1/subprojectA:branch2/subprojectA > git config --add git-p4.alienLabelBranchMap > branch1/subprojectB:branch1/subprojectA > git config --add git-p4.alienLabelBranchMap > branch2/subprojectB:branch2/subprojectA > ``` > > A similar use case, is when a label is placed on a changelist for an excluded > path. > 1. Depot layout is as follows: > //depot/branch1/... > //depot/branch1/exclude_me/... > 2. Labels are placed as follows: > * label 1 on //depot/branch1/... > * label 1E on //depot/branch1/exclude_me/... > 3. The goal is to import > //depot/... into depot.git excluding files under > //depot/branch1/exclude_me/... > and preserving all the branches and labels. > 4. Importing subprojectA. > Label 1 is imported fine because it's placed on certain commit on branch1. > However, label 1E is not imported because it's placed on a commit which > is excluded. > > For this use case, the config would be > ``` > git config --add git-p4.alienLabelBranchMap branch1/exclude_me:branch1 > ``` > > Note that the current implementation doesn't process alien branches > when a clientspec is used. > > Diff best viewed with --ignore-all-space . > > Signed-off-by: Andrey Mazo > --- > > Notes: > Documentation changes and tests are obviously missing, > but I hoped to get some feedback on the idea overall > before working on those. > > A better name for "alien" branches is very welcome. > > git-p4.py | 59 +
Re: [PATCH v2 2/7] git-p4: match branches case insensitively if configured
On Thu, 21 Mar 2019 at 22:32, Mazo, Andrey wrote: > > git-p4 knows how to handle case insensitivity in file paths > if core.ignorecase is set. > However, when determining a branch for a file, > it still does a case-sensitive prefix match. > This may result in some file changes to be lost on import. > > For example, given the following commits > 1. add //depot/main/file1 > 2. add //depot/DirA/file2 > 3. add //depot/dira/file3 > 4. add //depot/DirA/file4 > and "branchList = main:DirA" branch mapping, > commit 3 will be lost. > > So, do branch search case insensitively if running with core.ignorecase set. > Teach splitFilesIntoBranches() to use the p4PathStartsWith() function > for path prefix matches instead of always case-sensitive match. I wonder what other code paths break due to this problem! Looks reasonable but I fear there may be some other holes in there - quickly looking through the code suggests there are several other places this problem occurs. Luke > > Signed-off-by: Andrey Mazo > --- > git-p4.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-p4.py b/git-p4.py > index c0a3068b6f..91c610f960 100755 > --- a/git-p4.py > +++ b/git-p4.py > @@ -2721,11 +2721,11 @@ def splitFilesIntoBranches(self, commit): > 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, e.g. > -if relPath.startswith(branch + "/"): > +if p4PathStartsWith(relPath, branch + "/"): > if branch not in branches: > branches[branch] = [] > branches[branch].append(file) > break > > -- > 2.19.2 >
Re: [PATCH 1/1] add hook pre-p4-submit
On 23 July 2018 at 12:27, Chen Bin wrote:
> Hook pre-p4-submit is executed before git-p4 actually submits code.
> If the hook exits with non-zero value, submit process will abort.
Looks good to me - could you add some documentation please
(Documentation/git-p4.txt).
Thanks!
Luke
>
> Signed-off-by: Chen Bin
> ---
> git-p4.py | 6 ++
> t/t9800-git-p4-basic.sh | 22 ++
> 2 files changed, 28 insertions(+)
>
> diff --git a/git-p4.py b/git-p4.py
> index b449db1cc..69ee9ce41 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2303,6 +2303,12 @@ def run(self, args):
> sys.exit("number of commits (%d) must match number of shelved
> changelist (%d)" %
> (len(commits), num_shelves))
>
> +# locate hook at `.git/hooks/pre-p4-submit`
> +hook_file = os.path.join(read_pipe("git rev-parse
> --git-dir").strip(), "hooks", "pre-p4-submit")
> +# Execute hook. If it exit with non-zero value, do NOT continue.
> +if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and
> subprocess.call([hook_file]) != 0:
> +sys.exit(1)
> +
> #
> # Apply the commits, one at a time. On failure, ask if should
> # continue to try the rest of the patches, or quit.
> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
> index 4849edc4e..48b768fa7 100755
> --- a/t/t9800-git-p4-basic.sh
> +++ b/t/t9800-git-p4-basic.sh
> @@ -261,6 +261,28 @@ test_expect_success 'unresolvable host in P4PORT should
> display error' '
> )
> '
>
> +# Test following scenarios:
> +# - Without hook ".git/hooks/pre-p4-submit", submit should continue
> +# - With hook returning 0, submit should continue
> +# - With hook returning 1, submit should abort
> +test_expect_success 'run hook pre-p4-submit before submit' '
> + test_when_finished cleanup_git &&
> + git p4 clone --dest="$git" //depot &&
> + (
> + cd "$git" &&
> + echo "hello world" >hello.txt &&
> + git add hello.txt &&
> + git commit -m "add hello.txt" &&
> + git config git-p4.skipSubmitEdit true &&
> + git p4 submit --dry-run | grep "Would apply" &&
> + mkdir -p .git/hooks &&
> + : >.git/hooks/pre-p4-submit && chmod +x
> .git/hooks/pre-p4-submit &&
> + git p4 submit --dry-run | grep "Would apply" &&
> + echo #!/bin/sh && echo exit 1 >.git/hooks/pre-p4-submit &&
> + git p4 submit --dry-run | grep "Would apply" || echo "Abort
> submit"
> + )
> +'
> +
> test_expect_success 'submit from detached head' '
> test_when_finished cleanup_git &&
> git p4 clone --dest="$git" //depot &&
> --
> 2.18.0
>
Re: [PATCH 1/1] add hook pre-p4-submit
On 26 July 2018 at 10:21, Eric Sunshine wrote: > On Wed, Jul 25, 2018 at 10:08 PM chen bin wrote: >> The hook does not receive any information or input from git. The >> original requirement >> comes from my colleague. He want to run unit test automatically before >> submitting code >> to remote repository. Or else CI server will send the blame mail to the >> manager. > > Okay, that seems in line with a hook such as pre-commit. Please do > update the documentation to mention that the hook takes no arguments > and nothing on standard input, and perhaps describe in the > documentation an example use-case (as you did here). > > I'm not a p4 or git-p4 user, but, out of curiosity, would there be any > information which could be supplied to the hook as arguments or > standard input (or both) which would help the hook author implement > the hook more easily? Perhaps such information would be fodder for a > future enhancement (not necessarily needed for this patch). I tried to think of a use-case for a hook requiring any more information, but I can't think of any. You're already chdir()'d to the P4 shadow repo which is what you really need. Anything where you just need the commit hash (e.g. checking the commit message) can already be done with one of the existing git hooks; I don't think git-p4 needs to duplicate that. And we can't write a commit hook that can know about the Perforce changelist, because we don't know what it is yet. However, looking at the code, it runs the hook at the point just *before* the changes are applied to the P4 shadow repo. Would it make more sense to run the hook *after* they have been applied (but before being P4 submitted) ? That way you can run your tests on the checked-out P4 shadow directory with your changes - as it stands, you can only run them on your git repo at this point, which might not be in sync with Perforce (and could be quite a long way out in fact). Luke > >> The hook actually stops the submit process from start instead of abort >> submit in midway. >> So nothing is touched when hook exits with status 1. > > This might be a good thing to add to the documentation, as well. > >> I'm not sure whether `git-p4` should print some "hook rejection" message. >> Current implementation is same as other hooks (`pre-commit`, for example). >> Only hook itself is responsible to print error messages. >> >> Personally I don't have opinion whether we should print out hook >> related message inside >> `git-p4.py`. I just try to following existing convention of git. >> >> What you guys think? > > Following existing practice makes sense. It can always be revisited > later if needed. > > Thanks.
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
On 30 July 2018 at 20:07, Junio C Hamano wrote:
> Chen Bin writes:
>
>> The `p4-pre-submit` hook is executed before git-p4 submits code.
>> If the hook exits with non-zero value, submit process not start.
>>
>> Signed-off-by: Chen Bin
>> ---
>
> Luke, does this version look good to you?
>
Yes, I think so, We might find in the future that we do need an
additional hook *after* the change has been applied, but as per Chen's
comment, it sounds like that's not what is needed here; it can easily
be added in the future if necessary.
And there's no directly equivalent existing git-hook which could be
used instead, so this seems like a useful addition.
Possibly it should say "it takes no parameters" rather than "it takes
no parameter"; it is confusing that in English, zero takes the plural
rather than the singular. There's a PhD in linguistics there for
someone!
Luke
> I still think the addition to self.description is a bit too much but
> other than that the incremental since the last one looked sensible
> to my untrained eyes ;-)
>
> Thanks, both.
>
>> Documentation/git-p4.txt | 8
>> Documentation/githooks.txt | 7 +++
>> git-p4.py | 16 +++-
>> t/t9800-git-p4-basic.sh| 29 +
>> 4 files changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
>> index f0de3b891..a7aac1b92 100644
>> --- a/Documentation/git-p4.txt
>> +++ b/Documentation/git-p4.txt
>> @@ -374,6 +374,14 @@ These options can be used to modify 'git p4 submit'
>> behavior.
>> been submitted. Implies --disable-rebase. Can also be set with
>> git-p4.disableP4Sync. Sync with origin/master still goes ahead if
>> possible.
>>
>> +Hook for submit
>> +~~~
>> +The `p4-pre-submit` hook is executed if it exists and is executable.
>> +The hook takes no parameter and nothing from standard input. Exiting with
>> +non-zero status from this script prevents `git-p4 submit` from launching.
>> +
>> +One usage scenario is to run unit tests in the hook.
>> +
>> Rebase options
>> ~~
>> These options can be used to modify 'git p4 rebase' behavior.
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> index e3c283a17..22fcabbe2 100644
>> --- a/Documentation/githooks.txt
>> +++ b/Documentation/githooks.txt
>> @@ -485,6 +485,13 @@ The exit status determines whether git will use the
>> data from the
>> hook to limit its search. On error, it will fall back to verifying
>> all files and folders.
>>
>> +p4-pre-submit
>> +~
>> +
>> +This hook is invoked by `git-p4 submit`. It takes no parameter and nothing
>> +from standard input. Exiting with non-zero status from this script prevent
>> +`git-p4 submit` from launching. Run `git-p4 submit --help` for details.
>> +
>> GIT
>> ---
>> Part of the linkgit:git[1] suite
>> diff --git a/git-p4.py b/git-p4.py
>> index b449db1cc..879abfd2b 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -1494,7 +1494,13 @@ def __init__(self):
>> optparse.make_option("--disable-p4sync",
>> dest="disable_p4sync", action="store_true",
>> help="Skip Perforce sync of p4/master
>> after submit or shelve"),
>> ]
>> -self.description = "Submit changes from git to the perforce depot."
>> +self.description = """Submit changes from git to the perforce
>> depot.\n
>> +The `p4-pre-submit` hook is executed if it exists and is executable.
>> +The hook takes no parameter and nothing from standard input. Exiting
>> with
>> +non-zero status from this script prevents `git-p4 submit` from
>> launching.
>> +
>> +One usage scenario is to run unit tests in the hook."""
>> +
>> self.usage += " [name of git branch to submit into perforce depot]"
>> self.origin = ""
>> self.detectRenames = False
>> @@ -2303,6 +2309,14 @@ def run(self, args):
>> sys.exit("number of commits (%d) must match number of shelved
>> changelist (%d)" %
>> (len(commits), num_shelves))
>>
>> +hooks_path = gitConfig("core.hooksPath")
>> +if len(hooks_path) <= 0:
>> +hooks_path = os.path.join(os.environ.get("GIT_DIR", ".git"),
>> "hooks")
>> +
>> +hook_file = os.path.join(hooks_path, "p4-pre-submit")
>> +if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and
>> subprocess.call([hook_file]) != 0:
>> +sys.exit(1)
>> +
>> #
>> # Apply the commits, one at a time. On failure, ask if should
>> # continue to try the rest of the patches, or quit.
>> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
>> index 4849edc4e..2b7baa95d 100755
>> --- a/t/t9800-git-p4-basic.sh
>> +++ b/t/t9800-git-p4-basic.sh
>> @@ -261,6 +261,35 @@ test_expect_success 'unresolvable host in P4PORT should
>> display error' '
>> )
>> '
>>
>> +# Test fol
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
I think there is an error in the test harness. On 31 July 2018 at 10:46, SZEDER Gábor wrote: >> + test_must_fail git-p4 submit --dry-run >errs 2>&1 &&> >> + ! grep "Would apply" err It writes to the file "errs" but then looks for the message in "err". Luke
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
On 31 July 2018 at 16:40, Junio C Hamano wrote: > Luke Diamand writes: > >> I think there is an error in the test harness. >> >> On 31 July 2018 at 10:46, SZEDER Gábor wrote: >>>> + test_must_fail git-p4 submit --dry-run >errs 2>&1 &&> >>>> + ! grep "Would apply" err >> >> It writes to the file "errs" but then looks for the message in "err". >> >> Luke > > Sigh. Thanks for spotting, both of you. > > Here is what I'd locally squash in. If there is anything else, I'd > rather see a refreshed final one sent by the author. > > Thanks. > > t/t9800-git-p4-basic.sh | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh > index 2b7baa95d2..729cd25770 100755 > --- a/t/t9800-git-p4-basic.sh > +++ b/t/t9800-git-p4-basic.sh > @@ -274,19 +274,19 @@ test_expect_success 'run hook p4-pre-submit before > submit' ' > git add hello.txt && > git commit -m "add hello.txt" && > git config git-p4.skipSubmitEdit true && > - git-p4 submit --dry-run >out && > + git p4 submit --dry-run >out && > grep "Would apply" out && > mkdir -p .git/hooks && > write_script .git/hooks/p4-pre-submit <<-\EOF && > exit 0 > EOF > - git-p4 submit --dry-run >out && > + git p4 submit --dry-run >out && > grep "Would apply" out && > write_script .git/hooks/p4-pre-submit <<-\EOF && > exit 1 > EOF > - test_must_fail git-p4 submit --dry-run >errs 2>&1 && > - ! grep "Would apply" err > + test_must_fail git p4 submit --dry-run >errs 2>&1 && > + ! grep "Would apply" errs > ) > ' That set of deltas works for me. Sorry, I should have run the tests myself earlier and I would have picked up on this.
Re: git-p4: Clone p4 path with bidirectional integrations
On Mon, 19 Aug 2019 at 18:30, Aaron Miller wrote: > > Hi all, > > Is it possible to `git p4 clone --detect-branches` from a Perforce > path which contains bidirectional integrations? > > I've tried a bunch of things to get this to work, but here's an > example which hopefully illustrates what I'm trying to accomplish > and the issue I'm having. I have to admit I don't use the detect-branches code myself. It's possible that running with "-v" might give a bit more information. Can you write a test case, or even just a shell script, that might help figure out what's going on. Unfortunately Perforce doesn't really know about branches, so git-p4 has to make a guess! > > Perforce setup, assuming PWD is mapped to //depot/... in your client spec: > > 1. mkdir -p testing/master > 2. touch testing/master/test1 && p4 add testing/master/test1 && p4 submit > 3. p4 integrate //depot/testing/master/... > //depot/testing/staging/... && p4 submit > 3. touch testing/staging/test2 && p4 add testing/staging/test2 && p4 submit > 4. p4 integrate //depot/testing/staging/... > //depot/testing/master/... && p4 submit > > Now try to clone with git-p4: > > 1. git init p4_git_test && cd p4_git_test > 2. git config git-p4.branchList master:staging > 3. git config --add git-p4.branchList staging:master > 4. git p4 clone //depot/testing/...@all --detect-branches . > > You end up with a failure like: > > Importing from //depot/testing/...@all into . > Reinitialized existing Git repository in /home/amiller/p4_git_test/.git/ > Importing revision 1205832 (25%) > Importing new branch testing/master > > Resuming with change 1205832 > fatal: ambiguous argument 'refs/remotes/p4/testing/staging': unknown > revision or path not in the working tree. > Use '--' to separate paths from revisions, like this: > 'git [...] -- [...]' > Command failed: ['git', 'rev-list', '--reverse', '--no-merges', > 'refs/remotes/p4/testing/staging'] > > I'm using Git 2.22.1. > > Thanks, > Aaron
Re: [PATCH v2 1/1] git-p4: auto-delete named temporary file
On Tue, 27 Aug 2019 at 23:31, Junio C Hamano wrote:
>
> Andrey Mazo writes:
>
> > From: "Philip.McGraw"
> >
> > Avoid double-open exceptions on Windows platform when
> > calculating for lfs compressed size threshold
> > (git-p4.largeFileCompressedThreshold) comparisons.
> >
> > Take new approach using the NamedTemporaryFile()
> > file-like object as input to the ZipFile() which
> > auto-deletes after implicit close leaving with scope.
> >
> > Original code had double-open exception on Windows
> > platform because file still open from NamedTemporaryFile()
> > using generated filename instead of object.
> >
> > Thanks to Andrey for patiently suggesting several
> > iterations on this change for avoiding exceptions!
> >
> > Also print error details after resulting IOError to make
> > debugging cause of exception less mysterious when it has
> > nothing to do with "git version recent enough."
> >
> > Signed-off-by: Philip.McGraw
> > Reviewed-by: Andrey Mazo
> > ---
>
> Luke, does this look good?
>
> I know Mazo is the only other contributor who has multiple commits
> to git-p4.py in the past 2 years, to make Reviewed-by carry some
> weight ;-) but as we have so small number of people touching this
> script anyway, I'd rather see what the main contributor in the past
> 2 years thinks.
I think it looks reasonable.
Ack.
>
> Thanks.
>
> > git-p4.py | 13 ++---
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/git-p4.py b/git-p4.py
> > index c71a6832e2..33bdb14fd1 100755
> > --- a/git-p4.py
> > +++ b/git-p4.py
> > @@ -1158,17 +1158,15 @@ def exceedsLargeFileThreshold(self, relPath,
> > contents):
> > if gitConfigInt('git-p4.largeFileCompressedThreshold'):
> > contentsSize = sum(len(d) for d in contents)
> > if contentsSize <=
> > gitConfigInt('git-p4.largeFileCompressedThreshold'):
> > return False
> > contentTempFile = self.generateTempFile(contents)
> > -compressedContentFile =
> > tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
> > -zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
> > -zf.write(contentTempFile, compress_type=zipfile.ZIP_DEFLATED)
> > -zf.close()
> > -compressedContentsSize = zf.infolist()[0].compress_size
> > +compressedContentFile =
> > tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=True)
> > +with zipfile.ZipFile(compressedContentFile, mode='w') as zf:
> > +zf.write(contentTempFile,
> > compress_type=zipfile.ZIP_DEFLATED)
> > +compressedContentsSize = zf.infolist()[0].compress_size
> > os.remove(contentTempFile)
> > -os.remove(compressedContentFile.name)
> > if compressedContentsSize >
> > gitConfigInt('git-p4.largeFileCompressedThreshold'):
> > return True
> > return False
> >
> > def addLargeFile(self, relPath):
> > @@ -3512,12 +3510,13 @@ def importHeadRevision(self, revision):
> > details["time"] = res["time"]
> >
> > self.updateOptionDict(details)
> > try:
> > self.commit(details, self.extractFilesFromCommit(details),
> > self.branch)
> > -except IOError:
> > +except IOError as err:
> > print("IO error with git fast-import. Is your git version
> > recent enough?")
> > +print("IO error details: {}".format(err))
> > print(self.gitError.read())
> >
> > def openStreams(self):
> > self.importProcess = subprocess.Popen(["git", "fast-import"],
> >stdin=subprocess.PIPE,
> >
> > base-commit: 1feeaaf26bff51996f9f96c6dc41ca0f95ab5fc4
> > Pull-Request: https://github.com/gitgitgadget/git/pull/303
git-p4, and python2 EOL
We're coming up on when Python2 is end-of-lifed - we have until January 1st 2020. git-p4 uses python2, and doesn't work under python3 at all. The problem is the conversions between Python3 unicode strings and git (utf-8) and p4 (utf-8, except when it isn't). I had a go at fixing this here: https://github.com/luked99/git/commits/git-p4-python3-final-showdown You can see from the comments that I wasn't really finding it straightforward. I think I know a bit more about the problem now, but before I start having another go at fixing this, I wondered if anyone else had any thoughts on this, or even better, some time to spend on this. Thanks Luke
Re: [PATCH 1/1] git-p4: add format-patch subcommand
On 26 February 2018 at 23:48, Eric Sunshine wrote:
> On Mon, Feb 26, 2018 at 6:48 AM, Luke Diamand wrote:
>> It takes a list of P4 changelists and generates a patch for
>> each one, using "p4 describe".
>> [...]
Just to say that I almost got this working reasonably well, but it
gets pretty nasty making binary files work - in order to generate the
git binary format I need a base85 encoder which exists in Python3, but
not in Python2. And while I could obviously write it in Python easily
enough, it starts to feel like an awful lot of code implementing (and
maintaining) a slightly second-rate "git diff". And while it's
tempting to just not support binary files, in reality Perforce repos
seem to end up with lots of them.
So I think I might have another go at the previous scheme. What I will
have to do is to first create a faked-up commit representing the point
prior to the shelved commit for each file ("for file@rev-1 in files"),
and then create the real commit ("for file@rev in files"). It's
probably not as grim as it sounds and then means that we end up with
git doing all the hard work of diffing files, rather than relying on
Perforce's diff engine in conjunction with some Python.
>> Signed-off-by: Luke Diamand
>> ---
>> diff --git a/git-p4.py b/git-p4.py
>> @@ -3749,6 +3761,277 @@ class P4Branches(Command):
>> +class P4MakePatch(Command,P4UserMap):
>> +def run(self, args):
>> +if self.output and not os.path.isdir(self.output):
>> +sys.exit("output directory %s does not exist" % self.output)
>
> For consistency with "git format-patch", this could create the output
> directory automatically rather than erroring out.
>
>> +if self.strip_depot_prefix:
>> +self.clientSpec = getClientSpec()
>> +else:
>> +self.clientSpec = None
>> +
>> +self.loadUserMapFromCache()
>> +if len(args) < 1:
>> +return False
>
> Would it make sense to handle this no-arguments case earlier before
> doing work, such as loading the user map from cache?
>
>> +for change in args:
>> +self.make_patch(int(change))
>> +
>> +return True
>> +
>> +def p4_fetch_delta(self, change, files, shelved=False):
>> +cmd = ["describe"]
>> +if shelved:
>> +cmd += ["-S"]
>> +cmd += ["-du"]
>> +cmd += ["%s" % change]
>> +cmd = p4_build_cmd(cmd)
>> +
>> +p4 = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE)
>> +try:
>> +result = p4.stdout.readlines()
>> +except EOFError:
>> +pass
>> +in_diff = False
>> +matcher = re.compile('\s+(.*)#(\d+)\s+\(text\)\s+')
>> +diffmatcher = re.compile("Differences ...")
>
> I'm not familiar with the output of "p4 describe", but does it include
> user-supplied text? If so, would it be safer to anchor 'diffmatcher',
> for instance, "^Diferences...$"?
>
>> +delta = ""
>> +skip_next_blank_line = False
>> +
>> +for line in result:
>> +if diffmatcher.match(line):
>
> Stepping back, does "Differences..." appear on a line of its own? If
> so, why does this need to be a regex at all? Can't it just do a direct
> string comparison?
>
>> +in_diff = True
>> +continue
>> +
>> +if in_diff:
>> +
>> +if skip_next_blank_line and \
>> +line.rstrip() == "":
>> +skip_next_blank_line = False
>> +continue
>> +
>> +m = matcher.match(line)
>> +if m:
>> +file = self.map_path(m.group(1))
>> +ver = m.group(2)
>> +delta += "diff --git a%s b%s\n" % (file, file)
>> +delta += "--- a%s\n" % file
>> +delta += "+++ b%s\n" % file
>> +skip_next_blank_line = True
>> +else:
>> +delta += line
>> +
>> +delta += "\n"
>> +
>> +exitCode = p4.wait()
>> +if exitCode != 0:
>> +raise IOError("p4 '%s' command failed" % str(cmd))
>
> Since p4.stdout.readlines() gathered a
Re: [GSoC][PATCH] git-ci: use pylint to analyze the git-p4 code
On 13 March 2018 at 03:36, Viet Hung Tran wrote: > Hello Mr. Schneider, > > Here is my link for the passed CI build I tested on my fork: > https://travis-ci.org/VietHTran/git/builds/35210 > > In order to test .travis.yml configuration alone, I used a sample python > script call "test.py" that is already successfully tested on my computer > instead of using the git-p4.py like in the patch I submitted since the > git-p4.py file still reports a lot of errors when running pylint. > > It is possible to fix all the git-p4.py errors. However, I think it would > be best to fix each error separately with multiple commits (each should > fix a specific problem such as indentication, variable naming, etc.) > since there are a lot of changes needed to be made in the codes. Since the > microproject requires that I submit one patch, I decided to submit a patch > that includes changes in .travis.yml only. If you like, I can also submit the > patches addressing changes on the git-p4.py that fix the pylint errors. You can turn down the amount of messages it spews out, at which point it actually becomes quite useful. I had a quick go with the list of enabled checkers found here: https://github.com/ClusterHQ/flocker/blob/master/.pylintrc and it then gives about 40 warnings, all of which look like they could do with fixing. I think fixing them in a few patches would be OK, rather than submitting 40 separate patches. Quite a lot of the warnings are about the use of "file" - I've got a change I'm working on which fixes some of those already. Luke > > Thank you for feedback, > > Viet > > Signed-off-by: Viet Hung Tran > --- > .travis.yml | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/.travis.yml b/.travis.yml > index 5f5ee4f3b..581d75319 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -46,6 +46,16 @@ matrix: > - docker >before_install: >script: ci/run-linux32-docker.sh > +- env: jobname=Pylint > + compiler: > + addons: > +apt: > + packages: > + - python > + before_install: > + install: pip install --user pylint > + script: pylint git-p4.py > + after_failure: > - env: jobname=StaticAnalysis >os: linux >compiler: > -- > 2.16.2.440.gc6284da >
Re: git-p4: cloning with a change number does not import all files
On 2 December 2017 at 15:35, Patrick Rouleau wrote:
> On Fri, Dec 1, 2017 at 7:45 AM, Lars Schneider
> wrote:
>> Oh, I am with you. However, I only used git-p4 for a very short time in the
>> way you want to use it. Therefore, I don't have much experience in that kind
>> of usage pattern. I was able to convice my management to move all source to
>> Git and I used git-p4 to migrate the repositories ;-)
>>
>> Here is a talk on the topic if you want to learn more:
>> https://www.youtube.com/watch?v=QNixDNtwYJ0
>>
>> Cheers,
>> Lars
>
> Sadly, there is no way I convince the company to switch to git. They
> have acquired
> many companies in the past years and they have standardized around Perforce.
> It is in part because they want access control and they need to store
> a lot of binary
> files (including big VM images).
>
> I keep this video close to explain why I like git:
> https://www.youtube.com/watch?v=o4PFDKIc2fs
I feel your pain.
I think I've sort of stumbled across something like the problem you've
described in the past. Perhaps the files you need have been deleted
and then re-integrated or some such.
Would you be able to take a look at some files with this problem and
see if you can spot what's happened to it ("p4 changes" and perhaps
"p4 changes -i").
One thing that can certainly happen is that Perforce gets *very*
confused if you start getting too clever with symlinked directories,
and git-p4 can only do so much in the face of this. But it may be
something else.
Thanks
Luke
[PATCH 1/1] git-p4: update multiple shelved change lists
--update-shelve can now be specified multiple times on the
command-line, to update multiple shelved changelists in a single
submit.
This then means that a git patch series can be mirrored to a
sequence of shelved changelists, and (relatively easily) kept in
sync as changes are made in git.
Note that Perforce does not really support overlapping shelved
changelists where one change touches the files modified by
another. Trying to do this will result in merge conflicts.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 8 +++-
git-p4.py| 41 ++---
t/t9807-git-p4-submit.sh | 24 ++--
3 files changed, 47 insertions(+), 26 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 7436c64a9..d8c8f11c9 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -157,6 +157,12 @@ The p4 changes will be created as the user invoking 'git
p4 submit'. The
according to the author of the Git commit. This option requires admin
privileges in p4, which can be granted using 'p4 protect'.
+To shelve changes instead of submitting, use `--shelve` and `--update-shelve`:
+
+
+$ git p4 submit --shelve
+$ git p4 submit --update-shelve 1234 --update-shelve 2345
+
OPTIONS
---
@@ -310,7 +316,7 @@ These options can be used to modify 'git p4 submit'
behavior.
--update-shelve CHANGELIST::
Update an existing shelved changelist with this commit. Implies
- --shelve.
+ --shelve. Repeat for multiple shelved changelists.
--conflict=(ask|skip|quit)::
Conflicts can occur when applying a commit to p4. When this
diff --git a/git-p4.py b/git-p4.py
index 76859b453..7bb9cadc6 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1178,6 +1178,12 @@ class Command:
self.needsGit = True
self.verbose = False
+# This is required for the "append" cloneExclude action
+def ensure_value(self, attr, value):
+if not hasattr(self, attr) or getattr(self, attr) is None:
+setattr(self, attr, value)
+return getattr(self, attr)
+
class P4UserMap:
def __init__(self):
self.userMapFromPerforceServer = False
@@ -1343,9 +1349,10 @@ class P4Submit(Command, P4UserMap):
optparse.make_option("--shelve", dest="shelve",
action="store_true",
help="Shelve instead of submit. Shelved
files are reverted, "
"restoring the workspace to the state
before the shelve"),
-optparse.make_option("--update-shelve", dest="update_shelve",
action="store", type="int",
+optparse.make_option("--update-shelve", dest="update_shelve",
action="append", type="int",
metavar="CHANGELIST",
- help="update an existing shelved
changelist, implies --shelve")
+ help="update an existing shelved
changelist, implies --shelve, "
+ "repeat in-order for multiple
shelved changelists")
]
self.description = "Submit changes from git to the perforce depot."
self.usage += " [name of git branch to submit into perforce depot]"
@@ -1354,7 +1361,7 @@ class P4Submit(Command, P4UserMap):
self.preserveUser = gitConfigBool("git-p4.preserveUser")
self.dry_run = False
self.shelve = False
-self.update_shelve = None
+self.update_shelve = list()
self.prepare_p4_only = False
self.conflict_behavior = None
self.isWindows = (platform.system() == "Windows")
@@ -1809,9 +1816,10 @@ class P4Submit(Command, P4UserMap):
mode = filesToChangeExecBit[f]
setP4ExecBit(f, mode)
-if self.update_shelve:
-print("all_files = %s" % str(all_files))
-p4_reopen_in_change(self.update_shelve, all_files)
+update_shelve = 0
+if len(self.update_shelve) > 0:
+update_shelve = self.update_shelve.pop(0)
+p4_reopen_in_change(update_shelve, all_files)
#
# Build p4 change description, starting with the contents
@@ -1821,7 +1829,7 @@ class P4Submit(Command, P4UserMap):
logMessage = logMessage.strip()
(logMessage, jobs) = self.separate_jobs_from_description(logMessage)
-template = self.prepareSubmitTemplate(self.update_shelve)
+template = self.prepareSubmitTemplate(update_shelve)
submitTemplate = self.prepareLogMessage(template, logMessage, jobs)
if self.preserveUser:
@@ -1894,7 +1902,7 @@ class P4Submit(Command, P4UserMap
[PATCH 0/1] git-p4: update multiple shelved change lists
This change lets you update several P4 changelists in sequence. Say you have several git commits which are all somehow related. You would start by shelving them (e.g. for a review), something like this: git p4 submit --origin HEAD^2 --shelve You then make changes to these commits (in git) and now need to re-shelve them. Before this change you would need to cherry-pick each change onto a clean branch and do "git p4 --update-shelve", then remove the tip and repeat. With this change, you can just do: git p4 submit --origin HEAD^2 --update-shelve $CL1 --update-shelve $CL2 If the shelved changelists overlap (one changelist touches the same line as another) then this won't work, but that problem already exists with the --shelve option. Solving that is pretty hard to do as P4 really only understands files, not changes. Despite this shortcoming, it's very useful to be able to do update shelved changelists like this. Luke Diamand (1): git-p4: update multiple shelved change lists Documentation/git-p4.txt | 8 +++- git-p4.py| 41 ++--- t/t9807-git-p4-submit.sh | 24 ++-- 3 files changed, 47 insertions(+), 26 deletions(-) -- 2.15.0.276.g89ea799.dirty
git-p4 + watchman - watching the p4 repo?
Hi! I could be wrong about this, but I when I tried mixing watchman with git-p4, I found that on "git p4 submit" it ended up watching the p4 repo, which seems a bit pointless (and was also very slow). $ [create git-p4 clone of some p4 repo] $ : >bar $ git add bar && git commit -m 'adding bar' $ git p4 submit --origin HEAD^ --shelve Perforce checkout for depot path //depot/ located at /tmp/p4/cli/ Synchronizing p4 checkout... ... - file(s) up-to-date. Applying 4ce4057 change //depot/bar#1 - opened for edit Adding '/tmp/p4/cli' to watchman's watch list. Is there any way to stop it doing this? Thanks! Luke
[PATCH 0/3] git-p4: use symbolic-ref instead of name-rev
Followup to earlier discussion about use of name-rev in git-p4. http://marc.info/?l=git&m=148979063421355 Luke Diamand (3): git-p4: add failing test for name-rev rather than symbolic-ref git-p4: add read_pipe_text() internal function git-p4: don't use name-rev to get current branch git-p4.py| 38 +- t/t9807-git-p4-submit.sh | 16 2 files changed, 45 insertions(+), 9 deletions(-) -- 2.12.2.719.gcbd162c
[PATCH 3/3] git-p4: don't use name-rev to get current branch
git-p4 was using "git name-rev" to find out the current branch.
That is not safe, since if multiple branches or tags point at
the same revision, the result obtained might not be what is
expected.
Instead use "git symbolic-ref".
Signed-off-by: Luke Diamand
---
git-p4.py| 7 +--
t/t9807-git-p4-submit.sh | 2 +-
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 584b81775..8d151da91 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -602,12 +602,7 @@ def p4Where(depotPath):
return clientPath
def currentGitBranch():
-retcode = system(["git", "symbolic-ref", "-q", "HEAD"], ignore_error=True)
-if retcode != 0:
-# on a detached head
-return None
-else:
-return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
+return read_pipe_text(["git", "symbolic-ref", "--short", "-q", "HEAD"])
def isValidGitDir(path):
return git_dir(path) != None
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index ae05816e0..3457d5db6 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -139,7 +139,7 @@ test_expect_success 'submit with master branch name from
argv' '
)
'
-test_expect_failure 'allow submit from branch with same revision but different
name' '
+test_expect_success 'allow submit from branch with same revision but different
name' '
test_when_finished cleanup_git &&
git p4 clone --dest="$git" //depot &&
(
--
2.12.2.719.gcbd162c
[PATCH 1/3] git-p4: add failing test for name-rev rather than symbolic-ref
Using name-rev to find the current git branch means that git-p4 does not correctly get the current branch name if there are multiple branches pointing at HEAD, or a tag. This change adds a test case which demonstrates the problem. Configuring which branches are allowed to be submitted from goes wrong, as git-p4 gets confused about which branch is in use. This appears to be the only place that git-p4 actually cares about the current branch. Signed-off-by: Luke Diamand Signed-off-by: Junio C Hamano --- t/t9807-git-p4-submit.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh index e37239e65..ae05816e0 100755 --- a/t/t9807-git-p4-submit.sh +++ b/t/t9807-git-p4-submit.sh @@ -139,6 +139,22 @@ test_expect_success 'submit with master branch name from argv' ' ) ' +test_expect_failure 'allow submit from branch with same revision but different name' ' + test_when_finished cleanup_git && + git p4 clone --dest="$git" //depot && + ( + cd "$git" && + test_commit "file8" && + git checkout -b branch1 && + git checkout -b branch2 && + git config git-p4.skipSubmitEdit true && + git config git-p4.allowSubmit "branch1" && + test_must_fail git p4 submit && + git checkout branch1 && + git p4 submit + ) +' + # # Basic submit tests, the five handled cases # -- 2.12.2.719.gcbd162c
[PATCH 2/3] git-p4: add read_pipe_text() internal function
The existing read_pipe() function returns an empty string on
error, but also returns an empty string if the command returns
an empty string.
This leads to ugly constructions trying to detect error cases.
Add read_pipe_text() which just returns None on error.
Signed-off-by: Luke Diamand
---
git-p4.py | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index eab319d76..584b81775 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -160,17 +160,42 @@ def p4_write_pipe(c, stdin):
real_cmd = p4_build_cmd(c)
return write_pipe(real_cmd, stdin)
-def read_pipe(c, ignore_error=False):
+def read_pipe_full(c):
+""" Read output from command. Returns a tuple
+of the return status, stdout text and stderr
+text.
+"""
if verbose:
sys.stderr.write('Reading pipe: %s\n' % str(c))
expand = isinstance(c,basestring)
p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
shell=expand)
(out, err) = p.communicate()
-if p.returncode != 0 and not ignore_error:
-die('Command failed: %s\nError: %s' % (str(c), err))
+return (p.returncode, out, err)
+
+def read_pipe(c, ignore_error=False):
+""" Read output from command. Returns the output text on
+success. On failure, terminates execution, unless
+ignore_error is True, when it returns an empty string.
+"""
+(retcode, out, err) = read_pipe_full(c)
+if retcode != 0:
+if ignore_error:
+out = ""
+else:
+die('Command failed: %s\nError: %s' % (str(c), err))
return out
+def read_pipe_text(c):
+""" Read output from a command with trailing whitespace stripped.
+On error, returns None.
+"""
+(retcode, out, err) = read_pipe_full(c)
+if retcode != 0:
+return None
+else:
+return out.rstrip()
+
def p4_read_pipe(c, ignore_error=False):
real_cmd = p4_build_cmd(c)
return read_pipe(real_cmd, ignore_error)
--
2.12.2.719.gcbd162c
Re: [PATCH] git-p4: improve branch option handling
On 20 April 2017 at 14:52, Andrew Oakley wrote:
> It is sometimes useful (much quicker) to request commands only operate
> on a single branch.
>
> The P4Sync command has been updated to handle self.branch being None for
> consitency with the P4Submit.
Should that be consistency?
>
> The P4Rebase command has been given a branch option which is forwarded
> to the P4Sync command it runs.
>
> The P4Submit command has been simplified to not call P4Sync itself, it
> lets P4Rebase do it instead (now that the branch can be handled). This
> fixes an issue where P4Submit does a sync of the requested branch and
> then does a rebase which does a sync of all branches.
>
> Signed-off-by: Andrew Oakley
> ---
> Documentation/git-p4.txt | 4
> git-p4.py| 15 +++
> 2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index 7436c64..a03a291 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -328,6 +328,10 @@ Rebase options
> ~~
> These options can be used to modify 'git p4 rebase' behavior.
>
> +--branch ::
> + Sync this named branch instead of the default p4/master. See the
> + "Sync options" section above for more information.
> +
> --import-labels::
> Import p4 labels.
>
> diff --git a/git-p4.py b/git-p4.py
> index 8d151da..e58b34a 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2161,13 +2161,9 @@ class P4Submit(Command, P4UserMap):
> elif len(commits) == len(applied):
> print ("All commits {0}!".format(shelved_applied))
>
> -sync = P4Sync()
> -if self.branch:
> -sync.branch = self.branch
> -sync.run([])
> -
> rebase = P4Rebase()
> -rebase.rebase()
> +rebase.branch = self.branch
> +rebase.run([])
>
> else:
> if len(applied) == 0:
> @@ -2343,7 +2339,7 @@ class P4Sync(Command, P4UserMap):
> self.silent = False
> self.createdBranches = set()
> self.committedChanges = set()
> -self.branch = ""
> +self.branch = None
> self.detectBranches = False
> self.detectLabels = False
> self.importLabels = False
> @@ -3281,7 +3277,7 @@ class P4Sync(Command, P4UserMap):
> system("git fetch origin")
>
> branch_arg_given = bool(self.branch)
> -if len(self.branch) == 0:
> +if not branch_arg_given:
> self.branch = self.refPrefix + "master"
> if gitBranchExists("refs/heads/p4") and self.importIntoRemotes:
> system("git update-ref %s refs/heads/p4" % self.branch)
> @@ -3567,14 +3563,17 @@ class P4Rebase(Command):
> def __init__(self):
> Command.__init__(self)
> self.options = [
> +optparse.make_option("--branch", dest="branch"),
> optparse.make_option("--import-labels", dest="importLabels",
> action="store_true"),
> ]
> +self.branch = None
> self.importLabels = False
> self.description = ("Fetches the latest revision from perforce and "
> + "rebases the current work (branch) against it")
>
> def run(self, args):
> sync = P4Sync()
> +sync.branch = self.branch
> sync.importLabels = self.importLabels
> sync.run([])
>
> --
> 2.10.2
>
Apart from the typo above, this looks sensible to me. Would you be
able to add a test case?
Thanks!
Luke
Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key
On 17 April 2018 at 20:12, Thandesha VK wrote:
> I have few cases where even p4 -G sizes (or p4 sizes) is not returning
> the size value even with latest version of p4 (17.2). In that case, we
> have to regenerate the digest for file save it - It mean something is
> wrong with the file in perforce.
Just to be clear - git-p4 does not use the p4 "sizes" command anywhere AFAIK.
We are just talking about the output from "p4 print" and the
"fileSize" key, right?
Does that happen with the 17.2 version of p4?
> Regarding, sys.stdout.write v/s print, I see script using both of them
> without a common pattern. I can change it to whatever is more
> appropriate.
print() probably makes more sense; can we try to use the function form
so that we don't deliberately make the path to python3 harder (albeit
in a very tiny way).
>
> On Tue, Apr 17, 2018 at 11:47 AM, Mazo, Andrey wrote:
>> Does a missing "fileSize" actually mean that there is something wrong with
>> the file?
>> Because, for me, `p4 -G print` doesn't print "fileSize" for _any_ file.
>> (which I attribute to our rather ancient (2007.2) Perforce server)
>> I'm not an expert in Perforce, so don't know for sure.
My 2015 version of p4d reports a fileSize.
>>
>> However, `p4 -G sizes` works fine even with our p4 server.
>> Should we then go one step further and use `p4 -G sizes` to obtain the
>> "fileSize" when it's not returned by `p4 -G print`?
>> Or is it an overkill for a simple verbose print out?
If your server isn't reporting "fileSize" then there are a few other
places where I would expect git-p4 to also fail.
If we're going to support this very ancient version of p4d, then
gracefully handling a missing fileSize will be useful.
>>
>> Also, please, find one comment inline below.
>>
>> Thank you,
>> Andrey
>>
>> From: Thandesha VK
>>> Sounds good. How about an enhanced version of fix from both of us.
>>> This will let us know that something is not right with the file but
>>> will not bark
>>>
>>> $ git diff
>>> diff --git a/git-p4.py b/git-p4.py
>>> index 7bb9cadc6..df901976f 100755
>>> --- a/git-p4.py
>>> +++ b/git-p4.py
>>> @@ -2566,7 +2566,12 @@ class P4Sync(Command, P4UserMap):
>>> relPath = self.stripRepoPath(file['depotFile'],
>>> self.branchPrefixes)
>>> relPath = self.encodeWithUTF8(relPath)
>>> if verbose:
>>> -size = int(self.stream_file['fileSize'])
>>> +if 'fileSize' not in self.stream_file:
>>> + print "WARN: File size from perforce unknown. Please verify
>>> by p4 sizes %s" %(file['depotFile'])
>> For whatever reason, the code below uses sys.stdout.write() instead of
>> print().
>> Should it be used here for consistency as well?
>>
>>> + size = "-1"
>>> +else:
>>> + size = self.stream_file['fileSize']
>>> +size = int(size)
>>> sys.stdout.write('\r%s --> %s (%i MB)\n' %
>>> (file['depotFile'], relPath, size/1024/1024))
>>> sys.stdout.flush()
>>>
>>>
>>> On Tue, Apr 17, 2018 at 10:33 AM, Mazo, Andrey wrote:
Sure, I totally agree.
Sorry, I just wasn't clear enough in my previous email.
I meant that your patch suppresses "%s --> %s (%i MB)" line in case
"fileSize" is not available,
while my patch suppresses just "(%i MB)" portion if the "fileSize" is not
known.
In other words,
* if "fileSize" is known:
** both yours and mine patches don't change existing behavior;
* if "fileSize" is not known:
** your patch makes streamOneP4File() not print anything;
** my patch makes streamOneP4File() print "%s --> %s".
Hope, I'm clearer this time.
Thank you,
Andrey
From: Thandesha VK
> *I* think keeping the filesize info is better with --verbose option as
> that gives some clue about the file we are working on. What do you
> think?
> Script has similar checks of key existence at other places where it is
> looking for fileSize.
>
> On Tue, Apr 17, 2018 at 9:22 AM, Andrey Mazo wrote:
>> Huh, I actually have a slightly different fix for the same issue.
>> It doesn't suppress the corresponding verbose output completely, but
>> just removes the size information from it.
>>
>> Also, I'd mention that the workaround is trivial -- simply omit the
>> "--verbose" option.
>>
>> Andrey Mazo (1):
>> git-p4: fix `sync --verbose` traceback due to 'fileSize'
>>
>> git-p4.py | 8 ++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
Thanks
Luke
Re: [PATCH] git-p4 - Add option --sha1 to submit in p4
On 2 May 2018 at 15:32, Merland Romain wrote:
> From 4867808cad2b759ebf31c275356e602b72c5659f Mon Sep 17 00:00:00 2001
> From: Romain Merland
> To: git@vger.kernel.org
> Cc: Junio C Hamano , Jeff King , Luke
> Diamand , Vinicius Kursancew
> Date: Wed, 2 May 2018 15:02:11 +0200
> Subject: [PATCH] git-p4 - Add option --sha1 to submit in p4
>
> Add option --sha1 to command 'git-p4 submit' in order to submit in p4 a
> commit
> that is not necessarily on master.
> In that case, don't rebase the submitted changes.
That could be very useful, I often find the commit I want to submit is
half-way down a long list of other commits.
Currently I end up cherry-picking the one I want into a clean repo,
but that's much more awkward than your --sha1 change.
A few comments inline:
> Signed-off-by: Romain Merland
> ---
> git-p4.py | 21 +++--
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 7bb9cadc6..d64ff79dd 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1352,7 +1352,9 @@ class P4Submit(Command, P4UserMap):
> optparse.make_option("--update-shelve",
> dest="update_shelve", action="append", type="int",
> metavar="CHANGELIST",
> help="update an existing shelved
> changelist, implies --shelve, "
> - "repeat in-order for multiple
> shelved changelists")
> + "repeat in-order for multiple
> shelved changelists"),
> +optparse.make_option("--sha1", dest="sha1", metavar="SHA1",
> + help="submit only the specified
> commit, don't rebase afterwards")
Is there a better name than "sha1" ? If git ever changes its hash to
something else will this still make sense?
I wonder why you wouldn't rebase afterwards?
Perhaps an additional option to skip the rebase?
> ]
> self.description = "Submit changes from git to the perforce depot."
> self.usage += " [name of git branch to submit into perforce depot]"
> @@ -1362,6 +1364,7 @@ class P4Submit(Command, P4UserMap):
> self.dry_run = False
> self.shelve = False
> self.update_shelve = list()
> +self.sha1 = ""
> self.prepare_p4_only = False
> self.conflict_behavior = None
> self.isWindows = (platform.system() == "Windows")
> @@ -2103,9 +2106,12 @@ class P4Submit(Command, P4UserMap):
> else:
> commitish = 'HEAD'
>
> -for line in read_pipe_lines(["git", "rev-list", "--no-merges",
> "%s..%s" % (self.origin, commitish)]):
> -commits.append(line.strip())
> -commits.reverse()
> +if self.sha1 != "":
> +commits.append(self.sha1)
> +else:
> +for line in read_pipe_lines(["git", "rev-list", "--no-merges",
> "%s..%s" % (self.origin, commitish)]):
> +commits.append(line.strip())
> +commits.reverse()
>
> if self.preserveUser or gitConfigBool("git-p4.skipUserNameCheck"):
> self.checkAuthorship = False
> @@ -2215,8 +2221,11 @@ class P4Submit(Command, P4UserMap):
> sync.branch = self.branch
> sync.run([])
>
> -rebase = P4Rebase()
> -rebase.rebase()
> +if self.sha1 == "":
if not self.skip_rebase:
> +rebase = P4Rebase()
> +rebase.rebase()
> +else:
> +print "You will have to do 'git p4 sync' and rebase."
>
> else:
> if len(applied) == 0:
> --
> 2.17.0
>
>
This would be better with some documentation in git-p4.txt and a test case!
Regards!
Luke
Re: [PATCH] git-p4 - Add option --sha1 to submit in p4
On 2 May 2018 at 16:51, Merland Romain wrote: > Thanks Luke, > > Following your comments, I will: > - change the option name to --commit if it suits you Seems like a good name. > - add an option --force-rebase which defaults to false. Setting it to true > will rebase even with --commit option used. Or "--disable-rebase" ? I think there's no reason you couldn't rebase after doing a commit like this is there? And "--disable-rebase" would be useful generally, not just with the --commit option, but also with the regular options. "--force-rebase" is a bit confusing since for the normal flow, it always rebases anyway, and there's no need to force! But you're the one who is using this in anger, so your opinion likely counts for more than mine! > it can be useful too if some commits have been skipped and user wants to > rebase anyway > - add an entry in the documentation > - (learn how to create a test and) add tests for these specific cases To create a test just look in t/ for the t98*.sh files. Cut-n-paste one or add to an existing one. Send an email here if you get stuck (but it's pretty straightforward). You can run only the git-p4 tests with: $ (cd t && make T=t98* -j) > > What is the best practice ? to send a new email with the new diff ? to > continue this discussion ? I think either, probably a new email is easiest. See Documentation/SubmittingPatches for the general process. Regards! Luke
Re: [PATCH] git-p4: add options --commit and --disable-rebase
On 8 May 2018 at 01:37, Merland Romain wrote:
> From f5229be8e2a3340af853227929818940323a8062 Mon Sep 17 00:00:00 2001
> From: Romain Merland
> Date: Tue, 8 May 2018 02:03:11 +0200
> Subject: [PATCH] git-p4: add options --commit and --disable-rebase
> To: git@vger.kernel.org
> Cc: Luke Diamand ,
> Junio C Hamano ,
> Matthieu Moy ,
> Vinicius Kursancew ,
> Jeff King ,
> Cedric Borgese ,
> Fabien Boutantin
> Thanks-to: Cedric Borgese
> Signed-off-by: Romain Merland
>
> On a daily work with multiple local git branches, the usual way to submit
> only a
> specified commit was to cherry-pick the commit on master then run git-p4
> submit.
> It can be very annoying to switch between local branches and master, only to
> submit one commit.
> The proposed new way is to select directly the commit you want to submit.
>
> add option --commit to command 'git-p4 submit' in order to submit only
> specified commit(s) in p4.
>
> On a daily work developping software with big compilation time, one may not
> want
> to rebase on his local git tree, in order to avoid long recompilation.
>
> add option --disable-rebase to command 'git-p4 submit' in order to disable
> rebase after submission.
Looks good to me.
I just tried it (shelving a change about 3 commits down) and it works
like a charm!
The "--disable-rebase" will also be very useful (I wonder whether a
git-config option might be a useful addition at a later date).
(You might need to resubmit your change as plain text via "git
format-patch" and "git send-email" as I'm not sure if Junio will
accept a mime-encoded patch).
Thanks
Luke
> ---
> Documentation/git-p4.txt | 14 ++
> git-p4.py| 29 +++--
> t/t9807-git-p4-submit.sh | 40
> 3 files changed, 77 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index d8c8f11c9..88d109deb 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -149,6 +149,12 @@ To specify a branch other than the current one, use:
> $ git p4 submit topicbranch
>
>
> +To specify a single commit or a range of commits, use:
> +
> +$ git p4 submit --commit
> +$ git p4 submit --commit
> +
> +
> The upstream reference is generally 'refs/remotes/p4/master', but can
> be overridden using the `--origin=` command-line option.
>
> @@ -330,6 +336,14 @@ These options can be used to modify 'git p4 submit'
> behavior.
> p4/master. See the "Sync options" section above for more
> information.
>
> +--commit |::
> +Submit only the specified commit or range of commits, instead of the
> full
> +list of changes that are in the current Git branch.
> +
> +--disable-rebase::
> +Disable the automatic rebase after all commits have been successfully
> +submitted.
> +
> Rebase options
> ~~
> These options can be used to modify 'git p4 rebase' behavior.
> diff --git a/git-p4.py b/git-p4.py
> index 7bb9cadc6..f4a6f3b4c 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1352,7 +1352,12 @@ class P4Submit(Command, P4UserMap):
> optparse.make_option("--update-shelve",
> dest="update_shelve", action="append", type="int",
> metavar="CHANGELIST",
> help="update an existing shelved
> changelist, implies --shelve, "
> - "repeat in-order for multiple
> shelved changelists")
> + "repeat in-order for multiple
> shelved changelists"),
> +optparse.make_option("--commit", dest="commit",
> metavar="COMMIT",
> + help="submit only the specified
> commit(s), one commit or xxx..xxx"),
> +optparse.make_option("--disable-rebase",
> dest="disable_rebase", action="store_true",
> + help="Disable rebase after submit is
> completed. Can be useful if you "
> + "work from a local git branch that is
> not master")
> ]
> self.description = "Submit changes from git to the perforce depot."
> self.usage += " [name of git branch to submit into perforce depot]"
> @@ -1362,6 +1367,8 @@ class P4Submit(Command, P4UserM
Re: [PATCH v4 3/6] git-p4: change "commitish" typo to "committish"
On 10 May 2018 at 13:43, Ævar Arnfjörð Bjarmason wrote:
> This was the only occurrence of "commitish" in the tree, but as the
> log will reveal we've had others in the past. Fixes up code added in
> 00ad6e3182 ("git-p4: work with a detached head", 2015-11-21).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason
Looks good to me!
Thanks,
Luke
> ---
> git-p4.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 7bb9cadc69..1afa87cd9d 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2099,11 +2099,11 @@ class P4Submit(Command, P4UserMap):
>
> commits = []
> if self.master:
> -commitish = self.master
> +committish = self.master
> else:
> -commitish = 'HEAD'
> +committish = 'HEAD'
>
> -for line in read_pipe_lines(["git", "rev-list", "--no-merges",
> "%s..%s" % (self.origin, commitish)]):
> +for line in read_pipe_lines(["git", "rev-list", "--no-merges",
> "%s..%s" % (self.origin, committish)]):
> commits.append(line.strip())
> commits.reverse()
>
> --
> 2.17.0.410.g4ac3413cc8
>
[PATCH 1/1] git-p4: add unshelve command
This can be used to "unshelve" a shelved P4 commit into
a git commit.
For example:
$ git p4 unshelve 12345
The resulting commit ends up in the branch:
refs/remotes/p4/unshelved/12345
If that branch already exists, it is renamed - for example
the above branch would be saved as p4/unshelved/12345.1.
Caveat:
The unshelving is done against the current "p4/master" branch;
git-p4 uses "p4 print" to get the file contents at the requested
revision, and then fast-import creates a commit relative to p4/master.
Ideally what you would want is for fast-import to create the
commit based on the Perforce "revision" prior to the shelved commit,
but Perforce doesn't have such a concept - to do this, git-p4
would need to figure out the revisions of the individual files
before the shelved changelist, and then construct a temporary
git branch which matched this.
It's possible to do this, but doing so makes this change a lot more
complicated.
This limitation means that if you unshelve a change where some
of the changed files were not based on p4/master, you will get
an amalgam of the change you wanted, and these other changes.
The reference branch can be changed manually with the "--origin"
option.
The change adds a new Unshelve command class. This just runs the
existing P4Sync code tweaked to handle a shelved changelist.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 26 ++
git-p4.py| 171 ++-
t/t9832-unshelve.sh | 99 +++
3 files changed, 260 insertions(+), 36 deletions(-)
create mode 100755 t/t9832-unshelve.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index d8c8f11c9f..2d768eec10 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -164,6 +164,25 @@ $ git p4 submit --shelve
$ git p4 submit --update-shelve 1234 --update-shelve 2345
+
+Unshelve
+
+Unshelving will take a shelved P4 changelist, and produce the equivalent git
commit
+in the branch refs/remotes/p4/unshelved/.
+
+The git commit is created relative to the current p4/master, so if this
+is behind Perforce itself, it may include more changes than you expected. You
can
+change the reference branch with the "--origin" option.
+
+If the target branch in refs/remotes/p4/unshelved already exists, the old one
will
+be renamed.
+
+
+$ git p4 sync
+$ git p4 unshelve 12345
+$ git show refs/remotes/p4/unshelved/12345
+
+
OPTIONS
---
@@ -337,6 +356,13 @@ These options can be used to modify 'git p4 rebase'
behavior.
--import-labels::
Import p4 labels.
+Unshelve options
+
+
+--origin::
+Sets the git refspec against which the shelved P4 changelist is compared.
+Defaults to p4/master.
+
DEPOT PATH SYNTAX
-
The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69..dcf6dc9f4f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -316,12 +316,17 @@ def p4_last_change():
results = p4CmdList(["changes", "-m", "1"], skip_info=True)
return int(results[0]['change'])
-def p4_describe(change):
+def p4_describe(change, shelved=False):
"""Make sure it returns a valid result by checking for
the presence of field "time". Return a dict of the
results."""
-ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
+cmd = ["describe", "-s"]
+if shelved:
+cmd += ["-S"]
+cmd += [str(change)]
+
+ds = p4CmdList(cmd, skip_info=True)
if len(ds) != 1:
die("p4 describe -s %d did not return 1 result: %s" % (change,
str(ds)))
@@ -662,6 +667,12 @@ def gitBranchExists(branch):
stderr=subprocess.PIPE, stdout=subprocess.PIPE);
return proc.wait() == 0;
+def gitUpdateRef(ref, newvalue):
+subprocess.check_call(["git", "update-ref", ref, newvalue])
+
+def gitDeleteRef(ref):
+subprocess.check_call(["git", "update-ref", "-d", ref])
+
_gitConfig = {}
def gitConfig(key, typeSpecifier=None):
@@ -2411,6 +2422,7 @@ class P4Sync(Command, P4UserMap):
self.tempBranches = []
self.tempBranchLocation = "refs/git-p4-tmp"
self.largeFileSystem = None
+self.suppress_meta_comment = False
if gitConfig('git-p4.largeFileSystem'):
largeFileSystemConstructor =
globals()[gitConfig('git-p4.largeFileSystem')]
@@ -2421,6 +2433,18 @@ class P4Sync(Command, P4UserMap):
if gitConfig("git-p4.syncFromOrigin") == "false":
self.syncWithOrigin = False
+self.depotPaths = []
+self.changeR
[PATCHv2 0/1] add git-p4 unshelve command
This is another attempt to make a "git p4 unshelve" command. Unshelving in p4 is a bit like a cross between cherry-pick and "am", and is very commonly used for review. This command helps git users who want to try out a shelved p4 change from some other repo: e.g. $ git p4 unshelve 12345 unshelved CL12345 into refs/remotes/p4/unshelved/12345 $ git show refs/remotes/p4/unshelved/12345 I abandoned an earlier attempt because it seemed like there is no way to get around a rather nasty problem: git-p4 just constructs the commit and passes the file contents to git-fastimport. But there's no easy way to construct the *prior* commit, because Perforce doesn't record this information, and so you can end up with other changes mixed into the unshelved commit - these are the differences between your tree and the other tree, for each file that has been modified. However, I think the command is sufficiently useful that it's worth supporting anyway, even with that caveat. I also tried to use "p4 describe" to get the deltas, but that's very unsatisfactory: I found myself writing a second-rate version of git's diff tool to try to make up for the deficiencies in Perforce's diff tool. It might be possible to reconstruct the missing base commit information, but that's a reasonably tricky task. I have incorporated some of the comments from the earlier review rounds, in particular: - no longer adds the [git-p4...] annotation in unshelve - try to use .format() in place of % - rename the target branch if it already exists Luke Diamand (1): git-p4: add unshelve command Documentation/git-p4.txt | 26 ++ git-p4.py| 171 ++- t/t9832-unshelve.sh | 99 +++ 3 files changed, 260 insertions(+), 36 deletions(-) create mode 100755 t/t9832-unshelve.sh -- 2.17.0.392.gdeb1a6e9b7
[PATCHv3 0/1] git-p4 unshelve
This is a git-p4 unshelve command which detects when extra changes are going to be included, and refuses to unhshelve. As in my earlier unshelve change, this uses git fast-import to do the actual delta generation, but now checks to see if the files unshelved are based on the same revisions that fast-import will be using, using the revision numbers in the "p4 describe -S" command output. If they're different, it refuses to unshelve. That makes it safe to use, rather than potentially generating deltas that contain bits from other changes. I have added a test for this case. Luke Diamand (1): git-p4: add unshelve command Documentation/git-p4.txt | 32 ++ git-p4.py| 207 --- t/t9832-unshelve.sh | 153 + 3 files changed, 356 insertions(+), 36 deletions(-) create mode 100755 t/t9832-unshelve.sh -- 2.17.0.392.gdeb1a6e9b7
[PATCHv3 1/1] git-p4: add unshelve command
This can be used to "unshelve" a shelved P4 commit into
a git commit.
For example:
$ git p4 unshelve 12345
The resulting commit ends up in the branch:
refs/remotes/p4/unshelved/12345
If that branch already exists, it is renamed - for example
the above branch would be saved as p4/unshelved/12345.1.
git-p4 checks that the shelved changelist is based on files
which are at the same Perforce revision as the origin branch
being used for the unshelve (HEAD by default). If they are not,
it will refuse to unshelve. This is to ensure that the unshelved
change does not contain other changes mixed-in.
The reference branch can be changed manually with the "--origin"
option.
The change adds a new Unshelve command class. This just runs the
existing P4Sync code tweaked to handle a shelved changelist.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 32 ++
git-p4.py| 207 ---
t/t9832-unshelve.sh | 153 +
3 files changed, 356 insertions(+), 36 deletions(-)
create mode 100755 t/t9832-unshelve.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index d8c8f11c9f..d3cb249fc2 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -164,6 +164,31 @@ $ git p4 submit --shelve
$ git p4 submit --update-shelve 1234 --update-shelve 2345
+
+Unshelve
+
+Unshelving will take a shelved P4 changelist, and produce the equivalent git
commit
+in the branch refs/remotes/p4/unshelved/.
+
+The git commit is created relative to the current origin revision (HEAD by
default).
+If the shelved changelist's parent revisions differ, git-p4 will refuse to
unshelve;
+you need to be unshelving onto an equivalent tree.
+
+The origin revision can be changed with the "--origin" option.
+
+If the target branch in refs/remotes/p4/unshelved already exists, the old one
will
+be renamed.
+
+
+$ git p4 sync
+$ git p4 unshelve 12345
+$ git show refs/remotes/p4/unshelved/12345
+
+$ git p4 unshelve 12345
+
+
+
+
OPTIONS
---
@@ -337,6 +362,13 @@ These options can be used to modify 'git p4 rebase'
behavior.
--import-labels::
Import p4 labels.
+Unshelve options
+
+
+--origin::
+Sets the git refspec against which the shelved P4 changelist is compared.
+Defaults to p4/master.
+
DEPOT PATH SYNTAX
-
The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69..dcff31a1d4 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -316,12 +316,17 @@ def p4_last_change():
results = p4CmdList(["changes", "-m", "1"], skip_info=True)
return int(results[0]['change'])
-def p4_describe(change):
+def p4_describe(change, shelved=False):
"""Make sure it returns a valid result by checking for
the presence of field "time". Return a dict of the
results."""
-ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
+cmd = ["describe", "-s"]
+if shelved:
+cmd += ["-S"]
+cmd += [str(change)]
+
+ds = p4CmdList(cmd, skip_info=True)
if len(ds) != 1:
die("p4 describe -s %d did not return 1 result: %s" % (change,
str(ds)))
@@ -662,6 +667,12 @@ def gitBranchExists(branch):
stderr=subprocess.PIPE, stdout=subprocess.PIPE);
return proc.wait() == 0;
+def gitUpdateRef(ref, newvalue):
+subprocess.check_call(["git", "update-ref", ref, newvalue])
+
+def gitDeleteRef(ref):
+subprocess.check_call(["git", "update-ref", "-d", ref])
+
_gitConfig = {}
def gitConfig(key, typeSpecifier=None):
@@ -2411,6 +2422,7 @@ class P4Sync(Command, P4UserMap):
self.tempBranches = []
self.tempBranchLocation = "refs/git-p4-tmp"
self.largeFileSystem = None
+self.suppress_meta_comment = False
if gitConfig('git-p4.largeFileSystem'):
largeFileSystemConstructor =
globals()[gitConfig('git-p4.largeFileSystem')]
@@ -2421,6 +2433,18 @@ class P4Sync(Command, P4UserMap):
if gitConfig("git-p4.syncFromOrigin") == "false":
self.syncWithOrigin = False
+self.depotPaths = []
+self.changeRange = ""
+self.previousDepotPaths = []
+self.hasOrigin = False
+
+# map from branch depot path to parent branch
+self.knownBranches = {}
+self.initialParents = {}
+
+self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone %
3600) / 60))
+self.labels = {}
+
# Force a checkpoint in fast-import and wait for it to finish
def checkpoint(self):
self.git
Re: [PATCH 1/1] git-p4: add unshelve command
something_to_commit:
> +print "Nothing to commit. Exiting"
> +return True
> +
> +# get the diff and copy to diff directory
> +for f in editedFiles:
> +p4diff = p4_read_pipe(['diff2', '-du',
> f["depotFile"]+'#'+f["rev"], f["depotFile"]+'@='+self.unshelve])
> +p4diff = "\n".join(p4diff.split("\n")[1:])
> +p4diff = '--- '+f["gitFile"]+'\n' + '+++ '+f["gitFile"]+'\n'
> + p4diff
> +write_pipe(['patch', '-d/', '-p0'], p4diff)
> +write_pipe(['git', 'add', '-f', f["gitFile"]], "")
> + for f in filesToAdd:
> +p4_write_pipe(['print', '-o', f["gitFile"],
> f["depotFile"]+'@='+self.unshelve], "")
> +write_pipe(['git', 'add', '-f', f["gitFile"]], "")
> +for f in filesToDelete:
> +os.remove(f["gitFile"])
> +write_pipe(['git', 'rm', f["gitFile"]], "")
> +for f in binaryFiles:
> +p4_write_pipe(['print', '-o', f["gitFile"],
> f["depotFile"]+'@='+self.unshelve], "")
> +write_pipe(['git', 'add', '-f', f["gitFile"]], "")
> +
> +# finalize: commit in git
> +write_pipe(['git', 'commit', '-m', description["desc"]], "")
> +return True
> +
> print "Perforce checkout for depot path %s located at %s" %
> (self.depotPath, self.clientPath)
> self.oldWorkingDirectory = os.getcwd()
>
> Romain
>
>
>
> Le samedi 12 mai 2018 à 23:24:48 UTC+2, Luke Diamand a
> écrit :
>
>
>
>
>
> This can be used to "unshelve" a shelved P4 commit into
> a git commit.
>
> For example:
>
> $ git p4 unshelve 12345
>
> The resulting commit ends up in the branch:
> refs/remotes/p4/unshelved/12345
>
> If that branch already exists, it is renamed - for example
> the above branch would be saved as p4/unshelved/12345.1.
>
> Caveat:
>
> The unshelving is done against the current "p4/master" branch;
> git-p4 uses "p4 print" to get the file contents at the requested
> revision, and then fast-import creates a commit relative to p4/master.
>
> Ideally what you would want is for fast-import to create the
> commit based on the Perforce "revision" prior to the shelved commit,
> but Perforce doesn't have such a concept - to do this, git-p4
> would need to figure out the revisions of the individual files
> before the shelved changelist, and then construct a temporary
> git branch which matched this.
>
> It's possible to do this, but doing so makes this change a lot more
> complicated.
>
> This limitation means that if you unshelve a change where some
> of the changed files were not based on p4/master, you will get
> an amalgam of the change you wanted, and these other changes.
>
> The reference branch can be changed manually with the "--origin"
> option.
>
> The change adds a new Unshelve command class. This just runs the
> existing P4Sync code tweaked to handle a shelved changelist.
>
> Signed-off-by: Luke Diamand
Re: [PATCH] git-p4: add options --commit and --disable-rebase
On 9 May 2018 at 16:32, Romain Merland wrote:
> On a daily work with multiple local git branches, the usual way to submit
> only a
> specified commit was to cherry-pick the commit on master then run git-p4
> submit.
> It can be very annoying to switch between local branches and master, only to
> submit one commit.
> The proposed new way is to select directly the commit you want to submit.
>
> add option --commit to command 'git-p4 submit' in order to submit only
> specified commit(s) in p4.
>
> On a daily work developping software with big compilation time, one may not
> want
> to rebase on his local git tree, in order to avoid long recompilation.
>
> add option --disable-rebase to command 'git-p4 submit' in order to disable
> rebase after submission.
I've been using this for real and it works well for me. Ack.
Because of the way I'm using git-p4, the --disable-rebase option
doesn't really help me - I really need a --disable-sync option but
that's a different feature.
Thanks
Luke
> ---
> Documentation/git-p4.txt | 14 ++
> git-p4.py| 29 +++--
> t/t9807-git-p4-submit.sh | 40
> 3 files changed, 77 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index d8c8f11c9..88d109deb 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -149,6 +149,12 @@ To specify a branch other than the current one, use:
> $ git p4 submit topicbranch
>
>
> +To specify a single commit or a range of commits, use:
> +
> +$ git p4 submit --commit
> +$ git p4 submit --commit
> +
> +
> The upstream reference is generally 'refs/remotes/p4/master', but can
> be overridden using the `--origin=` command-line option.
>
> @@ -330,6 +336,14 @@ These options can be used to modify 'git p4 submit'
> behavior.
> p4/master. See the "Sync options" section above for more
> information.
>
> +--commit |::
> +Submit only the specified commit or range of commits, instead of the full
> +list of changes that are in the current Git branch.
> +
> +--disable-rebase::
> +Disable the automatic rebase after all commits have been successfully
> +submitted.
> +
> Rebase options
> ~~
> These options can be used to modify 'git p4 rebase' behavior.
> diff --git a/git-p4.py b/git-p4.py
> index 7bb9cadc6..f4a6f3b4c 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1352,7 +1352,12 @@ class P4Submit(Command, P4UserMap):
> optparse.make_option("--update-shelve",
> dest="update_shelve", action="append", type="int",
> metavar="CHANGELIST",
> help="update an existing shelved
> changelist, implies --shelve, "
> - "repeat in-order for multiple
> shelved changelists")
> + "repeat in-order for multiple
> shelved changelists"),
> +optparse.make_option("--commit", dest="commit",
> metavar="COMMIT",
> + help="submit only the specified
> commit(s), one commit or xxx..xxx"),
> +optparse.make_option("--disable-rebase",
> dest="disable_rebase", action="store_true",
> + help="Disable rebase after submit is
> completed. Can be useful if you "
> + "work from a local git branch that is
> not master")
> ]
> self.description = "Submit changes from git to the perforce depot."
> self.usage += " [name of git branch to submit into perforce depot]"
> @@ -1362,6 +1367,8 @@ class P4Submit(Command, P4UserMap):
> self.dry_run = False
> self.shelve = False
> self.update_shelve = list()
> +self.commit = ""
> +self.disable_rebase = False
> self.prepare_p4_only = False
> self.conflict_behavior = None
> self.isWindows = (platform.system() == "Windows")
> @@ -2103,9 +2110,18 @@ class P4Submit(Command, P4UserMap):
> else:
> commitish = 'HEAD'
>
> -for line in read_pipe_lines(["git", "rev-list", "--no-merges",
> "%s..%s" % (self.origin, commitish)]):
> -commits.append(line.strip())
> -commits.reverse()
> +if self.commit != "":
> +if self.commit.find("..") != -1:
> +limits_ish = self.commit.split("..")
> +for line in read_pipe_lines(["git", "rev-list",
> "--no-merges", "%s..%s" % (limits_ish[0], limits_ish[1])]):
> +commits.append(line.strip())
> +commits.reverse()
> +else:
> +commits.append(self.commit)
> +else:
> +for line in read_pipe_lines(["git", "rev-list", "--no-merges",
> "%s..%s" % (self.origin, commitish)]):
> +commit
[PATCHv4 1/1] git-p4: add unshelve command
This can be used to "unshelve" a shelved P4 commit into
a git commit.
For example:
$ git p4 unshelve 12345
The resulting commit ends up in the branch:
refs/remotes/p4/unshelved/12345
If that branch already exists, it is renamed - for example
the above branch would be saved as p4/unshelved/12345.1.
git-p4 checks that the shelved changelist is based on files
which are at the same Perforce revision as the origin branch
being used for the unshelve (HEAD by default). If they are not,
it will refuse to unshelve. This is to ensure that the unshelved
change does not contain other changes mixed-in.
The reference branch can be changed manually with the "--origin"
option.
The change adds a new Unshelve command class. This just runs the
existing P4Sync code tweaked to handle a shelved changelist.
Signed-off-by: Luke Diamand
---
Documentation/git-p4.txt | 32 ++
git-p4.py| 207 ---
t/t9832-unshelve.sh | 153 +
3 files changed, 356 insertions(+), 36 deletions(-)
create mode 100755 t/t9832-unshelve.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index d8c8f11c9f..d3cb249fc2 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -164,6 +164,31 @@ $ git p4 submit --shelve
$ git p4 submit --update-shelve 1234 --update-shelve 2345
+
+Unshelve
+
+Unshelving will take a shelved P4 changelist, and produce the equivalent git
commit
+in the branch refs/remotes/p4/unshelved/.
+
+The git commit is created relative to the current origin revision (HEAD by
default).
+If the shelved changelist's parent revisions differ, git-p4 will refuse to
unshelve;
+you need to be unshelving onto an equivalent tree.
+
+The origin revision can be changed with the "--origin" option.
+
+If the target branch in refs/remotes/p4/unshelved already exists, the old one
will
+be renamed.
+
+
+$ git p4 sync
+$ git p4 unshelve 12345
+$ git show refs/remotes/p4/unshelved/12345
+
+$ git p4 unshelve 12345
+
+
+
+
OPTIONS
---
@@ -337,6 +362,13 @@ These options can be used to modify 'git p4 rebase'
behavior.
--import-labels::
Import p4 labels.
+Unshelve options
+
+
+--origin::
+Sets the git refspec against which the shelved P4 changelist is compared.
+Defaults to p4/master.
+
DEPOT PATH SYNTAX
-
The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69..9390d58a84 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -316,12 +316,17 @@ def p4_last_change():
results = p4CmdList(["changes", "-m", "1"], skip_info=True)
return int(results[0]['change'])
-def p4_describe(change):
+def p4_describe(change, shelved=False):
"""Make sure it returns a valid result by checking for
the presence of field "time". Return a dict of the
results."""
-ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
+cmd = ["describe", "-s"]
+if shelved:
+cmd += ["-S"]
+cmd += [str(change)]
+
+ds = p4CmdList(cmd, skip_info=True)
if len(ds) != 1:
die("p4 describe -s %d did not return 1 result: %s" % (change,
str(ds)))
@@ -662,6 +667,12 @@ def gitBranchExists(branch):
stderr=subprocess.PIPE, stdout=subprocess.PIPE);
return proc.wait() == 0;
+def gitUpdateRef(ref, newvalue):
+subprocess.check_call(["git", "update-ref", ref, newvalue])
+
+def gitDeleteRef(ref):
+subprocess.check_call(["git", "update-ref", "-d", ref])
+
_gitConfig = {}
def gitConfig(key, typeSpecifier=None):
@@ -2411,6 +2422,7 @@ class P4Sync(Command, P4UserMap):
self.tempBranches = []
self.tempBranchLocation = "refs/git-p4-tmp"
self.largeFileSystem = None
+self.suppress_meta_comment = False
if gitConfig('git-p4.largeFileSystem'):
largeFileSystemConstructor =
globals()[gitConfig('git-p4.largeFileSystem')]
@@ -2421,6 +2433,18 @@ class P4Sync(Command, P4UserMap):
if gitConfig("git-p4.syncFromOrigin") == "false":
self.syncWithOrigin = False
+self.depotPaths = []
+self.changeRange = ""
+self.previousDepotPaths = []
+self.hasOrigin = False
+
+# map from branch depot path to parent branch
+self.knownBranches = {}
+self.initialParents = {}
+
+self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone %
3600) / 60))
+self.labels = {}
+
# Force a checkpoint in fast-import and wait for it to finish
def checkpoint(self):
self.git
[PATCHv4 0/1] git-p4: unshelving: fix for python2.6
This is the same as the previous unshelve change, other than fixing
the "{}".format(foo) constructs I introduced to be compatible with
Python2.6.
https://marc.info/?l=git&m=152637850403462
Python2.6 doesn't understand empty format fields ("{}"), so I have
added element indexes, e.g. "{0}".format(foo).
Luke Diamand (1):
git-p4: add unshelve command
Documentation/git-p4.txt | 32 ++
git-p4.py| 207 ---
t/t9832-unshelve.sh | 153 +
3 files changed, 356 insertions(+), 36 deletions(-)
create mode 100755 t/t9832-unshelve.sh
--
2.17.0.392.gdeb1a6e9b7
Re: Re :[PATCHv4 1/1] git-p4: add unshelve command
On 19 May 2018 at 12:26, merlo...@yahoo.fr wrote:
> Hi Luke,
>
> In the P4Unshelve classe, could you add an help description directly in the
> optparse.add_option of --origin ?
Sure. I'll do a re-roll.
>
> From the workfow point of you, do you think it will be possible to make
> changes in the git branch of the unshelved CL (remotes/p4/unshelved/),
> then update the p4 shelved CL directly ? It would be great.
That's an interesting idea. You would need to check it out somehow,
but then it should just work.
i.e.
$ git p4 unshelve
$ git checkout remotes/p4/unshelved/
$ vim foo.c
$ git commit --amend foo.c
$ git p4 submit --origin HEAD^ --update-shelve
I think it should work as-is.
>
> Romain
>
> Envoyé depuis Yahoo Mail pour Android
>
> Le sam., mai 19, 2018 à 12:00, Luke Diamand
> a écrit :
> This can be used to "unshelve" a shelved P4 commit into
> a git commit.
>
> For example:
>
> $ git p4 unshelve 12345
>
> The resulting commit ends up in the branch:
> refs/remotes/p4/unshelved/12345
>
> If that branch already exists, it is renamed - for example
> the above branch would be saved as p4/unshelved/12345.1.
>
> git-p4 checks that the shelved changelist is based on files
> which are at the same Perforce revision as the origin branch
> being used for the unshelve (HEAD by default). If they are not,
> it will refuse to unshelve. This is to ensure that the unshelved
> change does not contain other changes mixed-in.
>
> The reference branch can be changed manually with the "--origin"
> option.
>
> The change adds a new Unshelve command class. This just runs the
> existing P4Sync code tweaked to handle a shelved changelist.
>
> Signed-off-by: Luke Diamand
> ---
> Documentation/git-p4.txt | 32 ++
> git-p4.py| 207 ---
> t/t9832-unshelve.sh | 153 +
> 3 files changed, 356 insertions(+), 36 deletions(-)
> create mode 100755 t/t9832-unshelve.sh
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index d8c8f11c9f..d3cb249fc2 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -164,6 +164,31 @@ $ git p4 submit --shelve
> $ git p4 submit --update-shelve 1234 --update-shelve 2345
>
>
> +
> +Unshelve
> +
> +Unshelving will take a shelved P4 changelist, and produce the equivalent
> git commit
> +in the branch refs/remotes/p4/unshelved/.
> +
> +The git commit is created relative to the current origin revision (HEAD by
> default).
> +If the shelved changelist's parent revisions differ, git-p4 will refuse to
> unshelve;
> +you need to be unshelving onto an equivalent tree.
> +
> +The origin revision can be changed with the "--origin" option.
> +
> +If the target branch in refs/remotes/p4/unshelved already exists, the old
> one will
> +be renamed.
> +
> +
> +$ git p4 sync
> +$ git p4 unshelve 12345
> +$ git show refs/remotes/p4/unshelved/12345
> +
> +$ git p4 unshelve 12345
> +
> +
> +
> +
> OPTIONS
> ---
>
> @@ -337,6 +362,13 @@ These options can be used to modify 'git p4 rebase'
> behavior.
> --import-labels::
> Import p4 labels.
>
> +Unshelve options
> +
> +
> +--origin::
> +Sets the git refspec against which the shelved P4 changelist is
> compared.
> +Defaults to p4/master.
> +
> DEPOT PATH SYNTAX
> -
> The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
> diff --git a/git-p4.py b/git-p4.py
> index 7bb9cadc69..9390d58a84 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -316,12 +316,17 @@ def p4_last_change():
> results = p4CmdList(["changes", "-m", "1"], skip_info=True)
> return int(results[0]['change'])
>
> -def p4_describe(change):
> +def p4_describe(change, shelved=False):
> """Make sure it returns a valid result by checking for
> the presence of field "time". Return a dict of the
> results."""
>
> -ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
> +cmd = ["describe", "-s"]
> +if shelved:
> +cmd += ["-S"]
> +cmd += [str(change)]
> +
> +ds = p4CmdList(cmd, skip_info=True)
> if len(ds) != 1:
> die("p4 describe -s %d did not return 1 result: %s" % (change,
> str(ds)))
>
> @@ -662,6 +667,12 @@ def gitBranchExists(branch):
> stderr=subpro
Re: [PATCHv4 1/1] git-p4: add unshelve command
On 21 May 2018 at 08:07, Junio C Hamano wrote:
> SZEDER Gábor writes:
>
>>> diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
>>> new file mode 100755
>>> index 00..cca2dec536
>
> ... in short, I'd queue a fix-up on top like this to be later
> squashed after getting an ack?
That looks good to me, thanks! Ack.
Luke
>
> t/t9832-unshelve.sh | 23 ---
> 1 file changed, 4 insertions(+), 19 deletions(-)
>
> diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
> index cca2dec536..3513abd21a 100755
> --- a/t/t9832-unshelve.sh
> +++ b/t/t9832-unshelve.sh
> @@ -1,6 +1,6 @@
> #!/bin/sh
>
> -last_shelved_change() {
> +last_shelved_change () {
> p4 changes -s shelved -m1 | cut -d " " -f 2
> }
>
> @@ -17,7 +17,7 @@ test_expect_success 'init depot' '
> cd "$cli" &&
> echo file1 >file1 &&
> p4 add file1 &&
> - p4 submit -d "change 1"
> + p4 submit -d "change 1" &&
> : >file_to_delete &&
> p4 add file_to_delete &&
> p4 submit -d "file to delete"
> @@ -120,29 +120,14 @@ EOF
> )
> '
>
> -diff_adds_line() {
> - text="$1" &&
> - file="$2" &&
> - grep -q "^+$text" $file || (echo "expected \"text\" $text not found
> in $file" && exit 1)
> -}
> -
> -diff_excludes_line() {
> - text="$1" &&
> - file="$2" &&
> - if grep -q "^+$text" $file; then
> - echo "unexpected text \"$text\" found in $file" &&
> - exit 1
> - fi
> -}
> -
> # Now try to unshelve it. git-p4 should refuse to do so.
> test_expect_success 'try to unshelve the change' '
> test_when_finished cleanup_git &&
> (
> change=$(last_shelved_change) &&
> cd "$git" &&
> - ! git p4 unshelve $change >out.txt 2>&1 &&
> - grep -q "cannot unshelve" out.txt
> + test_must_fail git p4 unshelve $change 2>err.txt &&
> + grep -q "cannot unshelve" err.txt
> )
> '
>
Re: [PATCHv4 1/1] git-p4: add unshelve command
On 21 May 2018 at 22:39, SZEDER Gábor wrote: >> diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh ... ... >> +' > > This test fails on my box and on Travis CI (in all four standard Linux > and OSX build jobs) with: > > + cd /home/szeder/src/git/t/trash directory.t9832-unshelve/cli > + p4 edit file1 > //depot/file1#1 - opened for edit > + echo a change > + echo new file > + p4 add file2 > //depot/file2#1 - opened for add > + p4 delete file_to_delete > //depot/file_to_delete#1 - opened for delete > + p4 opened > //depot/file1#1 - edit default change (text) > //depot/file2#1 - add default change (text) > //depot/file_to_delete#1 - delete default change (text) > + p4 shelve -i > Change 3 created with 3 open file(s). > Shelving files for change 3. > edit //depot/file1#1 > add //depot/file2#1 > delete //depot/file_to_delete#1 > Change 3 files shelved. > + cd /home/szeder/src/git/t/trash directory.t9832-unshelve/git > + last_shelved_change > + p4 changes -s shelved -m1 > + cut -d -f 2 > + change=3 > + git p4 unshelve 3 > Traceback (most recent call last): > File "/home/szeder/src/git/git-p4", line 3975, in > main() > File "/home/szeder/src/git/git-p4", line 3969, in main > if not cmd.run(args): > File "/home/szeder/src/git/git-p4", line 3851, in run > sync.importChanges(changes, shelved=True, > origin_revision=origin_revision) > File "/home/szeder/src/git/git-p4", line 3296, in importChanges > files = self.extractFilesFromCommit(description, shelved, change, > origin_revision) > File "/home/szeder/src/git/git-p4", line 2496, in > extractFilesFromCommit > not self.cmp_shelved(path, file["rev"], origin_revision): > File "/home/szeder/src/git/git-p4", line 2467, in cmp_shelved > return ret["status"] == "identical" > KeyError: 'status' > error: last command exited with $?=1 > not ok 4 - create shelved changelist It works fine for me - but given where it's failing, my first suspicion would be p4 client version (or server) differences. I'm using 2015.1 for server and client. Could you check which version you are using? Thanks, Luke
Re: [PATCHv4 1/1] git-p4: add unshelve command
On 21 May 2018 at 23:09, Luke Diamand wrote: > On 21 May 2018 at 22:39, SZEDER Gábor wrote: >>> diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh > > ... > ... > >>> +' >> >> This test fails on my box and on Travis CI (in all four standard Linux >> and OSX build jobs) with: >> >> + cd /home/szeder/src/git/t/trash directory.t9832-unshelve/cli >> + p4 edit file1 >> //depot/file1#1 - opened for edit >> + echo a change >> + echo new file >> + p4 add file2 >> //depot/file2#1 - opened for add >> + p4 delete file_to_delete >> //depot/file_to_delete#1 - opened for delete >> + p4 opened >> //depot/file1#1 - edit default change (text) >> //depot/file2#1 - add default change (text) >> //depot/file_to_delete#1 - delete default change (text) >> + p4 shelve -i >> Change 3 created with 3 open file(s). >> Shelving files for change 3. >> edit //depot/file1#1 >> add //depot/file2#1 >> delete //depot/file_to_delete#1 >> Change 3 files shelved. >> + cd /home/szeder/src/git/t/trash directory.t9832-unshelve/git >> + last_shelved_change >> + p4 changes -s shelved -m1 >> + cut -d -f 2 >> + change=3 >> + git p4 unshelve 3 >> Traceback (most recent call last): >> File "/home/szeder/src/git/git-p4", line 3975, in >> main() >> File "/home/szeder/src/git/git-p4", line 3969, in main >> if not cmd.run(args): >> File "/home/szeder/src/git/git-p4", line 3851, in run >> sync.importChanges(changes, shelved=True, >> origin_revision=origin_revision) >> File "/home/szeder/src/git/git-p4", line 3296, in importChanges >> files = self.extractFilesFromCommit(description, shelved, change, >> origin_revision) >> File "/home/szeder/src/git/git-p4", line 2496, in >> extractFilesFromCommit >> not self.cmp_shelved(path, file["rev"], origin_revision): >> File "/home/szeder/src/git/git-p4", line 2467, in cmp_shelved >> return ret["status"] == "identical" >> KeyError: 'status' >> error: last command exited with $?=1 >> not ok 4 - create shelved changelist > > It works fine for me - but given where it's failing, my first > suspicion would be p4 client version (or server) differences. > > I'm using 2015.1 for server and client. Could you check which version > you are using? In fact, no need. It works fine with 2015.1 but 2017.1 fails with this error. Sigh. I'll reroll. Luke > > Thanks, > Luke
