Re: git-p4 out of memory for very large repository

[Luke Diamand]

On 23/08/13 02:12, Corey Thompson wrote:

Hello,

Has anyone actually gotten git-p4 to clone a large Perforce repository?


Yes. I've cloned repos with a couple of Gig of files.


I have one codebase in particular that gets to about 67%, then
consistently gets get-fast-import (and often times a few other
processes) killed by the OOM killer.


What size is this codebase? Which version and platform of git are you using?

Maybe it's a regression, or perhaps you've hit some new, previously 
unknown size limit?


Thanks
Luke




I've found some patches out there that claim to resolve this, but
they're all for versions of git-p4.py from several years ago.  Not only
will they not apply cleanly, but as far as I can tell the issues that
these patches are meant to address aren't in the current version,
anyway.

Any suggestions would be greatly appreciated.

Thanks,
Corey
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-p4 out of memory for very large repository

[Luke Diamand]

I think I've cloned files as large as that or larger. If you just want to
clone this and move on, perhaps you just need a bit more memory? What's the
size of your physical memory and swap partition? Per process memory limit?


On 23 Aug 2013 12:59, Corey Thompson cmt...@gmail.com wrote:
On 23/08/13 12:59, Corey Thompson wrote:
 On Fri, Aug 23, 2013 at 07:48:56AM -0400, Corey Thompson wrote:
 Sorry, I guess I could have included more details in my original post.
 Since then, I have also made an attempt to clone another (slightly more
 recent) branch, and at last had success.  So I see this does indeed
 work, it just seems to be very unhappy with one particular branch.

 So, here are a few statistics I collected on the two branches.

 branch-that-fails:
 total workspace disk usage (current head): 12GB
 68 files over 20MB
 largest three being about 118MB

 branch-that-clones:
 total workspace disk usage (current head): 11GB
 22 files over 20MB
 largest three being about 80MB

 I suspect that part of the problem here might be that my company likes
 to submit very large binaries into our repo (.tar.gzs, pre-compiled
 third party binaries, etc.).

 Is there any way I can clone this in pieces?  The best I've come up with
 is to clone only up to a change number just before it tends to fail, and
 then rebase to the latest.  My clone succeeded, but the rebase still
 runs out of memory.  It would be great if I could specify a change
 number to rebase up to, so that I can just take this thing a few hundred
 changes at a time.

 Thanks,
 Corey
 
 And I still haven't told you anything about my platform or git
 version...
 
 This is on Fedora Core 11, with git 1.8.3.4 built from the github repo
 (117eea7e).

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-p4 out of memory for very large repository

[Luke Diamand]

I guess you could try changing the OOM score for git-fast-import.

change /proc/pid/oomadj.

I think a value of -31 would make it very unlikely to be killed.

On 29/08/13 23:46, Pete Wyckoff wrote:

cmt...@gmail.com wrote on Wed, 28 Aug 2013 11:41 -0400:

On Mon, Aug 26, 2013 at 09:47:56AM -0400, Corey Thompson wrote:

You are correct that git-fast-import is killed by the OOM killer, but I
was unclear about which process was malloc()ing so much memory that the
OOM killer got invoked (as other completely unrelated processes usually
also get killed when this happens).

Unless there's one gigantic file in one change that gets removed by
another change, I don't think that's the problem; as I mentioned in
another email, the machine has 32GB physical memory and the largest
single file in the current head is only 118MB.  Even if there is a very
large transient file somewhere in the history, I seriously doubt it's
tens of gigabytes in size.

I have tried watching it with top before, but it takes several hours
before it dies.  I haven't been able to see any explosion of memory
usage, even within the final hour, but I've never caught it just before
it dies, either.  I suspect that whatever the issue is here, it happens
very quickly.

If I'm unable to get through this today using the incremental p4 sync
method you described, I'll try running a full-blown clone overnight with
top in batch mode writing to a log file to see whether it catches
anything.

Thanks again,
Corey


Unforunately I have not made much progress.  The incremental sync method
fails with the output pasted below.  The change I specified is only one
change number above where that repo was cloned...


I usually just do git p4 sync @505859.  The error message below
crops up when things get confused.  Usually after a previous
error.  I tend to destroy the repo and try again.  Sorry I don't
can't explain better what's happening here.  It's not a memory
issue; it reports only 24 MB used.


So I tried a 'git p4 rebase' overnight with top running, and as I feared
I did not see anything out of the ordinary.  git, git-fast-import, and
git-p4 all hovered under 1.5% MEM the entire time, right up until
death.  The last entry in my log shows git-fast-import at 0.8%, with git
and git-p4 at 0.0% and 0.1%, respectively.  I could try again with a
more granular period, but I feel like this method is ultimately a goose
chase.


Bizarre.  There is no good explanation why memory usage would go
up to 32 GB (?) within one top interval (3 sec ?).  My theory
about one gigantic object is debunked:  you have only the 118 MB
one.  Perhaps there's some container or process memory limit, as
Luke guessed, but it's not obvious here.

The other big hammer is strace.  If you're still interested in
playing with this, you could do:

 strace -vf -tt -s 200 -o /tmp/strace.out git p4 clone 

and hours later, see if something suggests itself toward the
end of that output file.

-- Pete


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Is there a way to find out which commit git rebase --skip skipped?

[Luke Diamand]

If I do git rebase --skip, is there a way to find out the commit SHA
that was skipped (other than just parsing the output of the command) ?

I'd like to modify git-p4 so that it can automatically skip past
conflicting changes, but I'd like it to keep a log of which commits
were skipped.

Thanks,
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/12] git p4: submit conflict handling

[Luke Diamand]

On 17/08/12 00:35, Pete Wyckoff wrote:

These patches rework how git p4 deals with conflicts that
arise during a git p4 submit.  These may arise due to
changes that happened in p4 since the last git p4 sync.

Luke: I especially wanted to get this out as you suggested
that you had a different way of dealing with skipped commits.

The part that needs the most attention is the interaction
loop that happens when a commit failed.  Currently, three
options are offered:

 [s]kip this commit, but continue to apply others
 [a]pply the commit forcefully, generating .rej files
 [w]rite the commit to a patch.txt file
 and the implicitctrl-c  to stop

After this series, it offers two:

 [c]ontinue to apply others
 [q]uit to stop

This feels more natural to me, and I like the term continue rather
than skip as it matches what rebase uses.  I'd like to know what
others think of the new flow.


The skip is still needed. In my workflow, git-p4 gets run periodically 
and does the usual sync+rebase on behalf of all the people who have 
pushed to the git repo.


If someone pushes a change which conflicts with something from Perforce 
land, then what I want to happen is for the script to discard the 
offending commit (git rebase --skip) and then carry on with the others.


In 99% of cases this does exactly what I need, as conflicting commits 
are usually caused by people committing the same fix to both p4 and git 
at around the same time (someone breaks top-of-tree with an obvious 
error, two separate people check in slightly different fixes). 
Discarding the git commit then means that everything carries on working.


I've got a small patch which makes skipping work non-interactively; the 
thing it's missing is reporting the commits which are skipped.




Other observable changes are new command-line options:

Alias -v for --verbose, similar to other git commands.

The --dry-run option addresses Luke's concern in

 http://thread.gmane.org/gmane.comp.version-control.git/201004/focus=201022

when I removed an unused self.interactive variable
that did a similar thing if you edited the code.  It prints
commits that would be applied to p4.

Option --prepare-p4-only is similar to --dry-run, in that
it does not submit anything to p4, but it does prepare the
p4 workspace, then prints long instructions about how to submit
everything properly.  It also serves, perhaps, as a replacement for
the [a]pply option in the submit-conflict loop.

Pete Wyckoff (12):
   git p4 test: remove bash-ism of combined export/assignment
   git p4 test: use p4d -L option to suppress log messages
   git p4: gracefully fail if some commits could not be applied
   git p4: remove submit failure options [a]pply and [w]rite
   git p4: move conflict prompt into run, use [c]ontinue and [q]uit
   git p4: standardize submit cancel due to unchanged template
   git p4: test clean-up after failed submit, fix added files
   git p4: rearrange submit template construction
   git p4: revert deleted files after submit cancel
   git p4: accept -v for --verbose
   git p4: add submit --dry-run option
   git p4: add submit --prepare-p4-only option

  Documentation/git-p4.txt   |  13 +-
  git-p4.py  | 213 +++--
  t/lib-git-p4.sh|  10 +-
  t/t9805-git-p4-skip-submit-edit.sh |   2 +-
  t/t9807-git-p4-submit.sh   |  65 +++
  t/t9810-git-p4-rcs.sh  |  50 +
  t/t9815-git-p4-submit-fail.sh  | 367 +
  7 files changed, 612 insertions(+), 108 deletions(-)
  create mode 100755 t/t9815-git-p4-submit-fail.sh



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/12] git p4 test: use p4d -L option to suppress log messages

[Luke Diamand]

On 17/08/12 00:35, Pete Wyckoff wrote:

Send p4d output to a logfile in the $TRASH_DIRECTORY.
Its messages add no value to testing.


I'm not totally sold on this; I still fairly frequently see weird errors 
from p4d and these help me work out what's going on. For example, at the 
moment if you run a test too quickly after the last one, then it won't 
start up (or something like that).


The problem with hiding the error messages is that I don't think I will 
think to look in this log file if tests start failing.




Signed-off-by: Pete Wyckoffp...@padd.com
---
  t/lib-git-p4.sh | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 482eeac..edb4033 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -35,12 +35,13 @@ db=$TRASH_DIRECTORY/db
  cli=$(test-path-utils real_path $TRASH_DIRECTORY/cli)
  git=$TRASH_DIRECTORY/git
  pidfile=$TRASH_DIRECTORY/p4d.pid
+logfile=$TRASH_DIRECTORY/p4d.log

  start_p4d() {
mkdir -p $db $cli $git
rm -f $pidfile
(
-   p4d -q -r $db -p $P4DPORT
+   p4d -q -r $db -p $P4DPORT -L $logfile
echo $!$pidfile
)



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/12] git p4: gracefully fail if some commits could not be applied

[Luke Diamand]

On 17/08/12 00:35, Pete Wyckoff wrote:

If a commit fails to apply cleanly to the p4 tree, an interactive
prompt asks what to do next.  In all cases (skip, apply, write),
the behavior after the prompt had a few problems.

Change it so that it does not claim erroneously that all commits
were applied.  Instead list the set of the patches under
consideration, and mark with an asterisk those that were
applied successfully.  Like this example:


I could be wrong about this, but this change doesn't seem to help out 
with git p4 rebase, which for me at least, is where the conflicts 
usually get picked up first.


I modified a file in p4, and the same file in git, and then did 'git p4 
rebase' and it just failed in the rebase in the usual way with a big 'ol 
python backtrace.


If this patch series is intended to sort out conflict handling, then it 
needs a bit more work.


(Says Luke, trying not to sound too confrontational, as I'm rubbish at 
handling conflict)


Thanks!
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Is there a way to find out which commit git rebase --skip skipped?

[Luke Diamand]

On 16/08/12 16:43, Junio C Hamano wrote:

Luke Diamandl...@diamand.org  writes:


If I do git rebase --skip, is there a way to find out the commit SHA
that was skipped (other than just parsing the output of the command) ?


There currently isn't, and I do not think it is doable in general
when the command ever gives control back to the user to futz with
the history, expecting the user only to fix up the conflict and make
a single commit (in which case you would want to say that old
commit was replayed as this commit with different patch id) or say
rebase --skip (in which case you could record that old commit was
manually skipped), but the user can do other things like resetting
the head to lose commits that have been rebased already, adding new
commits manually before continuing, etc., all of which will be done
outside of your control.


All I need is to be able to get the commit *immediately* after the 
failed 'git rebase'. It looks like .git/ORIG_HEAD has exactly what I need.


Thanks,
Luke

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 01/12] git p4 test: remove bash-ism of combined export/assignment

[Luke Diamand]

Looks good to me, ack.

On 09/09/12 21:16, Pete Wyckoff wrote:

Signed-off-by: Pete Wyckoffp...@padd.com
---
  t/lib-git-p4.sh | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 2d753ab..482eeac 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -26,9 +26,10 @@ testid=${this_test#t}
  git_p4_test_start=9800
  P4DPORT=$((10669 + ($testid - $git_p4_test_start)))

-export P4PORT=localhost:$P4DPORT
-export P4CLIENT=client
-export P4EDITOR=:
+P4PORT=localhost:$P4DPORT
+P4CLIENT=client
+P4EDITOR=:
+export P4PORT P4CLIENT P4EDITOR

  db=$TRASH_DIRECTORY/db
  cli=$(test-path-utils real_path $TRASH_DIRECTORY/cli)


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 03/12] git p4: remove submit failure options [a]pply and [w]rite

[Luke Diamand]

git-p4 won't be quite the same without these completely misleading and 
confusing messages :-)


Ack.



On 09/09/12 21:16, Pete Wyckoff wrote:

When a patch failed to apply, these interactive options offered
to:

 1) apply the patch anyway, leaving reject (.rej) files around, or,
 2) write the patch to a file (patch.txt)

In both cases it suggested to invoke git p4 submit --continue,
an unimplemented option.

While manually fixing the rejects and submitting the result might
work, there are many steps that must be done to the job properly:

 * apply patch
 * invoke p4 add and delete
 * change executable bits
 * p4 sync -f renamed/copied files
 * extract commit message into p4 change description and
   move Jobs lines out of description section
 * set changelist owner for --preserve-user

Plus the following manual sync/rebase will cause conflicts too,
which must be resolved once again.

Drop these workflows.  Instead users should do a sync/rebase in
git, fix the conflicts there, and do a clean git p4 submit.

Signed-off-by: Pete Wyckoffp...@padd.com
---
  git-p4.py | 20 ++--
  1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 2405f38..e08fea1 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1200,9 +1200,8 @@ class P4Submit(Command, P4UserMap):
  if not patch_succeeded:
  print What do you want to do?
  response = x
-while response != s and response != a and response != w:
-response = raw_input([s]kip this patch / [a]pply the patch 
forcibly 
- and with .rej files / [w]rite the patch to a 
file (patch.txt) )
+while response != s:
+response = raw_input([s]kip this patch )
  if response == s:
  print Skipping! Good luck with the next patches...
  for f in editedFiles:
@@ -1210,21 +1209,6 @@ class P4Submit(Command, P4UserMap):
  for f in filesToAdd:
  os.remove(f)
  return False
-elif response == a:
-os.system(applyPatchCmd)
-if len(filesToAdd)  0:
-print You may also want to call p4 add on the following 
files:
-print  .join(filesToAdd)
-if len(filesToDelete):
-print The following files should be scheduled for deletion 
with p4 delete:
-print  .join(filesToDelete)
-die(Please resolve and submit the conflict manually and 
-+ continue afterwards with git p4 submit --continue)
-elif response == w:
-system(diffcmd +   patch.txt)
-print Patch saved to patch.txt in %s ! % self.clientPath
-die(Please resolve and submit the conflict manually and 
-continue afterwards with git p4 submit --continue)

  system(applyPatchCmd)



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 04/12] git p4: move conflict prompt into run, add [q]uit input

[Luke Diamand]

I'll need to supply a followup patch to ensure that a config option can 
override the prompt.


Ack.

On 09/09/12 21:16, Pete Wyckoff wrote:

When applying a commit to the p4 workspace fails, a prompt
asks what to do next.  This belongs up in run() instead
of in applyCommit(), where run() can notice, for instance,
that the prompt is unnecessary because this is the last commit.

Offer two options about how to continue at conflict: [s]kip or
[q]uit.  Having an explicit quit option gives git p4 a chance
to clean up, show the applied-commit summary, and do tag export.

Signed-off-by: Pete Wyckoffp...@padd.com
---
  git-p4.py | 41 +
  t/t9815-git-p4-submit-fail.sh | 31 +++
  2 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index e08fea1..479f1fc 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1198,17 +1198,11 @@ class P4Submit(Command, P4UserMap):
  patch_succeeded = True

  if not patch_succeeded:
-print What do you want to do?
-response = x
-while response != s:
-response = raw_input([s]kip this patch )
-if response == s:
-print Skipping! Good luck with the next patches...
-for f in editedFiles:
-p4_revert(f)
-for f in filesToAdd:
-os.remove(f)
-return False
+for f in editedFiles:
+p4_revert(f)
+for f in filesToAdd:
+os.remove(f)
+return False

  system(applyPatchCmd)

@@ -1475,11 +1469,34 @@ class P4Submit(Command, P4UserMap):
  if gitConfig(git-p4.detectCopiesHarder, --bool) == true:
  self.diffOpts +=  --find-copies-harder

+#
+# Apply the commits, one at a time.  On failure, ask if should
+# continue to try the rest of the patches, or quit.
+#
  applied = []
-for commit in commits:
+last = len(commits) - 1
+for i, commit in enumerate(commits):
  ok = self.applyCommit(commit)
  if ok:
  applied.append(commit)
+else:
+if i  last:
+quit = False
+while True:
+print What do you want to do?
+response = raw_input([s]kip this commit but apply
+  the rest, or [q]uit? )
+if not response:
+continue
+if response[0] == s:
+print Skipping this commit, but applying the rest
+break
+if response[0] == q:
+print Quitting
+quit = True
+break
+if quit:
+break

  chdir(self.oldWorkingDirectory)

diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh
index 5c36714..397b3e8 100755
--- a/t/t9815-git-p4-submit-fail.sh
+++ b/t/t9815-git-p4-submit-fail.sh
@@ -18,7 +18,7 @@ test_expect_success 'init depot' '
)
  '

-test_expect_success 'conflict on one commit, skip' '
+test_expect_success 'conflict on one commit' '
test_when_finished cleanup_git
git p4 clone --dest=$git //depot
(
@@ -34,12 +34,12 @@ test_expect_success 'conflict on one commit, skip' '
echo line3file1
git add file1
git commit -m line3 in file1 will conflict
-   echo s | test_expect_code 1 git p4 submitout
+   test_expect_code 1 git p4 submitout
test_i18ngrep No commits applied out
)
  '

-test_expect_success 'conflict on second of two commits, skip' '
+test_expect_success 'conflict on second of two commits' '
test_when_finished cleanup_git
git p4 clone --dest=$git //depot
(
@@ -57,7 +57,7 @@ test_expect_success 'conflict on second of two commits, skip' 
'
echo line4file1
git add file1
git commit -m line4 in file1 will conflict
-   echo s | test_expect_code 1 git p4 submitout
+   test_expect_code 1 git p4 submitout
test_i18ngrep Applied only the commits out
)
  '
@@ -85,6 +85,29 @@ test_expect_success 'conflict on first of two commits, skip' 
'
)
  '

+test_expect_success 'conflict on first of two commits, quit' '
+   test_when_finished cleanup_git
+   git p4 clone --dest=$git //depot
+   (
+   cd $cli
+   p4 open file1
+   echo line7file1
+   p4 submit -d line7 in file1
+   )
+   (
+   cd $git
+   git config git-p4.skipSubmitEdit true
+   # 

Re: [PATCHv2 05/12] git p4: standardize submit cancel due to unchanged template

[Luke Diamand]

Ack.

On 09/09/12 21:16, Pete Wyckoff wrote:

When editing the submit template, if no change was made to it,
git p4 offers a prompt Submit anyway?.  Answering no cancels
the submit.

Previously, a no answer behaves like a [s]kip answer to the
failed-patch prompt, in that it proceeded to try to apply the
rest of the commits.  Instead, put users back into the new
[s]kip / [c]ontinue loop so that they can decide.  This makes
both cases of patch failure behave identically.

The return code of git p4 after a no answer is now the same
as that for a skip due to failed patch; update a test to
understand this.

Signed-off-by: Pete Wyckoffp...@padd.com
---
  git-p4.py  | 4 +++-
  t/t9805-git-p4-skip-submit-edit.sh | 2 +-
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 479f1fc..39fa2e1 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1262,6 +1262,7 @@ class P4Submit(Command, P4UserMap):

  if self.edit_template(fileName):
  # read the edited message and submit
+ret = True
  tmpFile = open(fileName, rb)
  message = tmpFile.read()
  tmpFile.close()
@@ -1285,6 +1286,7 @@ class P4Submit(Command, P4UserMap):

  else:
  # skip this patch
+ret = False
  print Submission cancelled, undoing p4 changes.
  for f in editedFiles:
  p4_revert(f)
@@ -1293,7 +1295,7 @@ class P4Submit(Command, P4UserMap):
  os.remove(f)

  os.remove(fileName)
-return True  # success
+return ret

  # Export git tags as p4 labels. Create a p4 label and then tag
  # with that.
diff --git a/t/t9805-git-p4-skip-submit-edit.sh 
b/t/t9805-git-p4-skip-submit-edit.sh
index fb3c8ec..ff2cc79 100755
--- a/t/t9805-git-p4-skip-submit-edit.sh
+++ b/t/t9805-git-p4-skip-submit-edit.sh
@@ -38,7 +38,7 @@ test_expect_success 'no config, unedited, say no' '
cd $git
echo linefile1
git commit -a -m change 3 (not really)
-   printf bad response\nn\n | git p4 submit
+   printf bad response\nn\n | test_expect_code 1 git p4 submit
p4 changes //depot/...wc
test_line_count = 2 wc
)


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 01/12] git p4 test: remove bash-ism of combined export/assignment

[Luke Diamand]

On 16/09/12 07:05, Junio C Hamano wrote:

Luke Diamandl...@diamand.org  writes:


Looks good to me, ack.


Thanks; is this an ack for the entire series, or are you expecting
further back-and-forth with Pete before the whole thing is ready?


An ack for the whole series. I'm just going through the rest of the 
patches now but I don't expect to find any issues.





--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 01/12] git p4 test: remove bash-ism of combined export/assignment

[Luke Diamand]

On 17/09/12 05:50, Junio C Hamano wrote:

Luke Diamandl...@diamand.org  writes:


On 16/09/12 07:05, Junio C Hamano wrote:

Luke Diamandl...@diamand.org   writes:


Looks good to me, ack.


Thanks; is this an ack for the entire series, or are you expecting
further back-and-forth with Pete before the whole thing is ready?


An ack for the whole series. I'm just going through the rest of the
patches now but I don't expect to find any issues.


As long as the parties involved in the part of the system can agree
that this series is basically sound, I'd be happy to merge it
down. Minor issues can be fixed up as follow-up patches.


Basically sound as far as I'm concerned.



Thanks.  Will merge it to 'next'.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git p4 submit failing

[Luke Diamand]

Just a thought, but check the files that are failing to see if they've
got RCS keywords in them ($Id$, $File$, $Date$, etc). These cause all
sorts of nasty problems.

That's assuming it's definitely not a CRLF line ending problem on Windows.

On Thu, Apr 11, 2013 at 8:01 PM, Christopher Yee Mon
christopher.yee...@gmail.com wrote:
 I tried running git p4 submit on a repo that I've been running as an
 interim bridge between git and perforce. Multiple people are using the
 repo as a remote and its being periodically submitted back to
 perforce.

 It's been working mostly fine. Then one day out of the blue I get this
 error. I can no longer push any git commits to perforce. (This is from
 the remote repo which I am pushing back to perforce)

 user@hostname:~/Source/code$ git p4 submit -M --export-labels
 Perforce checkout for depot path //depot/perforce/workspace/ located
 at /home/user/Source/git-p4-area/perforce/workspace/
 Synchronizing p4 checkout...
 ... - file(s) up-to-date.
 Applying ffa390f comments in config xml files
 //depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened for edit
 //depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened for edit
 //depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened for edit
 //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened for edit
 //depot/perforce/workspace/sub/folder/structure/fifth.xml#1 - opened for edit
 error: patch failed: sub/folder/structure/first.xml:1
 error: sub/folder/structure/first.xml: patch does not apply
 error: patch failed: sub/folder/structure/second.xml:1
 error: sub/folder/structure/second.xml: patch does not apply
 error: patch failed: sub/folder/structure/third.xml:1
 error: sub/folder/structure/third.xml: patch does not apply
 error: patch failed: sub/folder/structure/forth.xml:1
 error: sub/folder/structure/forth.xml: patch does not apply
 error: patch failed: sub/folder/structure/fifth.xml:1
 error: sub/folder/structure/fifth.xml: patch does not apply
 Unfortunately applying the change failed!
 //depot/perforce/workspace/sub/folder/structure/first.xml#1 - was edit, 
 reverted
 //depot/perforce/workspace/sub/folder/structure/second.xml#3 - was
 edit, reverted
 //depot/perforce/workspace/sub/folder/structure/third.xml#3 - was edit, 
 reverted
 //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - was edit, 
 reverted
 //depot/perforce/workspace/sub/folder/structure/fifth.xml#3 - was edit, 
 reverted
 No commits applied.

 I thought it could be the .gitattributes setting that I had which was
 this at the time was this:

 * text eol=lf

 My global core.autocrlf setting was also false.

 So I remade a new remote repo, and changed core.autocrlf to input and
 changed .gitattributes to this

 * text=auto

 *.php text eol=lf
 *.pl text eol=lf
 *.pm text eol=lf
 *.sh text eol=lf

 *.vbs text eol=crlf
 *.bat text eol=crlf
 *.ps1 text eol=crlf

 *.bdb binary
 *.mtr binary

 Then I started to realize that it could just be the files in the
 initial commit that are suspect, because when i made edits to other
 files in the repo then tried to push them back with git p4 submit,
 those files submitted successfully  But the files in the commit where
 I initially got the failure still give me this problem.

 Here's what it looks like when I retested with a fresh git repo cloned
 from perforce with git p4 clone and tried to do the git p4 submit with
 verbose turned on on only one of the suspecting files

 user@hostname:/code$ git p4 submit -M --export-labels --verbose
 Reading pipe: git name-rev HEAD
 Reading pipe: ['git', 'config', 'git-p4.allowSubmit']
 Reading pipe: git rev-parse --symbolic --remotes
 Reading pipe: git rev-parse p4/master
 Reading pipe: git cat-file commit 0457c7589ea679dcc0c9114b34f8f30bc2ee08cf
 Reading pipe: git cat-file commit HEAD~0
 Reading pipe: git cat-file commit HEAD~1
 Reading pipe: ['git', 'config', 'git-p4.conflict']
 Origin branch is remotes/p4/master
 Reading pipe: ['git', 'config', '--bool', 'git-p4.useclientspec']
 Opening pipe: ['p4', '-G', 'where', '//depot/perforce/workspace/...']
 Perforce checkout for depot path //depot/perforce/workspace/ located
 at /home/user/Source/git-p4-area/perforce/workspace/
 Synchronizing p4 checkout...
 ... - file(s) up-to-date.
 Opening pipe: p4 -G opened ...
 Reading pipe: ['git', 'rev-list', '--no-merges', 'remotes/p4/master..master']
 Reading pipe: ['git', 'config', '--bool', 'git-p4.skipUserNameCheck']
 Reading pipe: ['git', 'config', 'git-p4.detectCopies']
 Reading pipe: ['git', 'config', '--bool', 'git-p4.detectCopiesHarder']
 Reading pipe: ['git', 'show', '-s', '--format=format:%h %s',
 'ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e']
 Applying ef3b95f making test change
 Opening pipe: p4 -G users
 Reading pipe: ['git', 'log', '--max-count=1', '--format=%ae',
 'ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e']
 Reading pipe: git diff-tree -r -M
 ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e^
 

Re: 3 way merge during git p4 rebase is attempting to reapply past commits

[Luke Diamand]

On 08/05/13 00:12, Christopher Yee Mon wrote:

Hello,

I have a setup where I have a remote non-bare repo cloned from a
perforce workspace. It is used as a remote repo that people clone into
their own user repos, make commits to, then push back into the remote
repo.
Why is your p4 clone non-bare? I thought pushing into a non-bare repo 
tended to cause problems?


 Then I periodically run the following commands in a script to
 push those changes back to perforce.

% man cron

:-)




git checkout -f
git clean -f
git p4 rebase --import-labels
git p4 submit -M --export-labels
git checkout -f
git clean -f

Sometimes, always after commits from one user's machine specifically,
I get the following error below when pushing back to perforce at the
remote repo. It seems to happen randomly, or at least intermittently,
since I often can't discern any major error during git committing to
the remote repo that precipitates this error. It does happen pretty
reliably when I get a file conflict that I resolve and fix during
committing though.

Performing incremental import into refs/remotes/p4/master git branch
Depot paths: //depot/sub/folder/
No changes to import!
Rebasing the current branch onto remotes/p4/master
First, rewinding head to replay your work on top of it...
Applying: A commit that has already been made previously
Applying: A second commit that has already been made in a previous commit
Using index info to reconstruct a base tree...
stdin:15: space before tab in indent.
 a line of text
stdin:24: space before tab in indent.
 another line of text
stdin:25: space before tab in indent.
 a third line of text
stdin:33: trailing whitespace.
 a forth line of text
stdin:71: trailing whitespace.

warning: squelched 1 whitespace error
warning: 6 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging file from second
CONFLICT (content): Merge conflict in
a/file/in/the/second/pre-existing/commit/file.php
Auto-merging a/file/in/the/second/pre-existing/commit/file.php
Failed to merge in the changes.
Patch failed at 0002 A second commit that has already been made in a
previous commit

When you have resolved this problem run git rebase --continue.
If you would prefer to skip this patch, instead run git rebase --skip.
To check out the original branch and stop rebasing run git rebase --abort.

Traceback (most recent call last):
   File /usr/lib/git-core/git-p4, line 3373, inmodule
 main()
   File /usr/lib/git-core/git-p4, line 3367, in main
 if not cmd.run(args):
   File /usr/lib/git-core/git-p4, line 3150, in run
 return self.rebase()
   File /usr/lib/git-core/git-p4, line 3167, in rebase
 system(git rebase %s % upstream)
   File /usr/lib/git-core/git-p4, line 183, in system
 raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command 'git rebase remotes/p4/master'
returned non-zero exit status 1

The patch is usually one that is already in the remote git repo and in
perforce. At that point I have to run git rebase --skip, to skip the
patch, then rerun the commands in the script again. Sometimes it's
multiple patches that cause this problem and I have to run git rebase
--skip repeatedly. When I check the working copy of the remote repo, I
don't see any changes, no conflict markers, just the file.

The real problem happens when I run git rebase --continue. Usually I
end up with repeated submits in perforce when I do that, which is
obviously a corruption of data.

It sounds a lot like this error, except I don't know how git p4 is
branching, so I don't know how to diagnose it.

http://stackoverflow.com/questions/4033009/git-rebase-conflicts-keep-blocking-progress

I also asked stack overflow and someone there said it's probably the
perforce user being different from the git user info, so I had all the
git users switch to having the same info as the perforce user info and
that did NOT solve the problem.

http://stackoverflow.com/questions/16106900/git-p4-rebase-attempts-to-reapply-past-commits

I'm not sure what could possibly be causing this or how to fix it.
Does anyone have any ideas?

Thanks
Christopher Yee Mon
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git p4 diff-tree ambiguous argument error

[Luke Diamand]

Is this using NFS, or local storage?



On 10/07/14 18:30, Bill Door wrote:

$ git p4 sync --detect-branches --import-labels //main@all
... Lots of useful information elided
fatal: ambiguous argument 'git-p4-tmp/8031': unknown revision or path not in
the working tree.
Use '--' to separate paths from revisions, like this:
'git command [revision...] -- [file...]'
Command failed: ['git', 'diff-tree',
'6b3ef26a3e2635a5ff0170e15fdadb386672f8b9', 'git-p4-tmp/8031']

If I re-run the command, it works the second time. Of course there are
73000+ commits. This is gonna take a while.

I've done some debugging. It appears there is a timing problem between
git-p4 and git.

The failure occurs in P4Sync.searchParent(). Even though a checkpoint is
sent to git (for fast-import) just prior to the call to searchParent() in
importChanges(), the file does not yet exist. I used pdb, paused the program
just before the call to diff-tree and the file was missing. After the
program exits due to the error the file exists (i.e. the OS flushed the
file). This is why re-running continues to work, there is an old file with
basically the same information laying around (dangerous).

How can I get git (fast-import) to flush the file at the right time?

$ git --version
git version 1.7.12.4
$ python --version
Python 2.6.6
OS: GNU/Linux 2.6.32-431.el6.x86_64




--
View this message in context: 
http://git.661346.n2.nabble.com/git-p4-diff-tree-ambiguous-argument-error-tp7614774.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Correction to git-p4 exclude change

[Luke Diamand]

(Resending as plain text).

I could be wrong about this, but my correction above doesn't seem to
be in 'next'. Does that mean (reading your last what's cooking) that
the broken version is going to go out to 'master' soon?

Thanks,
Luke

On 5 February 2015 at 08:19, Luke Diamand l...@diamand.org wrote:
 I could be wrong about this, but my correction above doesn't seem to be in
 'next'. Does that mean (reading your last what's cooking) that the broken
 version is going to go out to 'master' soon?

 Thanks,
 Luke


 On 28 January 2015 at 20:49, Junio C Hamano gits...@pobox.com wrote:

 Luke Diamand l...@diamand.org writes:

  My previous change for adding support for exclude to git-p4 sync
  was incorrect, missing out a comma, which stopped git-p4 from working.
  This change fixes that.
 
  I've also noticed that t9814-git-p4-rename.sh has stopped working; I'm
  going to follow up with a fix for that once I've worked out what's
  wrong with it. There's a small shell syntax problem (missing esac)
  but after fixing that it still fails, so I'm not sure what's happening
  yet. It was discussed a while back.
 
  Luke Diamand (1):
git-p4: correct exclude change
 
   git-p4.py | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

 Thanks.

 Will keep out of 'master' for now.  It may not be a bad idea to
 squash this fix (and any futher ones if needed) into a single patch
 when we rewind 'next' after 2.3 final is tagged.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-p4: support exclude in 'git p4 sync'

[Luke Diamand]

The git-p4 'clone' subcommand has long had the option to specify
parts of the repo to be excluded, on the command line. But this has
not been present in 'sync', which makes it less than useful: as
soon as you do a sync, the excluded parts start being repopulated
as those directories are changed.

(You can achieve the same effect by using a client specification to
do the exclusion, but that's then an extra step).

The code for doing the exclusion is actually all present in the base
'P4Sync' class: this change turns that on by moving the definition
of the command-line switch.

It also updates the documentation and adds a test-case.

Thanks,
Luke

And yes, I'm back to using version control systems other than git :-(

Luke Diamand (1):
  git-p4: support excluding paths on sync

 Documentation/git-p4.txt  |6 ++--
 git-p4.py |   18 ++--
 t/t9817-git-p4-exclude.sh |   71 +
 3 files changed, 83 insertions(+), 12 deletions(-)
 create mode 100755 t/t9817-git-p4-exclude.sh

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-p4: support excluding paths on sync

[Luke Diamand]

The clone subcommand has long had support for excluding
subdirectories, but sync has not. This is a nuisance,
since as soon as you do a sync, any changed files that
were initially excluded start showing up.

Move the exclude command-line option into the parent
class; the actual behavior was already present there so
it simply had to be exposed.

Signed-off-by: Luke Diamand l...@diamand.org
---
 Documentation/git-p4.txt  |6 ++--
 git-p4.py |   18 ++--
 t/t9817-git-p4-exclude.sh |   71 +
 3 files changed, 83 insertions(+), 12 deletions(-)
 create mode 100755 t/t9817-git-p4-exclude.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 6ab5f94..a1664b9 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -241,6 +241,9 @@ Git repository:
Use a client spec to find the list of interesting files in p4.
See the CLIENT SPEC section below.
 
+-/ path::
+   Exclude selected depot paths when cloning or syncing.
+
 Clone options
 ~
 These options can be used in an initial 'clone', along with the 'sync'
@@ -254,9 +257,6 @@ options described above.
 --bare::
Perform a bare clone.  See linkgit:git-clone[1].
 
--/ path::
-   Exclude selected depot paths when cloning.
-
 Submit options
 ~~
 These options can be used to modify 'git p4 submit' behavior.
diff --git a/git-p4.py b/git-p4.py
index ff132b2..38029a4 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1916,6 +1916,9 @@ class P4Sync(Command, P4UserMap):
  help=Keep entire BRANCH/DIR/SUBDIR 
prefix during import),
 optparse.make_option(--use-client-spec, 
dest=useClientSpec, action='store_true',
  help=Only sync files that are included 
in the Perforce Client Spec)
+optparse.make_option(-/, dest=cloneExclude,
+ action=append, type=string,
+ help=exclude depot path),
 ]
 self.description = Imports from Perforce into a git repository.\n
 example:
@@ -1950,6 +1953,12 @@ class P4Sync(Command, P4UserMap):
 if gitConfig(git-p4.syncFromOrigin) == false:
 self.syncWithOrigin = 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)
+
 # Force a checkpoint in fast-import and wait for it to finish
 def checkpoint(self):
 self.gitStream.write(checkpoint\n\n)
@@ -3101,9 +3110,6 @@ class P4Clone(P4Sync):
 optparse.make_option(--destination, dest=cloneDestination,
  action='store', default=None,
  help=where to leave result of the clone),
-optparse.make_option(-/, dest=cloneExclude,
- action=append, type=string,
- help=exclude depot path),
 optparse.make_option(--bare, dest=cloneBare,
  action=store_true, default=False),
 ]
@@ -3111,12 +3117,6 @@ class P4Clone(P4Sync):
 self.needsGit = False
 self.cloneBare = 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)
-
 def defaultDestination(self, args):
 ## TODO: use common prefix of args?
 depotPath = args[0]
diff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh
new file mode 100755
index 000..aac568e
--- /dev/null
+++ b/t/t9817-git-p4-exclude.sh
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='git p4 tests for excluded paths during clone and sync'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+# Create a repo with the structure:
+#
+#//depot/wanted/foo
+#//depot/discard/foo
+#
+# Check that we can exclude a subdirectory with both
+# clone and sync operations.
+
+test_expect_success 'create exclude repo' '
+   (
+   cd $cli 
+   mkdir -p wanted discard 
+   echo wanted wanted/foo 
+   echo discard discard/foo 
+   p4 add wanted/foo discard/foo 
+   p4 submit -d initial revision
+   )
+'
+
+test_expect_success 'check the repo was created correctly' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot/...@all 
+   (
+   cd $git 
+   test_path_is_file wanted/foo 
+   test_path_is_file discard/foo
+   )
+'
+
+test_expect_success 'clone, excluding part of repo' '
+   test_when_finished cleanup_git 
+   git p4 clone

Re: git-p4 is not cloning perforce code properly

[Luke Diamand]

When you say skipping, can you be more specific please? What command
line are you using?

If I want to clone the P4 tree at the current revision, I do something like:

$ git p4 clone //depot/sometree/...

That gets me just a single revision.

If I want all revisions back to the start of time, I do:

$ git p4 clone //depot/sometree/...@all

Thanks,
Luke


On 10 February 2015 at 13:54, Sandeep varshney...@gmail.com wrote:
 Dear All,

 I am trying to clone perforce branch from git to my local drive, but it's
 skipping too many files and change list while fetching it from perforce.

 it'll be very helpful if anyone can suggest me about how to git rid with
 this issue.

 Thanks in Advance,
 Sandeep



 --
 View this message in context: 
 http://git.661346.n2.nabble.com/git-p4-is-not-cloning-perforce-code-properly-tp7625215.html
 Sent from the git mailing list archive at Nabble.com.
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-p4: correct exclude change

[Luke Diamand]

The previous change for excluding paths in the sync
subcommand was incorrect, missing a comma, preventing
git-p4 from working.

Signed-off-by: Luke Diamand l...@diamand.org
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 6ff0b76..549022e 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1915,7 +1915,7 @@ class P4Sync(Command, P4UserMap):
 optparse.make_option(--keep-path, dest=keepRepoPath, 
action='store_true',
  help=Keep entire BRANCH/DIR/SUBDIR 
prefix during import),
 optparse.make_option(--use-client-spec, 
dest=useClientSpec, action='store_true',
- help=Only sync files that are included 
in the Perforce Client Spec)
+ help=Only sync files that are included 
in the Perforce Client Spec),
 optparse.make_option(-/, dest=cloneExclude,
  action=append, type=string,
  help=exclude depot path),
-- 
2.3.0.rc2.188.g4b64765.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Correction to git-p4 exclude change

[Luke Diamand]

My previous change for adding support for exclude to git-p4 sync
was incorrect, missing out a comma, which stopped git-p4 from working.
This change fixes that.

I've also noticed that t9814-git-p4-rename.sh has stopped working; I'm
going to follow up with a fix for that once I've worked out what's
wrong with it. There's a small shell syntax problem (missing esac)
but after fixing that it still fails, so I'm not sure what's happening
yet. It was discussed a while back.

Luke Diamand (1):
  git-p4: correct exclude change

 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.3.0.rc2.188.g4b64765.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-p4: correct --prepare-p4-only instructions

[Luke Diamand]

If you use git-p4 with the --prepare-p4-only option, then
it prints the p4 command line to use. However, the command
line was incorrect: the changelist specification must be
supplied on standard input, not as an argument to p4.

Signed-off-by: Luke Diamand l...@diamand.org
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index ff132b2..90447de 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1442,7 +1442,7 @@ class P4Submit(Command, P4UserMap):
 print+ self.clientPath
 print
 print To submit, use \p4 submit\ to write a new description,
-print or \p4 submit -i %s\ to use the one prepared by \
+print or \p4 submit -i %s\ to use the one prepared by \
\git p4\. % fileName
 print You can delete the file \%s\ when finished. % fileName
 
-- 
2.1.3.1037.g95a6691.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-p4: correct --prepare-p4-only instructions

[Luke Diamand]

This fixes a small error in the command that git-p4 suggests when
the user gives the --prepare-p4-only option.

It tells the user to use p4 submit -i filename but the p4 submit
command reads a change specification on standard input. The correct
command line is therefore:

   p4 submit -i filename

Luke Diamand (1):
  git-p4: correct --prepare-p4-only instructions

 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.1.3.1037.g95a6691.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git=p4.py rebase now honor's client spec

[Luke Diamand]

On 19/03/15 21:58, brian m. carlson wrote:

On Thu, Mar 19, 2015 at 12:28:09PM +, Sam Bishop wrote:

When using the git-p4.py script, I found that if I used a client spec when
cloning out a perforce repository, and then using a git-p4.py rebase, that
the rebase command would be using the current perforce client spec,
instead of the one used when doing the initial clone. My proposed patch
causes the rebase command to switch to the perforce client spec used when
doing the initial git-p4.py clone.



That's very useful, thanks. I've noticed that behaviour in the past and 
always thought it ought to be fixed. As Brian notes, it needs a 
Signed-off line in the patch.


A very small nit: could you prefix the subject with git p4: so that 
it's consistent with other recent git-p4 changes - it makes it easier to 
pick them out when reviewing changes.


Could you run the git-p4 unit tests on it please (t/t98*)? I could be 
wrong about this, but it looks to me like t9806-git-p4-options.sh 
doesn't pass now with this change (*)


Thanks!
Luke

(*) There's at least one test that doesn't pass anyway, but this seems 
to be new.






--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] t9814: Guarantee only one source exists in git-p4 copy tests

[Luke Diamand]

I'm on holiday this week, so I'll not get a chance to look at these
properly until next week.

Luke


On 30 March 2015 at 04:03, Junio C Hamano gits...@pobox.com wrote:
 Vitor Antunes vitor@gmail.com writes:

 * Modify source file (file2) before copying the file.
 * Check that only file2 is the source in the output of p4 filelog.
 * Remove all case statements and replace them simple tests to check that
   source is file2.

 Signed-off-by: Vitor Antunes vitor@gmail.com
 ---

 I am not a Perfoce user, so I'd like to ask Pete's and Luke's
 comments on these changes.

  t/t9814-git-p4-rename.sh |   46 
 +++---
  1 file changed, 31 insertions(+), 15 deletions(-)

 diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
 index 8b9c295..d8fb22d 100755
 --- a/t/t9814-git-p4-rename.sh
 +++ b/t/t9814-git-p4-rename.sh
 @@ -132,6 +132,9 @@ test_expect_success 'detect copies' '
   cd $git 
   git config git-p4.skipSubmitEdit true 

 + echo file8  file2 

 Style: please lose SP between redirection and its target, i.e.

 echo file8 file2 

 The same comment applies to everywhere else.

 + git commit -a -m Differentiate file2 
 + git p4 submit 
   cp file2 file8 
   git add file8 
   git commit -a -m Copy file2 to file8 
 @@ -140,6 +143,10 @@ test_expect_success 'detect copies' '
   p4 filelog //depot/file8 
   p4 filelog //depot/file8 | test_must_fail grep -q branch 
 from 

 + echo file9  file2 
 + git commit -a -m Differentiate file2 
 + git p4 submit 
 +
   cp file2 file9 
   git add file9 
   git commit -a -m Copy file2 to file9 
 @@ -149,28 +156,39 @@ test_expect_success 'detect copies' '
   p4 filelog //depot/file9 
   p4 filelog //depot/file9 | test_must_fail grep -q branch 
 from 

 + echo file10  file2 
 + git commit -a -m Differentiate file2 
 + git p4 submit 
 +
   echo file2 file2 
   cp file2 file10 
   git add file2 file10 
   git commit -a -m Modify and copy file2 to file10 
   git diff-tree -r -C HEAD 
 + src=$(git diff-tree -r -C HEAD | sed 1d | sed 2d | cut -f2) 
 + test $src = file2 
   git p4 submit 
   p4 filelog //depot/file10 
 - p4 filelog //depot/file10 | grep -q branch from //depot/file 
 
 + p4 filelog //depot/file10 | grep -q branch from 
 //depot/file2 
 +
 + echo file11  file2 
 + git commit -a -m Differentiate file2 
 + git p4 submit 

   cp file2 file11 
   git add file11 
   git commit -a -m Copy file2 to file11 
   git diff-tree -r -C --find-copies-harder HEAD 
   src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | 
 cut -f2) 
 - case $src in
 - file2 | file10) : ;; # happy
 - *) false ;; # not
 - esac 
 + test $src = file2 
   git config git-p4.detectCopiesHarder true 
   git p4 submit 
   p4 filelog //depot/file11 
 - p4 filelog //depot/file11 | grep -q branch from //depot/file 
 
 + p4 filelog //depot/file11 | grep -q branch from 
 //depot/file2 
 +
 + echo file12  file2 
 + git commit -a -m Differentiate file2 
 + git p4 submit 

   cp file2 file12 
   echo some text file12 
 @@ -180,15 +198,16 @@ test_expect_success 'detect copies' '
   level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d 
 | cut -f1 | cut -d  -f5 | sed s/C0*//) 
   test -n $level  test $level -gt 0  test $level -lt 
 98 
   src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | 
 cut -f2) 
 - case $src in
 - file10 | file11) : ;; # happy
 - *) false ;; # not
 - esac 
 + test $src = file2 
   git config git-p4.detectCopies $(($level + 2)) 
   git p4 submit 
   p4 filelog //depot/file12 
   p4 filelog //depot/file12 | test_must_fail grep -q branch 
 from 

 + echo file13  file2 
 + git commit -a -m Differentiate file2 
 + git p4 submit 
 +
   cp file2 file13 
   echo different text file13 
   git add file13 
 @@ -197,14 +216,11 @@ test_expect_success 'detect copies' '
   level=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d 
 | cut -f1 | cut -d  -f5 | sed s/C0*//) 
   test -n $level  test $level -gt 2  test $level -lt 
 100 
   src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | 
 cut -f2) 
 - case 

Re: [PATCH] git-p4: fix filetype detection on files opened exclusively

[Luke Diamand]

(+Pete for interest).

On 31 March 2015 at 22:54, Holloway, Blair
blair_hollo...@playstation.sony.com wrote:
 If a Perforce server is configured to automatically set +l (exclusive lock) on
 add of certain file types, git p4 submit will fail during getP4OpenedType, as
 the regex doesn't expect the trailing '*exclusive*' from p4 opened:

Thanks - that actually fixes a part of the long-standing problem of
handling locked files which Pete identified about a year ago.

There's a test case for handling of locked files,
t9816-git-p4-locked.sh, which needs updating now as a bit more of it
passes. Junio, I'll submit a followup patch to update those tests once
I get back from vacation.

(Fixing t9816 is very slightly non-trivial because test #4 is subtly
broken and so incorrectly passes).

Ack,
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 3/4] git-p4: fix filetype detection on files opened exclusively

[Luke Diamand]

From: Holloway, Blair blair_hollo...@playstation.sony.com

If a Perforce server is configured to automatically set +l (exclusive lock) on
add of certain file types, git p4 submit will fail during getP4OpenedType, as
the regex doesn't expect the trailing '*exclusive*' from p4 opened:

//depot/file.png#1 - add default change (binary+l) *exclusive*

Signed-off-by: Blair Holloway blair_hollo...@playstation.sony.com
Acked-by: Luke Diamand l...@diamand.org
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index ff132b2..d43482a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -368,7 +368,7 @@ def getP4OpenedType(file):
 # Returns the perforce file type for the given file.
 
 result = p4_read_pipe([opened, wildcard_encode(file)])
-match = re.match(.*\((.+)\)\r?$, result)
+match = re.match(.*\((.+)\)( \*exclusive\*)?\r?$, result)
 if match:
 return match.group(1)
 else:
-- 
2.3.0.rc1.30.g76afe74

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/4] git-p4: fix small bug in locked test scripts

[Luke Diamand]

Test script t9816-git-p4-locked.sh test #4 tests for
adding a file that is locked by Perforce automatially.
This is currently not supported by git-p4 and so is
expected to fail.

However, a small typo meant it always failed, even with
a fixed git-p4. Fix the typo to resolve this.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9816-git-p4-locked.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index e71e543..ce0eb22 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -41,7 +41,7 @@ test_expect_failure 'add with lock not taken' '
(
cd $git 
echo line1 add-lock-not-taken 
-   git add file2 
+   git add add-lock-not-taken 
git commit -m add add-lock-not-taken 
git config git-p4.skipSubmitEdit true 
git p4 submit --verbose
-- 
2.3.0.rc1.30.g76afe74

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 4/4] git-p4: update locked test case

[Luke Diamand]

The add-new-file and copy-existing-file tests from
t9816-git-p4-locked.sh now pass. Update the test
case accordingly.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9816-git-p4-locked.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index 464f10b..d048bd3 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -35,7 +35,7 @@ test_expect_success 'edit with lock not taken' '
)
 '
 
-test_expect_failure 'add with lock not taken' '
+test_expect_success 'add with lock not taken' '
test_when_finished cleanup_git 
git p4 clone --dest=$git //depot 
(
@@ -107,7 +107,7 @@ test_expect_failure 'chmod with lock taken' '
)
 '
 
-test_expect_failure 'copy with lock taken' '
+test_expect_success 'copy with lock taken' '
lock_in_another_client 
test_when_finished cleanup_git 
test_when_finished cd \$cli\  p4 revert file2  rm -f file2 
-- 
2.3.0.rc1.30.g76afe74

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 2/4] git-p4: small fix for locked-file-move-test

[Luke Diamand]

The test for handling of failure when trying to move a file
that is locked by another client was not quite correct - it
failed early on because the target file in the move already
existed.

The test now fails because git-p4 does not properly detect
that p4 has rejected the move, and instead just crashes. At
present, git-p4 has no support for detecting that a file
has been locked and reporting it to the user, so this is
the expected outcome.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9816-git-p4-locked.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index ce0eb22..464f10b 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -130,8 +130,8 @@ test_expect_failure 'move with lock taken' '
git p4 clone --dest=$git //depot 
(
cd $git 
-   git mv file1 file2 
-   git commit -m mv file1 to file2 
+   git mv file1 file3 
+   git commit -m mv file1 to file3 
git config git-p4.skipSubmitEdit true 
git config git-p4.detectRenames true 
git p4 submit --verbose
-- 
2.3.0.rc1.30.g76afe74

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 0/4] git-p4: fix filetype detection on files opened exclusively

[Luke Diamand]

This is a followup series to Blair's patch to fix filetype detection on files
opened exclusively. It updates the git-p4 unit tests to catchup with that
fix, fixing a couple of small bugs in the original tests.

Holloway, Blair (1):
  git-p4: fix filetype detection on files opened exclusively

Luke Diamand (3):
  git-p4: fix small bug in locked test scripts
  git-p4: small fix for locked-file-move-test
  git-p4: update locked test case

 git-p4.py|  2 +-
 t/t9816-git-p4-locked.sh | 10 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.3.0.rc1.30.g76afe74

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-p4: Use -m when running p4 changes

[Luke Diamand]

On 11 April 2015 at 16:17, Lex Spoon l...@lexspoon.org wrote:


 Signed-off-by: Lex Spoon l...@lexspoon.org
 ---
 This patch addresses a problem I am running into with a client. I am
 attempting to mirror their Perforce repository into Git, and on certain
 branches their Perforce server is responding with an error about too many
 rows scanned. This change has git-p4 use the -m option to return just 500
 changes at a time, thus avoiding the problem.

Thanks - that's a problem I also occasionally hit, and it definitely
needs fixing.

Your fix is quite nice - I started out thinking this should be easy,
but it's not!

A test case addition would be good if you can though - otherwise it's
certain to break at some point in the future. Would you have time to
add that?

Thanks!
Luke



 I have tested this on a small test repository (2000 revisions) and it
 appears to work fine. I have also run all the t98* tests; those print a
 number of yellow not ok results but no red ones. I presume this is the
 expected test behavior?

Yes.


 I considered making the block size configurable, but it seems unlikely
 anyone will strongly benefit from changing it. 500 is large enough that it
 should only take a modest number of iterations to scan the full changes
 list, but it's small enough that any reasonable Perforce server should allow
 the request.

Might be useful when making test harnesses though :-)



 This patch is also available on GitHub:
 https://github.com/lexspoon/git/tree/p4-sync-batches

  git-p4.py | 40 +---
  1 file changed, 33 insertions(+), 7 deletions(-)

 diff --git a/git-p4.py b/git-p4.py
 index 549022e..ce1447b 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -742,15 +742,41 @@ def originP4BranchesExist():

  def p4ChangesForPaths(depotPaths, changeRange):
  assert depotPaths
 -cmd = ['changes']
 -for p in depotPaths:
 -cmd += [%s...%s % (p, changeRange)]
 -output = p4_read_pipe_lines(cmd)

 +# Parse the change range into start and end
 +if changeRange is None or changeRange == '':
 +changeStart = '@1'
 +changeEnd = '#head'
 +else:
 +parts = changeRange.split(',')
 +assert len(parts) == 2
 +changeStart = parts[0]
 +changeEnd = parts[1]
 +
 +# Accumulate change numbers in a dictionary to avoid duplicates
  changes = {}
 -for line in output:
 -changeNum = int(line.split( )[1])
 -changes[changeNum] = True
 +
 +for p in depotPaths:
 +# Retrieve changes a block at a time, to prevent running
 +# into a MaxScanRows error from the server.
 +block_size = 500
 +start = changeStart
 +end = changeEnd
 +get_another_block = True
 +while get_another_block:
 +new_changes = []
 +cmd = ['changes']
 +cmd += ['-m', str(block_size)]
 +cmd += [%s...%s,%s % (p, start, end)]
 +for line in p4_read_pipe_lines(cmd):
 +changeNum = int(line.split( )[1])
 +new_changes.append(changeNum)
 +changes[changeNum] = True
 +if len(new_changes) == block_size:
 +get_another_block = True
 +end = '@' + str(min(new_changes))
 +else:
 +get_another_block = False

  changelist = changes.keys()
  changelist.sort()
 --
 1.9.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] git-p4: Use -m when running p4 changes

[Luke Diamand]

On 18/04/15 00:11, Lex Spoon wrote:

Simply running p4 changes on a large branch can
result in a too many rows scanned error from the
Perforce server. It is better to use a sequence
of smaller calls to p4 changes, using the -m
option to limit the size of each call.

Signed-off-by: Lex Spoon l...@lexspoon.org
Reviewed-by: Junio C Hamano gits...@pobox.com
Reviewed-by: Luke Diamand l...@diamand.org


I could be wrong about this, but it looks like importNewBranches() is 
taking an extra argument, but that isn't reflected in the place where it 
gets called. I think it just got missed.


As a result, t9801-git-p4-branch.sh fails with this error:

Importing revision 3 (37%)
Importing new branch depot/branch1
Traceback (most recent call last):
  File /home/lgd/git/git/git-p4, line 3327, in module
main()
  File /home/lgd/git/git/git-p4, line 3321, in main
if not cmd.run(args):
  File /home/lgd/git/git/git-p4, line 3195, in run
if not P4Sync.run(self, depotPaths):
  File /home/lgd/git/git/git-p4, line 3057, in run
self.importChanges(changes)
  File /home/lgd/git/git/git-p4, line 2692, in importChanges
if self.importNewBranch(branch, change - 1):
TypeError: importNewBranch() takes exactly 4 arguments (3 given)
rm: cannot remove `/home/lgd/git/git/t/trash 
directory.t9801-git-p4-branch/git/.git/objects/pack': Directory not empty

not ok 8 - import depot, branch detection, branchList branch definition


Thanks!
Luke



---
Updated as suggested:
- documentation added
- avoided touch(1)
- used test_seq
- used || exit for test commands inside for loops
- more tabs
- fewer line breaks
- expanded commit message

  Documentation/git-p4.txt | 17 ++---
  git-p4.py| 54 +++-
  t/t9818-git-p4-block.sh  | 64 
  3 files changed, 120 insertions(+), 15 deletions(-)
  create mode 100755 t/t9818-git-p4-block.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index a1664b9..82aa5d6 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -225,9 +225,20 @@ Git repository:
they can find the p4 branches in refs/heads.

  --max-changes n::
-   Limit the number of imported changes to 'n'.  Useful to
-   limit the amount of history when using the '@all' p4 revision
-   specifier.
+   Import at most 'n' changes, rather than the entire range of
+   changes included in the given revision specifier. A typical
+   usage would be use '@all' as the revision specifier, but then
+   to use '--max-changes 1000' to import only the last 1000
+   revisions rather than the entire revision history.
+
+--changes-block-size n::
+   The internal block size to use when converting a revision
+   specifier such as '@all' into a list of specific change
+   numbers. Instead of using a single call to 'p4 changes' to
+   find the full list of changes for the conversion, there are a
+   sequence of calls to 'p4 changes -m', each of which requests
+   one block of changes of the given size. The default block size
+   is 500, which should usually be suitable.

  --keep-path::
The mapping of file names from the p4 depot path to Git, by
diff --git a/git-p4.py b/git-p4.py
index 549022e..1fba3aa 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -740,17 +740,43 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = 
refs/remotes/p4/, silent
  def originP4BranchesExist():
  return gitBranchExists(origin) or gitBranchExists(origin/p4) or 
gitBranchExists(origin/p4/master)

-def p4ChangesForPaths(depotPaths, changeRange):
+def p4ChangesForPaths(depotPaths, changeRange, block_size):
  assert depotPaths
-cmd = ['changes']
-for p in depotPaths:
-cmd += [%s...%s % (p, changeRange)]
-output = p4_read_pipe_lines(cmd)
+assert block_size
+
+# Parse the change range into start and end
+if changeRange is None or changeRange == '':
+changeStart = '@1'
+changeEnd = '#head'
+else:
+parts = changeRange.split(',')
+assert len(parts) == 2
+changeStart = parts[0]
+changeEnd = parts[1]

+# Accumulate change numbers in a dictionary to avoid duplicates
  changes = {}
-for line in output:
-changeNum = int(line.split( )[1])
-changes[changeNum] = True
+
+for p in depotPaths:
+# Retrieve changes a block at a time, to prevent running
+# into a MaxScanRows error from the server.
+start = changeStart
+end = changeEnd
+get_another_block = True
+while get_another_block:
+new_changes = []
+cmd = ['changes']
+cmd += ['-m', str(block_size)]
+cmd += [%s...%s,%s % (p, start, end)]
+for line in p4_read_pipe_lines(cmd):
+changeNum = int(line.split( )[1])
+new_changes.append(changeNum)
+changes

Re: [PATCH v4] git-p4: Use -m when running p4 changes

[Luke Diamand]

Sorry - could you resubmit your patch (PATCHv4 it will be) with this
change squashed in please? It will make life much easier, especially
for Junio!

Thanks!
Luke


On 20 April 2015 at 16:00, Lex Spoon l...@lexspoon.org wrote:
 Simply running p4 changes on a large branch can
 result in a too many rows scanned error from the
 Perforce server. It is better to use a sequence
 of smaller calls to p4 changes, using the -m
 option to limit the size of each call.

 Signed-off-by: Lex Spoon l...@lexspoon.org
 Reviewed-by: Junio C Hamano gits...@pobox.com
 Reviewed-by: Luke Diamand l...@diamand.org
 ---
 Updated to avoid the crash Luke pointed out.
 All t98* tests pass now except for t9814,
 which is already failing on master for some reason.

  Documentation/git-p4.txt | 17 ++---
  git-p4.py| 52 ++-
  t/t9818-git-p4-block.sh  | 64 
 
  3 files changed, 119 insertions(+), 14 deletions(-)
  create mode 100755 t/t9818-git-p4-block.sh

 diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
 index a1664b9..82aa5d6 100644
 --- a/Documentation/git-p4.txt
 +++ b/Documentation/git-p4.txt
 @@ -225,9 +225,20 @@ Git repository:
 they can find the p4 branches in refs/heads.

  --max-changes n::
 -   Limit the number of imported changes to 'n'.  Useful to
 -   limit the amount of history when using the '@all' p4 revision
 -   specifier.
 +   Import at most 'n' changes, rather than the entire range of
 +   changes included in the given revision specifier. A typical
 +   usage would be use '@all' as the revision specifier, but then
 +   to use '--max-changes 1000' to import only the last 1000
 +   revisions rather than the entire revision history.
 +
 +--changes-block-size n::
 +   The internal block size to use when converting a revision
 +   specifier such as '@all' into a list of specific change
 +   numbers. Instead of using a single call to 'p4 changes' to
 +   find the full list of changes for the conversion, there are a
 +   sequence of calls to 'p4 changes -m', each of which requests
 +   one block of changes of the given size. The default block size
 +   is 500, which should usually be suitable.

  --keep-path::
 The mapping of file names from the p4 depot path to Git, by
 diff --git a/git-p4.py b/git-p4.py
 index 549022e..e28033f 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -740,17 +740,43 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = 
 refs/remotes/p4/, silent
  def originP4BranchesExist():
  return gitBranchExists(origin) or gitBranchExists(origin/p4) or 
 gitBranchExists(origin/p4/master)

 -def p4ChangesForPaths(depotPaths, changeRange):
 +def p4ChangesForPaths(depotPaths, changeRange, block_size):
  assert depotPaths
 -cmd = ['changes']
 -for p in depotPaths:
 -cmd += [%s...%s % (p, changeRange)]
 -output = p4_read_pipe_lines(cmd)
 +assert block_size
 +
 +# Parse the change range into start and end
 +if changeRange is None or changeRange == '':
 +changeStart = '@1'
 +changeEnd = '#head'
 +else:
 +parts = changeRange.split(',')
 +assert len(parts) == 2
 +changeStart = parts[0]
 +changeEnd = parts[1]

 +# Accumulate change numbers in a dictionary to avoid duplicates
  changes = {}
 -for line in output:
 -changeNum = int(line.split( )[1])
 -changes[changeNum] = True
 +
 +for p in depotPaths:
 +# Retrieve changes a block at a time, to prevent running
 +# into a MaxScanRows error from the server.
 +start = changeStart
 +end = changeEnd
 +get_another_block = True
 +while get_another_block:
 +new_changes = []
 +cmd = ['changes']
 +cmd += ['-m', str(block_size)]
 +cmd += [%s...%s,%s % (p, start, end)]
 +for line in p4_read_pipe_lines(cmd):
 +changeNum = int(line.split( )[1])
 +new_changes.append(changeNum)
 +changes[changeNum] = True
 +if len(new_changes) == block_size:
 +get_another_block = True
 +end = '@' + str(min(new_changes))
 +else:
 +get_another_block = False

  changelist = changes.keys()
  changelist.sort()
 @@ -1911,7 +1937,10 @@ class P4Sync(Command, P4UserMap):
  optparse.make_option(--import-labels, dest=importLabels, 
 action=store_true),
  optparse.make_option(--import-local, 
 dest=importIntoRemotes, action=store_false,
   help=Import into refs/heads/ , not 
 refs/remotes),
 -optparse.make_option(--max-changes, dest=maxChanges),
 +optparse.make_option(--max-changes, dest=maxChanges,
 + help=Maximum number

Re: git-p4 Question

[Luke Diamand]

On 23/04/15 14:42, FusionX86 wrote:

Hi Luke,

I found a silly mistake I was making in the command I've been using.
The folder under the depot should have been capitalized, but it
wasn't. Also, I expected that if there was a problem with the command,
it would fail with some message instead of creating an empty local git
repo.


I would expect that as well - it will usually create the empty git repo, 
but it should then fail with an error message, like this:


$ git p4 clone //depot/main/nosuchpath
Importing from //depot/main/nosuchpath into nosuchpath
Initialized empty Git repository in 
/home/lgd/p4-hacking/git/nosuchpath/.git/
Doing initial import of //depot/main/nosuchpath/ from revision #head 
into refs/remotes/p4/master

p4 returned an error: //depot/main/nosuchpath/...#head - no such file(s).

$ echo $?
1

If you get a moment can you send your command output; if it's not doing 
something like the above, then it's a bug.


Thanks!

Luke

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fast-import: add options to enable/disable case folding

[Luke Diamand]

On 18/04/15 08:36, Mike Hommey wrote:

On Fri, Apr 17, 2015 at 11:44:00AM -0700, Junio C Hamano wrote:

So perhaps we should rip the case folding out altogether instead?
The entry for the change in the Release Notes may say:

  * git fast-import incorrectly case-folded the paths recorded in
the history when core.ignorease is set (i.e. the repository's
working tree is incapable of expressing paths that differ only in
their cases); this old bug was reported in 2012 and was finally
corrected.

or something like that?


Is anything else then git-p4 known to rely on case folding? If not, I
guess that's a reasonable plan. We could even add an option to
fast-import that would allow to turn case folding back on, and make
git-p4 use it, so that its expectations are fulfilled. Although at some
point, it could (should?) do case folding itself(?)


git-p4 has a single line of code that checks if core.ignorecase is 
turned on, and uses this to decide whether to skip files that are 
outside the depot being tracked and I *think* is not really related to 
fast-import.


I don't know to what extent though git-p4 relies on the current 
behaviour of git fast-import to fold case for it.


There's a 'p4 info' command which tells you what the server thinks it's 
doing:


$ p4 info | grep Case
Case Handling: sensitive

I don't know how long that support has been present (it might not work 
on older servers that some people are still using).


It's also possible to force the server to be case-insensitive on the 
Linux version. That's useful, as it we could construct some test cases 
to see what we're likely to break without having to force people to 
install a case-insensitive OS in order to run the git regression tests.


Luke




Mike
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-p4 Question

[Luke Diamand]

On 24/04/15 15:36, FusionX86 wrote:

I get an error if I misspell part of the path. For example, if I type
//depot/maain instead of //depot/main I will get the no such files
message you indicated. BUT using incorrect case like //depot/main
instead of //depot/Main doesn't return any error, but still completes
and creates an empty repo. If it does require correct case, then it
should throw an error for //depot/main as well.


Thanks, that's a somewhat subtle bug!

Luke

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] git-p4: improve client path detection when branches are used

[Luke Diamand]

On 22/04/15 18:11, Junio C Hamano wrote:

Vitor Antunes vitor@gmail.com writes:


The updates introduced in the third revision of these two patches consist only
on updates to the commit messages to better clarify what they implement.

Vitor Antunes (2):
   t9801: check git-p4's branch detection with client spec enabled
   git-p4: improve client path detection when branches are used

  git-p4.py|   13 --
  t/t9801-git-p4-branch.sh |  106 ++
  2 files changed, 115 insertions(+), 4 deletions(-)


Thanks; will re-queue.  Luke, could you comment?


First off: kudos to Vitor for daring to enter this particular dragon's 
den. The combination of branch-detection and use-client-spec isn't so 
bad, but throwing in the handling of excluding bits of the tree via the 
P4 client spec (like, who would even do that?) makes it into a real mind 
twister!


I've held off commenting as I don't feel I know the branch detection 
code as well as I would like. The change though seems a lot more robust 
now that the search is anchored. Having a test case is always good!


However, playing around with this (incredibly complex and obscure) 
scenario, I'm not yet sure about it.


I created a depot that had //depot/main and //depot/branch, and a branch 
mapping between the two. I cloned that in git using --use-client-spec 
and --branch-detect, and all was well.


I then modified my client spec to exclude //depot/main/excluded, and 
then started adding files in git to the 'excluded' directory. When I 
submit them, I get:


$ echo hello excluded/f1.c
$ echo hello f2.c
$ git add excluded/f1.c f2.c
$ git commit -m 'Partially excluded'
$ git-p4.py submit
DEBUG: self.useClientSpec = True
Perforce checkout for depot path //depot/main/ located at 
/home/lgd/p4-hacking/cli/main/

Synchronizing p4 checkout...
... - file(s) up-to-date.
Applying 51f187b Excluded added from git
excluded/c - file(s) not in client view.
excluded/c - file(s) not opened on this client.
Could not determine file type for excluded/c (result: '')

When I reverted this change, it failed differently, and appeared to be 
extremely confused in the way that I think Vitor originally describes, 
getting hopelessly baffled by the client spec layout.


It's entirely possibly I've messed up my manual testing though. I need 
to go and have a very strong cup of tea before I can look at this again.


Thanks!
Luke


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 0/2] git-p4: improve client path detection when branches are used

[Luke Diamand]

On 23 April 2015 at 09:37, Vitor Antunes vitor@gmail.com wrote:

 That was a good combination to test. In fact, I am using such
 a client spec at my work place to exclude the import from
 Perforce of a folder that only contains binary files, but I never
 even considered to add files to that folder from git!
 Although I do agree that git-p4 should be able to exit sanely
 in this scenario, it is also my opinion that this is a different
 scenario from the one I'm tryig to fix in this set of patches and
 that it should not be enough to stop this merge.

 I will take this scenario into consideration, create a new test
 case and finally fix git-p4 to exit sanely in such a scenario.
 This new test will also be able to show that folder exclusion
 is working perfectly during import, which is important to
 guarantee that that functionality is not broken in future.

 BTW, no kudos is necessary because I've already walked this
 path before :) I've introduced branchList and improved how
 git-p4 looks for the original changelist used to create the new
 branch in Perforce side. If I remember correctly, many of the
 test cases in 9801 file were also created by me before Pete
 started splitting the git-p4 test file into topics.


Yes, I think you're right that this is a different, albeit related
problem. It was broken before, and it's still broken. So I'm happy
with this change - Ack.

Teaching git-p4 to handle this corner case would be a good thing to do
as a separate followup if you're OK to do that.

Thanks!
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] git-p4: Use -m when running p4 changes

[Luke Diamand]

On 20/04/15 16:25, Lex Spoon wrote:

On Mon, Apr 20, 2015 at 11:15 AM, Luke Diamand l...@diamand.org wrote:

Sorry - could you resubmit your patch (PATCHv4 it will be) with this
change squashed in please? It will make life much easier, especially
for Junio!


The message you just responded is already the squashed version. It's a
single patch that includes all changes so far discussed. The subject
line says PATCH v4, although since it's in the same thread, not all
email clients will show the subject change.


Not sure how I missed that! It looks good, now, Ack!

Thanks!
Luke

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-p4 Question

[Luke Diamand]

On 20/04/15 17:41, FusionX86 wrote:

Hello,

Hopefully this is an appropriate place to ask questions about git-p4.

I started at a company that wants to migrate from Perforce to Git. I'm
new to Perforce and have been trying to learn just enough about it to
get through this migration. Anyway, I've been playing with git-p4 and
have one question/problem to discuss.

After setting up the p4 cli client I can 'p4 sync' some
//depot/main/app1 which pulls down the files I would expect from the
Perforce server. If I use 'git p4 clone //depot/main/app1', I get:

Doing initial import of //depot/main/app1/ from revision #head into
refs/remotes/p4/master

But I don't get any files from that depot/folder pulled down. I can
git p4 clone other depot/folders though and get some files. I suspect
that I'm just not understanding how the git-p4 module works.


You could try doing the clone with '-v' to get a bit more information.



Basically, I'm hoping to setup a live sync of Perforce to Git of
certain depots in preparation for the migration. Also, if anyone has
pointers or guides for this type of migration, any help is
appreciated.


I've done something similar in the past. You'll want to enable the 
--preserve-user option, for which you will need admin rights.


If it's a one-way mirror (p4-to-git) then just run git-p4 periodically 
(if you use cron, then try to avoid having two or more instances running 
at the same time).


If you want it to be two-way then it gets a bit more complicated.

You might also want to consider using git fusion, which is Perforce's 
take on this problem. I've not used it myself.


From past experience though I would say the biggest problem is getting 
developers to switch from the P4 mindset (centralized; code review hard 
to do or ignored) to the git mindset (decentralized; code review 
actively supported by the version control system).



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-p4 Question

[Luke Diamand]

Can you post up the output from 'git p4 clone', and also see what the
output from doing this is:

$ p4 print //depot/some/branch/missingfile.c



On 21 April 2015 at 14:33, FusionX86 fusion...@gmail.com wrote:
 Hi Luke,

 Using -v was a good suggestion. Unfortunately I still don't see what
 the problem is. I'm starting to think that maybe I should just create
 the client views I need and setup a cron job that p4 syncs and then
 git commits/pushes.

 The --preserve-user option is for submitting back to Perforce correct?
 I'm hoping to get away with a one-way sync from Perforce to Git...and
 then eventually just cut over to Git.

 I also looked at git fusion, but unfortunately the version of Perforce
 we're running (2012.1) doesn't meet the requirements for fusion. I
 wish it did.

 Good point on developer mindset. I think we definitely have some
 training and habit changing in the future.

 Thanks for the suggestions and pointers, it's much appreciated.

 On Mon, Apr 20, 2015 at 1:26 PM, Luke Diamand l...@diamand.org wrote:
 On 20/04/15 17:41, FusionX86 wrote:

 Hello,

 Hopefully this is an appropriate place to ask questions about git-p4.

 I started at a company that wants to migrate from Perforce to Git. I'm
 new to Perforce and have been trying to learn just enough about it to
 get through this migration. Anyway, I've been playing with git-p4 and
 have one question/problem to discuss.

 After setting up the p4 cli client I can 'p4 sync' some
 //depot/main/app1 which pulls down the files I would expect from the
 Perforce server. If I use 'git p4 clone //depot/main/app1', I get:

 Doing initial import of //depot/main/app1/ from revision #head into
 refs/remotes/p4/master

 But I don't get any files from that depot/folder pulled down. I can
 git p4 clone other depot/folders though and get some files. I suspect
 that I'm just not understanding how the git-p4 module works.


 You could try doing the clone with '-v' to get a bit more information.


 Basically, I'm hoping to setup a live sync of Perforce to Git of
 certain depots in preparation for the migration. Also, if anyone has
 pointers or guides for this type of migration, any help is
 appreciated.


 I've done something similar in the past. You'll want to enable the
 --preserve-user option, for which you will need admin rights.

 If it's a one-way mirror (p4-to-git) then just run git-p4 periodically (if
 you use cron, then try to avoid having two or more instances running at the
 same time).

 If you want it to be two-way then it gets a bit more complicated.

 You might also want to consider using git fusion, which is Perforce's take
 on this problem. I've not used it myself.

 From past experience though I would say the biggest problem is getting
 developers to switch from the P4 mindset (centralized; code review hard to
 do or ignored) to the git mindset (decentralized; code review actively
 supported by the version control system).

 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-p4: add failing tests for case-folding p4d

[Luke Diamand]

(Adding Pete, Vitor, and Fusion in case they have any thoughts on 
working with P4 servers that do case-folding, or at least failing 
gracefully).


On 29/04/15 00:01, Lex Spoon wrote:

The last comment in the test took me a minute to decipher. I would
suggest no repo path called LC instead of no repo called LC. Also,
it would have helped me to either have a little comment on the UC
version of the test, or to make the previous comment a little more
neutral so that it will apply to both test cases.


OK, thanks!



Otherwise, while I am not a regular maintainer of this code, the patch
does LGTM. Certainly it's good to have more test coverage.

For the underlying problem, I haven't thought about it very much, but
it looks like a plausible first step might be to simply probe the
given file name and see if it comes back the same way. If it comes
back differently, then maybe the command should abort?


I think the problem may be a bit trickier than that.

I think what's happening when cloning is that when files come back from 
the server, git-p4 checks that they are contained within the directory 
it is cloning. This happens in p4StartsWith(), (called from 
extractFilesFromCommit()) which already tries to fix this problem by 
checking 'core.ignorecase'. However, that won't work if the local 
machine is case sensitive but the server isn't (e.g. Linux client, 
Windows server).


git-p4 does this because it's fetching *commits* from Perforce, and a 
commit might have files that are outside the directory being cloned.


I tried teaching p4StartsWith() to ask the server if it is case-folding 
('p4 info') and that then means that the git-p4 clone actually succeeds. 
However, git-p4 submit then fails because it gets terribly confused 
about pathnames - it probably needs to do some lowercasing somewhere. So 
that might be worth pursuing.


Open to other suggestions though!




What a tough problem all around...


Indeed!

Luke

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-p4: prevent --chain-lint failure

[Luke Diamand]

t9814 has a test that simply sets up a pre-requisite for
another test, and as such, always succeeds. The way it was
written doesn't quite work with the test lint checks introduced
with the --chain-lint option.

Add an additional layer of {} to prevent the --chain-lint
code getting confused.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9814-git-p4-rename.sh | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 99bb71b..14f9dc3 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -227,13 +227,15 @@ test_expect_success 'detect copies' '
 # See if configurables can be set, and in particular if the run.move.allow
 # variable exists, which allows admins to disable the p4 move command.
 test_expect_success 'p4 configure command and run.move.allow are available' '
-   p4 configure show run.move.allow out ; retval=$? 
-   test $retval = 0 
-   {
-   egrep ^run.move.allow: out 
-   test_set_prereq P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW ||
-   true
-   } || true
+{
+   p4 configure show run.move.allow out ; retval=$? 
+   test $retval = 0 
+   {
+   egrep ^run.move.allow: out 
+   test_set_prereq P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW ||
+   true
+   } || true
+}
 '
 
 # If move can be disabled, turn it off and test p4 move handling
-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Fixup test-lint error in git-p4 t9814 test

[Luke Diamand]

While running the git-p4 tests, I noticed that t9814 has started
failing due to the (very ingenious!) chain-lint detection introduced
in:

   bb79af9 t/test-lib: introduce --chain-lint option

I think that what's going on is that the chain-lint test is
getting itself confused by this test, which is designed to always
succeed, regardless of whether the individual sub-commands succeed
or not, since it's just setting up a pre-requisite for later use.

I've added an additional set of braces, which makes it clearer
to the --chain-lint code what's going on, but I'd be interested to
know if this is the right way to fix this.

Thanks,
Luke

Luke Diamand (1):
  git-p4: prevent --chain-lint failure

 t/t9814-git-p4-rename.sh | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-p4: add failing tests for case-folding p4d

[Luke Diamand]

When p4d runs on a case-folding OS, git-p4 can end up getting
very confused. This adds failing tests to demonstrate the problem.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/lib-git-p4.sh|  2 +-
 t/t9819-git-p4-case-folding.sh | 54 ++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100755 t/t9819-git-p4-case-folding.sh

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 5aa8adc..7548225 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -69,7 +69,7 @@ start_p4d() {
(
cd $db 
{
-   p4d -q -p $P4DPORT 
+   p4d -q -p $P4DPORT $@ 
echo $! $pidfile
}
) 
diff --git a/t/t9819-git-p4-case-folding.sh b/t/t9819-git-p4-case-folding.sh
new file mode 100755
index 000..78f1d0f
--- /dev/null
+++ b/t/t9819-git-p4-case-folding.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+test_description='interaction with P4 case-folding'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d with case folding enabled' '
+   start_p4d -C1
+'
+
+test_expect_success 'Create a repo, name is lowercase' '
+   (
+   client_view //depot/... //client/... 
+   cd $cli 
+   mkdir -p lc UC 
+   lc/file.txt  UC/file.txt 
+   p4 add lc/file.txt UC/file.txt 
+   p4 submit -d Add initial lc and UC repos
+   )
+'
+
+test_expect_success 'Check p4 is in case-folding mode' '
+   (
+   cd $cli 
+   lc/FILE.TXT 
+   p4 add lc/FILE.TXT 
+   test_must_fail p4 submit -d Cannot add file differing only in 
case lc/FILE.TXT
+   )
+'
+
+# Check we created the repo properly
+test_expect_success 'Clone lc repo using lc name' '
+   git p4 clone //depot/lc/... 
+   test_path_is_file lc/file.txt 
+   git p4 clone //depot/UC/... 
+   test_path_is_file UC/file.txt
+'
+
+# The clone should fail, since there is no repo called LC, but because
+# we have case-insensitive p4d enabled, it appears to go ahead and work,
+# but leaves an empty git repo in place.
+test_expect_failure 'Clone lc repo using uc name' '
+   test_must_fail git p4 clone //depot/LC/...
+'
+
+test_expect_failure 'Clone UC repo with lc name' '
+   test_must_fail git p4 clone //depot/uc/...
+'
+
+test_expect_success 'kill p4d' '
+   kill_p4d
+'
+
+test_done
-- 
2.4.0.rc3.380.g8e2ddc7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-p4: add failing tests for case-folding in p4d

[Luke Diamand]

Lex found out recently that when git-p4 is asked to clone a repo
and the case of the repo is incorrect (but otherwise correct) that
git-p4, instead of reporting an error, appears to work fine, but
actually produces an empty repo. This can be quite confusing.

This patch adds a couple of failing test cases that illustrate the
problem.

The next step is to figure out where it's going wrong, and how it
should actually behave.

Luke Diamand (1):
  git-p4: add failing tests for case-folding p4d

 t/lib-git-p4.sh|  2 +-
 t/t9819-git-p4-case-folding.sh | 54 ++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100755 t/t9819-git-p4-case-folding.sh

-- 
2.4.0.rc3.380.g8e2ddc7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3] git-p4: t9814: prevent --chain-lint failure

[Luke Diamand]

Use test_lazy_prereq to setup prerequisites for the p4 move
test. This both makes the test simpler and clearer, and also
means it no longer fails the new --chain-lint tests.

Suggested-by: Jeff King p...@peff.net
Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9814-git-p4-rename.sh | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 99bb71b..c89992c 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -226,14 +226,9 @@ test_expect_success 'detect copies' '
 
 # See if configurables can be set, and in particular if the run.move.allow
 # variable exists, which allows admins to disable the p4 move command.
-test_expect_success 'p4 configure command and run.move.allow are available' '
-   p4 configure show run.move.allow out ; retval=$? 
-   test $retval = 0 
-   {
-   egrep ^run.move.allow: out 
-   test_set_prereq P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW ||
-   true
-   } || true
+test_lazy_prereq P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW '
+   p4 configure show run.move.allow out 
+   egrep ^run.move.allow: out
 '
 
 # If move can be disabled, turn it off and test p4 move handling
-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3] Fixup test-lint error in git-p4 t9814 test

[Luke Diamand]

Using Jeff's suggestion of converting the t9814 test to use
test_lazy_prereq makes the test a lot clearer, and as a bonus,
also fixes the --chain-lint error.

Version 3 of the patch corrects a small typo in the commit message
of version 2.

Luke Diamand (1):
  git-p4: t9814: prevent --chain-lint failure

 t/t9814-git-p4-rename.sh | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] git-p4: t9814: prevent --chain-lint failure

[Luke Diamand]

Use test_lazy_prereq to setup prerequisites for the p4 move
test. This both makes the test simpler and clearer, and also
means they no longer fail the new --chain-lint tests.

Suggested-by: Jeff King p...@peff.net
Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9814-git-p4-rename.sh | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 99bb71b..c89992c 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -226,14 +226,9 @@ test_expect_success 'detect copies' '
 
 # See if configurables can be set, and in particular if the run.move.allow
 # variable exists, which allows admins to disable the p4 move command.
-test_expect_success 'p4 configure command and run.move.allow are available' '
-   p4 configure show run.move.allow out ; retval=$? 
-   test $retval = 0 
-   {
-   egrep ^run.move.allow: out 
-   test_set_prereq P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW ||
-   true
-   } || true
+test_lazy_prereq P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW '
+   p4 configure show run.move.allow out 
+   egrep ^run.move.allow: out
 '
 
 # If move can be disabled, turn it off and test p4 move handling
-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] Fixup test-lint error in git-p4 t9814 test

[Luke Diamand]

Using Jeff's suggestion of converting the t9814 test to use
test_lazy_prereq makes the test a lot clearer, and as a bonus,
also fixes the --chain-lint error.

Thanks,
Luke

Luke Diamand (1):
  git-p4: t9814: prevent --chain-lint failure

 t/t9814-git-p4-rename.sh | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-p4: add failing tests for case-folding in p4d

[Luke Diamand]

On 28/04/15 10:08, Luke Diamand wrote:

Lex found out recently that when git-p4 is asked to clone a repo
and the case of the repo is incorrect (but otherwise correct) that
git-p4, instead of reporting an error, appears to work fine, but
actually produces an empty repo. This can be quite confusing.


Oops, not Lex, but Fusion, noticed the problem!

Thanks!
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git p4 clone - exclude file types

[Luke Diamand]

On 18/05/15 18:59, FusionX86 wrote:

Hello,

Anyone know of a way to 'git p4 clone' and exclude files by type or
name? For example, I want to clone a depot, but not pull down any .exe
files. Haven't been able to find an answer in docs or other searches.


I think you can use a client spec which excludes the files you want.

First, create a client spec that excludes the files you don't want:

Client: myclient
View:
//depot/mystuff/...  //myclient/...
-//depot/mystuff/...exe  //myclient/...exe

Then clone with the --use-client-spec option:

$ export P4CLIENT=myclient
$ git p4 clone --use-client-spec //depot/mystuff

And later on, when you want to catch up:

$ cd mystuff
$ git p4 sync --use-client-spec

Luke

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/2] git-p4: add failing test for P4EDITOR handling

[Luke Diamand]

Add test case that git-p4 handles a setting of P4EDITOR
that takes arguments, e.g. gvim -f

Signed-off-by: Luke Diamand l...@diamand.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t9820-git-p4-editor-handling.sh | 38 ++
 1 file changed, 38 insertions(+)
 create mode 100755 t/t9820-git-p4-editor-handling.sh

diff --git a/t/t9820-git-p4-editor-handling.sh 
b/t/t9820-git-p4-editor-handling.sh
new file mode 100755
index 000..e0a3c52
--- /dev/null
+++ b/t/t9820-git-p4-editor-handling.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='git p4 handling of EDITOR'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+test_expect_success 'init depot' '
+   (
+   cd $cli 
+   echo file1 file1 
+   p4 add file1 
+   p4 submit -d file1
+   )
+'
+
+test_expect_failure 'EDITOR has options' '
+# Check that the P4EDITOR argument can be given command-line
+# options, which git-p4 will then pass through to the shell.
+   git p4 clone --dest=$git //depot 
+   test_when_finished cleanup_git 
+   (
+   cd $git 
+   echo change file1 
+   git commit -m change file1 
+   P4EDITOR=touch \$git/touched\ git p4 submit 
+   test_path_is_file $git/touched
+   )
+'
+
+test_expect_success 'kill p4d' '
+   kill_p4d
+'
+
+test_done
-- 
2.4.0.rc3.380.g8e2ddc7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 2/2] git-p4: fix handling of multi-word P4EDITOR

[Luke Diamand]

This teaches git-p4 to pass the P4EDITOR variable to the
shell for expansion, so that any command-line arguments are
correctly handled. Without this, git-p4 can only launch the
editor if P4EDITOR is solely the path to the binary, without
any arguments.

This also fixes t9805, which relied on the previous
behaviour.

Suggested-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Luke Diamand l...@diamand.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-p4.py  | 2 +-
 t/t9805-git-p4-skip-submit-edit.sh | 2 +-
 t/t9820-git-p4-editor-handling.sh  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 41a77e6..ca6bb95 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1248,7 +1248,7 @@ class P4Submit(Command, P4UserMap):
 editor = os.environ.get(P4EDITOR)
 else:
 editor = read_pipe(git var GIT_EDITOR).strip()
-system([editor, template_file])
+system([sh, -c, ('%s $@' % editor), editor, template_file])
 
 # If the file was not saved, prompt to see if this patch should
 # be skipped.  But skip this verification step if configured so.
diff --git a/t/t9805-git-p4-skip-submit-edit.sh 
b/t/t9805-git-p4-skip-submit-edit.sh
index 8931188..5fbf904 100755
--- a/t/t9805-git-p4-skip-submit-edit.sh
+++ b/t/t9805-git-p4-skip-submit-edit.sh
@@ -90,7 +90,7 @@ test_expect_success 'no config, edited' '
cd $git 
echo line file1 
git commit -a -m change 5 
-   P4EDITOR=$TRASH_DIRECTORY/ed.sh 
+   P4EDITOR=\$TRASH_DIRECTORY/ed.sh\ 
export P4EDITOR 
git p4 submit 
p4 changes //depot/... wc 
diff --git a/t/t9820-git-p4-editor-handling.sh 
b/t/t9820-git-p4-editor-handling.sh
index e0a3c52..c178bd7 100755
--- a/t/t9820-git-p4-editor-handling.sh
+++ b/t/t9820-git-p4-editor-handling.sh
@@ -17,9 +17,9 @@ test_expect_success 'init depot' '
)
 '
 
-test_expect_failure 'EDITOR has options' '
 # Check that the P4EDITOR argument can be given command-line
 # options, which git-p4 will then pass through to the shell.
+test_expect_success 'EDITOR has options' '
git p4 clone --dest=$git //depot 
test_when_finished cleanup_git 
(
-- 
2.4.0.rc3.380.g8e2ddc7

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-p4: Use -m when running p4 changes

[Luke Diamand]

On 15/04/15 04:47, Lex Spoon wrote:

 From 9cc607667a20317c837afd90d50c078da659b72f Mon Sep 17 00:00:00 2001
From: Lex Spoon l...@lexspoon.org
Date: Sat, 11 Apr 2015 10:01:15 -0400
Subject: [PATCH] git-p4: Use -m when running p4 changes


This patch didn't want to apply for me, I'm not quite sure why but 
possibly it's become scrambled? Either that or I'm doing it wrong! If 
you use git send-email it should Just Work.


As an aside could you post reworked versions of patches with a subject 
line of [PATCH v2], [PATCH v3], etc, so reviewers can keep track of 
what's going on?


Note to other reviewers: the existing git-p4 has a --max-changes option 
for 'sync', but this doesn't do the same thing at all. It doesn't limit 
the number of changes requested from the server, it just limits the 
number of changes pulled down, after the p4 server has supplied those 
changes. This confused me at first!


Lex - I should have mentioned this before, but would you be able to add 
some documentation to Documentation/git-p4.txt to explain what your new 
option does? It would help to distinguish between your option and the 
existing --max-changes option.


I've put a few remarks below in your shell script; there are a few minor 
issues that could do with being tidied up.


Thanks!
Luke

snip


diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
new file mode 100755
index 000..73e545d
--- /dev/null
+++ b/t/t9818-git-p4-block.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+
+test_description='git p4 fetching changes in multiple blocks'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'Create a repo with 100 changes' '
+ (
+ cd $cli 


This doesn't look like enough indentation. The tests normally get a hard 
tab indent at each level.



+ touch file.txt 
+ p4 add file.txt 
+ p4 submit -d Add file.txt 
+ for i in 0 1 2 3 4 5 6 7 8 9
+ do
+ touch outer$i.txt 
+ p4 add outer$i.txt 
+ p4 submit -d Adding outer$i.txt 
+ for j in 0 1 2 3 4 5 6 7 8 9
+ do
+ p4 edit file.txt 
+ echo $i$j  file.txt 


Please put the file argument immediately after the redirection, i.e.

   echo $i$j file.txt 

(Which you've done below in fact).


+ p4 submit -d Commit $i$j
+ done
+ done
+ )
+'
+
+test_expect_success 'Clone the repo' '
+ git p4 clone --dest=$git --changes-block-size=10 --verbose //depot@all
+'
+
+test_expect_success 'All files are present' '
+ echo file.txt expected 
+ test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt
outer4.txt expected 
+ test_write_lines outer5.txt outer6.txt outer7.txt outer8.txt
outer9.txt expected 
+ ls $git current 
+ test_cmp expected current
+'
+
+test_expect_success 'file.txt is correct' '
+ echo 99 expected 
+ test_cmp expected $git/file.txt
+'
+
+test_expect_success 'Correct number of commits' '
+ (cd $git; git log --oneline) log 


Use  rather than ;


+ test_line_count = 111 log
+'
+
+test_expect_success 'Previous version of file.txt is correct' '
+ (cd $git; git checkout HEAD^^) 


As above.


+ echo 97 expected 
+ test_cmp expected $git/file.txt
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done



Looks good other than that (+Junio's comments).

Thanks!
Luke


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/3] git-p4: fix filetype detection on files opened exclusively

[Luke Diamand]

From: Holloway, Blair blair_hollo...@playstation.sony.com

If a Perforce server is configured to automatically set +l (exclusive lock) on
add of certain file types, git p4 submit will fail during getP4OpenedType, as
the regex doesn't expect the trailing '*exclusive*' from p4 opened:

//depot/file.png#1 - add default change (binary+l) *exclusive*

Signed-off-by: Blair Holloway blair_hollo...@playstation.sony.com
Acked-by: Luke Diamand l...@diamand.org
Signed-off-by: Luke Diamand l...@diamand.org
---
 git-p4.py| 2 +-
 t/t9816-git-p4-locked.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index ff132b2..d43482a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -368,7 +368,7 @@ def getP4OpenedType(file):
 # Returns the perforce file type for the given file.
 
 result = p4_read_pipe([opened, wildcard_encode(file)])
-match = re.match(.*\((.+)\)\r?$, result)
+match = re.match(.*\((.+)\)( \*exclusive\*)?\r?$, result)
 if match:
 return match.group(1)
 else:
diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index 464f10b..d048bd3 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -35,7 +35,7 @@ test_expect_success 'edit with lock not taken' '
)
 '
 
-test_expect_failure 'add with lock not taken' '
+test_expect_success 'add with lock not taken' '
test_when_finished cleanup_git 
git p4 clone --dest=$git //depot 
(
@@ -107,7 +107,7 @@ test_expect_failure 'chmod with lock taken' '
)
 '
 
-test_expect_failure 'copy with lock taken' '
+test_expect_success 'copy with lock taken' '
lock_in_another_client 
test_when_finished cleanup_git 
test_when_finished cd \$cli\  p4 revert file2  rm -f file2 
-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/3] git-p4: small fix for locked-file-move-test

[Luke Diamand]

The test for handling of failure when trying to move a file
that is locked by another client was not quite correct - it
failed early on because the target file in the move already
existed.

The test now fails because git-p4 does not properly detect
that p4 has rejected the move, and instead just crashes. At
present, git-p4 has no support for detecting that a file
has been locked and reporting it to the user, so this is
the expected outcome.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9816-git-p4-locked.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index ce0eb22..464f10b 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -130,8 +130,8 @@ test_expect_failure 'move with lock taken' '
git p4 clone --dest=$git //depot 
(
cd $git 
-   git mv file1 file2 
-   git commit -m mv file1 to file2 
+   git mv file1 file3 
+   git commit -m mv file1 to file3 
git config git-p4.skipSubmitEdit true 
git config git-p4.detectRenames true 
git p4 submit --verbose
-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/3] git-p4: fix small bug in locked test scripts

[Luke Diamand]

Test script t9816-git-p4-locked.sh test #4 tests for
adding a file that is locked by Perforce automatically.
This is currently not supported by git-p4 and so is
expected to fail.

However, a small typo meant it always failed, even with
a fixed git-p4. Fix the typo to resolve this.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9816-git-p4-locked.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
index e71e543..ce0eb22 100755
--- a/t/t9816-git-p4-locked.sh
+++ b/t/t9816-git-p4-locked.sh
@@ -41,7 +41,7 @@ test_expect_failure 'add with lock not taken' '
(
cd $git 
echo line1 add-lock-not-taken 
-   git add file2 
+   git add add-lock-not-taken 
git commit -m add add-lock-not-taken 
git config git-p4.skipSubmitEdit true 
git p4 submit --verbose
-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/3] git-p4: updated locked file handling patch series

[Luke Diamand]

Updated patch series for fixing git-p4 filetype detection when one or more
files have been locked automatically by p4 (fix provided by Blair),
incorporating comments from Eric:

 - squashes the actual fix and the test case change together
 - fixes typo

Luke

Holloway, Blair (1):
  git-p4: fix filetype detection on files opened exclusively

Luke Diamand (2):
  git-p4: fix small bug in locked test scripts
  git-p4: small fix for locked-file-move-test

 git-p4.py|  2 +-
 t/t9816-git-p4-locked.sh | 10 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] git-p4: Improve client path detection

[Luke Diamand]

On 28/03/15 12:28, Vitor Antunes wrote:

I'm adding a test case for a scenario I was confronted with when using branch
detection and a client view specification. It is possible that the implemented
fix may not cover all possible scenarios, but there is no regression in the
available tests.


Vitor, one thing I wondered about with this part of the change:

-if entry[depotFile] == depotPath:
+if entry[depotFile].find(depotPath) = 0:

Does this mean that if 'p4 where' produces multiple lines of output that 
this will get confused, as it's just going to search for an instance of 
depotPath.


The example in the Perforce man page for 'p4 where' would trigger this 
for example:


http://www.perforce.com/perforce/r14.2/manuals/cmdref/p4_where.html

-//a/b/file.txt //client/a/b/file.txt //home/user/root/a/b/file.txt
//a/b/file.txt //client/b/file.txt /home/user/root/b/file.txt

As an experiment, I hacked git-p4 to always use p4Where rather than 
getClientRoot(), which I would have thought ought to work, but while 
most of the tests passed, Pete's client-spec torture tests failed.


Luke




Vitor Antunes (2):
   git-p4: Check branch detection and client view together
   git-p4: Improve client path detection when branches are used

  git-p4.py|   11 --
  t/t9801-git-p4-branch.sh |   98 ++
  2 files changed, 105 insertions(+), 4 deletions(-)



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 1/3] git-p4: add failing test for P4EDITOR handling

[Luke Diamand]

On 20/05/15 21:56, Junio C Hamano wrote:

Junio C Hamano gits...@pobox.com writes:


Luke Diamand l...@diamand.org writes:


+
+test_expect_failure 'EDITOR has options' '
+# Check that the P4EDITOR argument can be given command-line
+# options, which git-p4 will then pass through to the shell.
+test_expect_success 'EDITOR has options' '
+   git p4 clone --dest=$git //depot 


Oops?  I assume that the one before the comment should go and this
one is

test_expect_failure 'Editor with an option' '

or something.


I'll queue the three patches, each of them followed with its own
SQUASH commit.  Could you sanity check them?  If everything looks OK
then I'll just squash them and that way we can save back-and-forth.


That would be great, thanks!


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv1 3/3] git-p4: fixing --changes-block-size handling

[Luke Diamand]

On 07/06/15 17:33, Lex Spoon wrote:

The implementation looks fine, especially given the test cases that
back it up. I am only curious why the block size is set to a default
of None. To put it as contcretely as possible: is there any expected
configuration where None would work but 500 would not? We know there
are many cases of the other way around, and those cases are going to
send users to StackOverflow to find the right workaround.


I think it was just caution: it's pretty easy to make it fall back to 
the old non-batched scheme, so if it turns out that there *is* a 
problem, fewer people will hit the problem and we're less likely to have 
a paper-bag release.




Dropping the option would also simplify the code in several places.
The complex logic around get_another_block could be removed, and
instead there could be a loop from start to mostRecentCommit by
block_size. Several places that check if not block_size could just
choose the other branch.


Fair point. I'll give it a go and see what happens.

(Plus 500 is a very unnatural number, chosen just because we still place 
some kind of significance on a chance evolutionary accident that gave 
our ape ancestors 5 digits on each hand :-)


Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv1 0/3] git-p4: fixing --changes-block-size support

[Luke Diamand]

On 07/06/15 17:01, Lex Spoon wrote:

Great work.


Thanks! I actually found the problem in my day job, so it was very handy 
having all the infrastructure already in place!


For curiosity's sake, the -m solution has been observed to work on at
least one Perforce installation. However clearly it doesn't work on
others, so the batch ranges approach looks like it will be better.


Yes, I can easily imagine that it's changed from one version to the 
next. I tried going back to a 2014.2 server which still had the same 
problem (with maxresults), but my investigations were not very exhaustive!




Based on what has been seen so far, the Perforce maxscanrows setting
must be applying the low-level database queries that Perforce uses
internally in its implementation. That makes the precise effect on
external queries rather hard to predict. It likely also depends on the
version of Perforce.


Indeed. All sorts of things can cause it to fail; I've seen it reject 
p4 files and p4 print, albeit with artificially low maxscanrows and 
maxresults values. I think this means there's no way to ever make it 
reliably work for all possible sizes of depot and values of 
maxscanrows/maxresults.


Luke

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 2/3] git-p4: test with limited p4 server results

[Luke Diamand]

Change the --changes-block-size git-p4 test to use an account with
limited maxresults and maxscanrows values.

These conditions are applied in the server *before* the -m maxchanges
parameter to p4 changes is applied, and so the strategy that git-p4
uses for limiting the number of changes does not work. As a result,
the tests all fail.

Note that maxscanrows is set quite high, as it appears to not only
limit results from p4 changes, but *also* limits results from
p4 print. Files that have more than maxscanrows changes seem
(experimentally) to be impossible to print. There's no good way to
work around this.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9818-git-p4-block.sh | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 79765a4..aae1121 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -8,6 +8,19 @@ test_expect_success 'start p4d' '
start_p4d
 '
 
+create_restricted_group() {
+   p4 group -i -EOF
+   Group: restricted
+   MaxResults: 7
+   MaxScanRows: 40
+   Users: author
+   EOF
+}
+
+test_expect_success 'Create group with limited maxrows' '
+   create_restricted_group
+'
+
 test_expect_success 'Create a repo with many changes' '
(
client_view //depot/included/... //client/included/... \
@@ -32,11 +45,15 @@ test_expect_success 'Create a repo with many changes' '
)
 '
 
-test_expect_success 'Clone the repo' '
+test_expect_success 'Default user cannot fetch changes' '
+   ! p4 changes -m 1 //depot/...
+'
+
+test_expect_failure 'Clone the repo' '
git p4 clone --dest=$git --changes-block-size=7 --verbose 
//depot/included@all
 '
 
-test_expect_success 'All files are present' '
+test_expect_failure 'All files are present' '
echo file.txt expected 
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt 
expected 
test_write_lines outer5.txt expected 
@@ -44,18 +61,18 @@ test_expect_success 'All files are present' '
test_cmp expected current
 '
 
-test_expect_success 'file.txt is correct' '
+test_expect_failure 'file.txt is correct' '
echo 55 expected 
test_cmp expected $git/file.txt
 '
 
-test_expect_success 'Correct number of commits' '
+test_expect_failure 'Correct number of commits' '
(cd $git  git log --oneline) log 
wc -l log 
test_line_count = 43 log
 '
 
-test_expect_success 'Previous version of file.txt is correct' '
+test_expect_failure 'Previous version of file.txt is correct' '
(cd $git  git checkout HEAD^^) 
echo 53 expected 
test_cmp expected $git/file.txt
@@ -85,7 +102,7 @@ test_expect_success 'Add some more files' '
 
 # This should pick up the 10 new files in included, but not be confused
 # by the additional files in excluded
-test_expect_success 'Syncing files' '
+test_expect_failure 'Syncing files' '
(
cd $git 
git p4 sync --changes-block-size=7 
-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 3/3] git-p4: fixing --changes-block-size handling

[Luke Diamand]

The --changes-block-size handling was intended to help when
a user has a limited maxscanrows (see p4 group). It used
p4 changes -m $maxchanges to limit the number of results.

Unfortunately, it turns out that the maxscanrows and maxresults
limits are actually applied *before* the -m maxchanges parameter
is considered (experimentally).

Fix the block-size handling so that it gets blocks of changes
limited by revision number ($Start..$Start+$N, etc). This limits
the number of results early enough that both sets of tests pass.

If the --changes-block-size option is not in use, then the code
naturally falls back to the original scheme and gets as many changes
as possible.

Unfortunately, it also turns out that p4 print can fail on
files with more changes than maxscanrows. This fix is unable to
workaround this problem, although in the real world this shouldn't
normally happen.

Signed-off-by: Luke Diamand l...@diamand.org
---
 git-p4.py   | 48 +++-
 t/t9818-git-p4-block.sh | 12 ++--
 2 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 26ad4bc..0e29b75 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -744,41 +744,63 @@ def originP4BranchesExist():
 
 def p4ChangesForPaths(depotPaths, changeRange, block_size):
 assert depotPaths
-assert block_size
 
 # Parse the change range into start and end
 if changeRange is None or changeRange == '':
-changeStart = '@1'
-changeEnd = '#head'
+changeStart = 1
+changeEnd = None
 else:
 parts = changeRange.split(',')
 assert len(parts) == 2
-changeStart = parts[0]
-changeEnd = parts[1]
+changeStart = int(parts[0][1:])
+if parts[1] == '#head':
+changeEnd = None
+else:
+changeEnd = int(parts[1])
 
 # Accumulate change numbers in a dictionary to avoid duplicates
 changes = {}
 
+# We need the most recent change list number if we're operating in
+# batch mode. For whatever reason, clients with limited MaxResults
+# can get this for the entire depot, but not for individual bits of
+# the depot.
+if block_size:
+results = p4CmdList([changes, -m, 1])
+mostRecentCommit = int(results[0]['change'])
+
 for p in depotPaths:
 # Retrieve changes a block at a time, to prevent running
 # into a MaxScanRows error from the server.
 start = changeStart
-end = changeEnd
 get_another_block = True
 while get_another_block:
 new_changes = []
 cmd = ['changes']
-cmd += ['-m', str(block_size)]
-cmd += [%s...%s,%s % (p, start, end)]
+
+if block_size:
+end = changeStart + block_size# only fetch a few at a time
+else:
+end = changeEnd # fetch as many as possible
+
+if end:
+endStr = str(end)
+else:
+endStr = '#head'
+
+cmd += [%s...@%d,%s % (p, changeStart, endStr)]
 for line in p4_read_pipe_lines(cmd):
 changeNum = int(line.split( )[1])
 new_changes.append(changeNum)
 changes[changeNum] = True
-if len(new_changes) == block_size:
-get_another_block = True
-end = '@' + str(min(new_changes))
-else:
+
+if not block_size:
+# Not batched, so nothing more to do
 get_another_block = False
+elif end = mostRecentCommit:
+get_another_block = False
+else:
+changeStart = end + 1
 
 changelist = changes.keys()
 changelist.sort()
@@ -1974,7 +1996,7 @@ class P4Sync(Command, P4UserMap):
 self.syncWithOrigin = True
 self.importIntoRemotes = True
 self.maxChanges = 
-self.changes_block_size = 500
+self.changes_block_size = None
 self.keepRepoPath = False
 self.depotPaths = None
 self.p4BranchesInGit = []
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index aae1121..3b3ae1f 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -49,11 +49,11 @@ test_expect_success 'Default user cannot fetch changes' '
! p4 changes -m 1 //depot/...
 '
 
-test_expect_failure 'Clone the repo' '
+test_expect_success 'Clone the repo' '
git p4 clone --dest=$git --changes-block-size=7 --verbose 
//depot/included@all
 '
 
-test_expect_failure 'All files are present' '
+test_expect_success 'All files are present' '
echo file.txt expected 
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt 
expected 
test_write_lines outer5.txt expected 
@@ -61,18 +61,18 @@ test_expect_failure 'All files are present' '
test_cmp expected current
 '
 
-test_expect_failure 'file.txt

[PATCHv1 1/3] git-p4: additional testing of --changes-block-size

[Luke Diamand]

Add additional tests of some corner-cases of the
--changes-block-size git-p4 parameter.

Also reduce the number of p4 changes created during the
tests, so that they complete faster.

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9818-git-p4-block.sh | 56 +
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 153b20a..79765a4 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -8,18 +8,21 @@ test_expect_success 'start p4d' '
start_p4d
 '
 
-test_expect_success 'Create a repo with ~100 changes' '
+test_expect_success 'Create a repo with many changes' '
(
-   cd $cli 
+   client_view //depot/included/... //client/included/... \
+   //depot/excluded/... //client/excluded/... 
+   mkdir -p $cli/included $cli/excluded 
+   cd $cli/included 
file.txt 
p4 add file.txt 
p4 submit -d Add file.txt 
-   for i in $(test_seq 0 9)
+   for i in $(test_seq 0 5)
do
outer$i.txt 
p4 add outer$i.txt 
p4 submit -d Adding outer$i.txt 
-   for j in $(test_seq 0 9)
+   for j in $(test_seq 0 5)
do
p4 edit file.txt 
echo $i$j file.txt 
@@ -30,33 +33,68 @@ test_expect_success 'Create a repo with ~100 changes' '
 '
 
 test_expect_success 'Clone the repo' '
-   git p4 clone --dest=$git --changes-block-size=10 --verbose //depot@all
+   git p4 clone --dest=$git --changes-block-size=7 --verbose 
//depot/included@all
 '
 
 test_expect_success 'All files are present' '
echo file.txt expected 
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt 
expected 
-   test_write_lines outer5.txt outer6.txt outer7.txt outer8.txt outer9.txt 
expected 
+   test_write_lines outer5.txt expected 
ls $git current 
test_cmp expected current
 '
 
 test_expect_success 'file.txt is correct' '
-   echo 99 expected 
+   echo 55 expected 
test_cmp expected $git/file.txt
 '
 
 test_expect_success 'Correct number of commits' '
(cd $git  git log --oneline) log 
-   test_line_count = 111 log
+   wc -l log 
+   test_line_count = 43 log
 '
 
 test_expect_success 'Previous version of file.txt is correct' '
(cd $git  git checkout HEAD^^) 
-   echo 97 expected 
+   echo 53 expected 
test_cmp expected $git/file.txt
 '
 
+# Test git-p4 sync, with some files outside the client specification.
+
+p4_add_file() {
+   (cd $cli 
+   $1 
+   p4 add $1 
+   p4 submit -d Added a file $1
+   )
+}
+
+test_expect_success 'Add some more files' '
+   for i in $(test_seq 0 10)
+   do
+   p4_add_file included/x$i 
+   p4_add_file excluded/x$i
+   done 
+   for i in $(test_seq 0 10)
+   do
+   p4_add_file excluded/y$i
+   done
+'
+
+# This should pick up the 10 new files in included, but not be confused
+# by the additional files in excluded
+test_expect_success 'Syncing files' '
+   (
+   cd $git 
+   git p4 sync --changes-block-size=7 
+   git checkout p4/master 
+   ls -l x*  log 
+   test_line_count = 11 log
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 0/3] git-p4: fixing --changes-block-size support

[Luke Diamand]

We recently added support to git-p4 to limit the number of changes it
would try to import at a time. That was to help clients who were being
limited by the maxscanrows limit. This used the -m maxchanges
argument to p4 changes to limit the number of results returned to
git-p4.

Unfortunately it turns out that in practice, the server limits the
number of results returned *before* the -m maxchanges argument is
considered. Even supplying a -m 1 argument doesn't help.

This affects both the maxscanrows and maxresults group options.

This set of patches updates the t9818 git-p4 tests to show the problem,
and then adds a fix which works by iterating over the changes in batches
(as at present) but using a revision range to limit the number of changes,
rather than -m $BATCHSIZE.

That means it will in most cases require more transactions with the server,
but usually the effect will be small.

Along the way I also found that p4 print can fail if you have a file
with too many changes in it, but there's unfortunately no way to workaround
this. It's fairly unlikely to ever happen in practice.

I think I've covered everything in this fix, but it's possible that there
are still bugs to be uncovered; I find the way that these limits interact
somewhat tricky to understand.

Thanks,
Luke

Luke Diamand (3):
  git-p4: additional testing of --changes-block-size
  git-p4: test with limited p4 server results
  git-p4: fixing --changes-block-size handling

 git-p4.py   | 48 +++-
 t/t9818-git-p4-block.sh | 73 +++--
 2 files changed, 99 insertions(+), 22 deletions(-)

-- 
2.3.4.48.g223ab37

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 1/4] git-p4: additional testing of --changes-block-size

[Luke Diamand]

Add additional tests of some corner-cases of the
--changes-block-size git-p4 parameter.

Also reduce the number of p4 changes created during the
tests, so that they complete faster.

Signed-off-by: Luke Diamand l...@diamand.org
Acked-by: Lex Spoon l...@lexspoon.org
---
 t/t9818-git-p4-block.sh | 56 +
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 153b20a..79765a4 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -8,18 +8,21 @@ test_expect_success 'start p4d' '
start_p4d
 '
 
-test_expect_success 'Create a repo with ~100 changes' '
+test_expect_success 'Create a repo with many changes' '
(
-   cd $cli 
+   client_view //depot/included/... //client/included/... \
+   //depot/excluded/... //client/excluded/... 
+   mkdir -p $cli/included $cli/excluded 
+   cd $cli/included 
file.txt 
p4 add file.txt 
p4 submit -d Add file.txt 
-   for i in $(test_seq 0 9)
+   for i in $(test_seq 0 5)
do
outer$i.txt 
p4 add outer$i.txt 
p4 submit -d Adding outer$i.txt 
-   for j in $(test_seq 0 9)
+   for j in $(test_seq 0 5)
do
p4 edit file.txt 
echo $i$j file.txt 
@@ -30,33 +33,68 @@ test_expect_success 'Create a repo with ~100 changes' '
 '
 
 test_expect_success 'Clone the repo' '
-   git p4 clone --dest=$git --changes-block-size=10 --verbose //depot@all
+   git p4 clone --dest=$git --changes-block-size=7 --verbose 
//depot/included@all
 '
 
 test_expect_success 'All files are present' '
echo file.txt expected 
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt 
expected 
-   test_write_lines outer5.txt outer6.txt outer7.txt outer8.txt outer9.txt 
expected 
+   test_write_lines outer5.txt expected 
ls $git current 
test_cmp expected current
 '
 
 test_expect_success 'file.txt is correct' '
-   echo 99 expected 
+   echo 55 expected 
test_cmp expected $git/file.txt
 '
 
 test_expect_success 'Correct number of commits' '
(cd $git  git log --oneline) log 
-   test_line_count = 111 log
+   wc -l log 
+   test_line_count = 43 log
 '
 
 test_expect_success 'Previous version of file.txt is correct' '
(cd $git  git checkout HEAD^^) 
-   echo 97 expected 
+   echo 53 expected 
test_cmp expected $git/file.txt
 '
 
+# Test git-p4 sync, with some files outside the client specification.
+
+p4_add_file() {
+   (cd $cli 
+   $1 
+   p4 add $1 
+   p4 submit -d Added a file $1
+   )
+}
+
+test_expect_success 'Add some more files' '
+   for i in $(test_seq 0 10)
+   do
+   p4_add_file included/x$i 
+   p4_add_file excluded/x$i
+   done 
+   for i in $(test_seq 0 10)
+   do
+   p4_add_file excluded/y$i
+   done
+'
+
+# This should pick up the 10 new files in included, but not be confused
+# by the additional files in excluded
+test_expect_success 'Syncing files' '
+   (
+   cd $git 
+   git p4 sync --changes-block-size=7 
+   git checkout p4/master 
+   ls -l x*  log 
+   test_line_count = 11 log
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
2.4.1.502.gb11c5ab

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 0/4] git-p4: fixing --changes-block-size handling

[Luke Diamand]

This series of patches teaches git-p4 to break up calls to the
P4 server into smaller chunks, to avoid hitting the maxresults and
maxscanrows server-side limits.

The previous iteration of this series didn't handle non-integer
P4 revision ranges (e.g. //depot/...@2014/1/1,2015/1/1).

This version now breaks up the commit into blocks iff the revision
range specified is an integer range: @all, M,N or N,#head. If the
range is non-numeric then it falls back to relying on Perforce to
parse the revisions. In this mode it is no longer possible to fetch
changes in blocks (and requests to do so will be rejected).

Another alternative would be to turn the date (or whatever) revisions
into numeric revision numbers, but there doesn't seem to be a simple
way to do this. The best I can come up with is to get the changes
around the date in question (perhaps binary-chopping from years down
to seconds to find a range that works?) and then take the lowest
revision supplied. But that's quite a bit more complex.

Luke Diamand (4):
  git-p4: additional testing of --changes-block-size
  git-p4: test with limited p4 server results
  git-p4: add tests for non-numeric revision range
  git-p4: fixing --changes-block-size handling

 git-p4.py   | 85 -
 t/t9800-git-p4-basic.sh | 38 ++
 t/t9818-git-p4-block.sh | 73 --
 3 files changed, 165 insertions(+), 31 deletions(-)

-- 
2.4.1.502.gb11c5ab

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 3/4] git-p4: add tests for non-numeric revision range

[Luke Diamand]

Test that git-p4 can handle a sync with a non-numeric revision
range (e.g. a date).

Signed-off-by: Luke Diamand l...@diamand.org
---
 t/t9800-git-p4-basic.sh | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 5b56212..90d41ed 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -131,6 +131,44 @@ test_expect_success 'clone two dirs, @all, conflicting 
files' '
)
 '
 
+revision_ranges=2000/01/01,#head \
+1,2080/01/01 \
+2000/01/01,2080/01/01 \
+2000/01/01,1000 \
+1,1000
+
+test_expect_success 'clone using non-numeric revision ranges' '
+   test_when_finished cleanup_git 
+   for r in $revision_ranges
+   do
+   rm -fr $git 
+   test ! -d $git 
+   git p4 clone --dest=$git //depot@$r 
+   (
+   cd $git 
+   git ls-files lines 
+   test_line_count = 6 lines
+   )
+   done
+'
+
+test_expect_success 'clone with date range, excluding some changes' '
+   test_when_finished cleanup_git 
+   before=$(date +%Y/%m/%d:%H:%M:%S) 
+   sleep 2 
+   (
+   cd $cli 
+   :date_range_test 
+   p4 add date_range_test 
+   p4 submit -d Adding file
+   ) 
+   git p4 clone --dest=$git //depot@1,$before 
+   (
+   cd $git 
+   test_path_is_missing date_range_test
+   )
+'
+
 test_expect_success 'exit when p4 fails to produce marshaled output' '
mkdir badp4dir 
test_when_finished rm badp4dir/p4  rmdir badp4dir 
-- 
2.4.1.502.gb11c5ab

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 4/4] git-p4: fixing --changes-block-size handling

[Luke Diamand]

The --changes-block-size handling was intended to help when
a user has a limited maxscanrows (see p4 group). It used
p4 changes -m $maxchanges to limit the number of results.

Unfortunately, it turns out that the maxscanrows and maxresults
limits are actually applied *before* the -m maxchanges parameter
is considered (experimentally).

Fix the block-size handling so that it gets blocks of changes
limited by revision number ($Start..$Start+$N, etc). This limits
the number of results early enough that both sets of tests pass.

Note that many other Perforce operations can fail for the same
reason (p4 print, p4 files, etc) and it's probably not possible
to workaround this. In the real world, this is probably not
usually a problem.

Signed-off-by: Luke Diamand l...@diamand.org
---
 git-p4.py   | 85 -
 t/t9818-git-p4-block.sh | 12 +++
 2 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 26ad4bc..073f87b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -43,6 +43,9 @@ verbose = False
 # 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
+
 def p4_build_cmd(cmd):
 Build a suitable p4 command line.
 
@@ -249,6 +252,10 @@ def p4_reopen(type, f):
 def p4_move(src, dest):
 p4_system([move, -k, wildcard_encode(src), wildcard_encode(dest)])
 
+def p4_last_change():
+results = p4CmdList([changes, -m, 1])
+return int(results[0]['change'])
+
 def p4_describe(change):
 Make sure it returns a valid result by checking for
the presence of field time.  Return a dict of the
@@ -742,43 +749,77 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = 
refs/remotes/p4/, silent
 def originP4BranchesExist():
 return gitBranchExists(origin) or gitBranchExists(origin/p4) or 
gitBranchExists(origin/p4/master)
 
-def p4ChangesForPaths(depotPaths, changeRange, block_size):
+
+def p4ParseNumericChangeRange(parts):
+changeStart = int(parts[0][1:])
+if parts[1] == '#head':
+changeEnd = p4_last_change()
+else:
+changeEnd = int(parts[1])
+
+return (changeStart, changeEnd)
+
+def chooseBlockSize(blockSize):
+if blockSize:
+return blockSize
+else:
+return defaultBlockSize
+
+def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
 assert depotPaths
-assert block_size
 
-# Parse the change range into start and end
+# Parse the change range into start and end. Try to find integer
+# revision ranges as these can be broken up into blocks to avoid
+# hitting server-side limits (maxrows, maxscanresults). But if
+# that doesn't work, fall back to using the raw revision specifier
+# strings, without using block mode.
+
 if changeRange is None or changeRange == '':
-changeStart = '@1'
-changeEnd = '#head'
+changeStart = 1
+changeEnd = p4_last_change()
+block_size = chooseBlockSize(requestedBlockSize)
 else:
 parts = changeRange.split(',')
 assert len(parts) == 2
-changeStart = parts[0]
-changeEnd = parts[1]
+try:
+(changeStart, changeEnd) = p4ParseNumericChangeRange(parts)
+block_size = chooseBlockSize(requestedBlockSize)
+except:
+changeStart = parts[0][1:]
+changeEnd = parts[1]
+if requestedBlockSize:
+die(cannot use --changes-block-size with non-numeric 
revisions)
+block_size = None
 
 # Accumulate change numbers in a dictionary to avoid duplicates
 changes = {}
 
 for p in depotPaths:
 # Retrieve changes a block at a time, to prevent running
-# into a MaxScanRows error from the server.
-start = changeStart
-end = changeEnd
-get_another_block = True
-while get_another_block:
-new_changes = []
+# into a MaxResults/MaxScanRows error from the server.
+
+while True:
 cmd = ['changes']
-cmd += ['-m', str(block_size)]
-cmd += [%s...%s,%s % (p, start, end)]
+
+if block_size:
+end = min(changeEnd, changeStart + block_size)
+revisionRange = %d,%d % (changeStart, end)
+else:
+revisionRange = %s,%s % (changeStart, changeEnd)
+
+cmd += [%s...@%s % (p, revisionRange)]
+
 for line in p4_read_pipe_lines(cmd):
 changeNum = int(line.split( )[1])
-new_changes.append(changeNum)
 changes[changeNum] = True
-if len(new_changes) == block_size:
-get_another_block = True
-end = '@' + str(min(new_changes))
-else:
-get_another_block = False
+
+if not block_size

[PATCHv3 2/4] git-p4: test with limited p4 server results

[Luke Diamand]

Change the --changes-block-size git-p4 test to use an account with
limited maxresults and maxscanrows values.

These conditions are applied in the server *before* the -m maxchanges
parameter to p4 changes is applied, and so the strategy that git-p4
uses for limiting the number of changes does not work. As a result,
the tests all fail.

Note that maxscanrows is set quite high, as it appears to not only
limit results from p4 changes, but *also* limits results from
p4 print. Files that have more than maxscanrows changes seem
(experimentally) to be impossible to print. There's no good way to
work around this.

Signed-off-by: Luke Diamand l...@diamand.org
Acked-by: Lex Spoon l...@lexspoon.org
---
 t/t9818-git-p4-block.sh | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 79765a4..aae1121 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -8,6 +8,19 @@ test_expect_success 'start p4d' '
start_p4d
 '
 
+create_restricted_group() {
+   p4 group -i -EOF
+   Group: restricted
+   MaxResults: 7
+   MaxScanRows: 40
+   Users: author
+   EOF
+}
+
+test_expect_success 'Create group with limited maxrows' '
+   create_restricted_group
+'
+
 test_expect_success 'Create a repo with many changes' '
(
client_view //depot/included/... //client/included/... \
@@ -32,11 +45,15 @@ test_expect_success 'Create a repo with many changes' '
)
 '
 
-test_expect_success 'Clone the repo' '
+test_expect_success 'Default user cannot fetch changes' '
+   ! p4 changes -m 1 //depot/...
+'
+
+test_expect_failure 'Clone the repo' '
git p4 clone --dest=$git --changes-block-size=7 --verbose 
//depot/included@all
 '
 
-test_expect_success 'All files are present' '
+test_expect_failure 'All files are present' '
echo file.txt expected 
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt 
expected 
test_write_lines outer5.txt expected 
@@ -44,18 +61,18 @@ test_expect_success 'All files are present' '
test_cmp expected current
 '
 
-test_expect_success 'file.txt is correct' '
+test_expect_failure 'file.txt is correct' '
echo 55 expected 
test_cmp expected $git/file.txt
 '
 
-test_expect_success 'Correct number of commits' '
+test_expect_failure 'Correct number of commits' '
(cd $git  git log --oneline) log 
wc -l log 
test_line_count = 43 log
 '
 
-test_expect_success 'Previous version of file.txt is correct' '
+test_expect_failure 'Previous version of file.txt is correct' '
(cd $git  git checkout HEAD^^) 
echo 53 expected 
test_cmp expected $git/file.txt
@@ -85,7 +102,7 @@ test_expect_success 'Add some more files' '
 
 # This should pick up the 10 new files in included, but not be confused
 # by the additional files in excluded
-test_expect_success 'Syncing files' '
+test_expect_failure 'Syncing files' '
(
cd $git 
git p4 sync --changes-block-size=7 
-- 
2.4.1.502.gb11c5ab

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 2/3] git-p4: test with limited p4 server results

[Luke Diamand]

Change the --changes-block-size git-p4 test to use an account with
limited maxresults and maxscanrows values.

These conditions are applied in the server *before* the -m maxchanges
parameter to p4 changes is applied, and so the strategy that git-p4
uses for limiting the number of changes does not work. As a result,
the tests all fail.

Note that maxscanrows is set quite high, as it appears to not only
limit results from p4 changes, but *also* limits results from
p4 print. Files that have more than maxscanrows changes seem
(experimentally) to be impossible to print. There's no good way to
work around this.

Signed-off-by: Luke Diamand l...@diamand.org
Acked-by: Lex Spoon l...@lexspoon.org
---
 t/t9818-git-p4-block.sh | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 79765a4..aae1121 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -8,6 +8,19 @@ test_expect_success 'start p4d' '
start_p4d
 '
 
+create_restricted_group() {
+   p4 group -i -EOF
+   Group: restricted
+   MaxResults: 7
+   MaxScanRows: 40
+   Users: author
+   EOF
+}
+
+test_expect_success 'Create group with limited maxrows' '
+   create_restricted_group
+'
+
 test_expect_success 'Create a repo with many changes' '
(
client_view //depot/included/... //client/included/... \
@@ -32,11 +45,15 @@ test_expect_success 'Create a repo with many changes' '
)
 '
 
-test_expect_success 'Clone the repo' '
+test_expect_success 'Default user cannot fetch changes' '
+   ! p4 changes -m 1 //depot/...
+'
+
+test_expect_failure 'Clone the repo' '
git p4 clone --dest=$git --changes-block-size=7 --verbose 
//depot/included@all
 '
 
-test_expect_success 'All files are present' '
+test_expect_failure 'All files are present' '
echo file.txt expected 
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt 
expected 
test_write_lines outer5.txt expected 
@@ -44,18 +61,18 @@ test_expect_success 'All files are present' '
test_cmp expected current
 '
 
-test_expect_success 'file.txt is correct' '
+test_expect_failure 'file.txt is correct' '
echo 55 expected 
test_cmp expected $git/file.txt
 '
 
-test_expect_success 'Correct number of commits' '
+test_expect_failure 'Correct number of commits' '
(cd $git  git log --oneline) log 
wc -l log 
test_line_count = 43 log
 '
 
-test_expect_success 'Previous version of file.txt is correct' '
+test_expect_failure 'Previous version of file.txt is correct' '
(cd $git  git checkout HEAD^^) 
echo 53 expected 
test_cmp expected $git/file.txt
@@ -85,7 +102,7 @@ test_expect_success 'Add some more files' '
 
 # This should pick up the 10 new files in included, but not be confused
 # by the additional files in excluded
-test_expect_success 'Syncing files' '
+test_expect_failure 'Syncing files' '
(
cd $git 
git p4 sync --changes-block-size=7 
-- 
2.4.1.502.gb11c5ab

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/3] git-p4: additional testing of --changes-block-size

[Luke Diamand]

Add additional tests of some corner-cases of the
--changes-block-size git-p4 parameter.

Also reduce the number of p4 changes created during the
tests, so that they complete faster.

Signed-off-by: Luke Diamand l...@diamand.org
Acked-by: Lex Spoon l...@lexspoon.org
---
 t/t9818-git-p4-block.sh | 56 +
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 153b20a..79765a4 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -8,18 +8,21 @@ test_expect_success 'start p4d' '
start_p4d
 '
 
-test_expect_success 'Create a repo with ~100 changes' '
+test_expect_success 'Create a repo with many changes' '
(
-   cd $cli 
+   client_view //depot/included/... //client/included/... \
+   //depot/excluded/... //client/excluded/... 
+   mkdir -p $cli/included $cli/excluded 
+   cd $cli/included 
file.txt 
p4 add file.txt 
p4 submit -d Add file.txt 
-   for i in $(test_seq 0 9)
+   for i in $(test_seq 0 5)
do
outer$i.txt 
p4 add outer$i.txt 
p4 submit -d Adding outer$i.txt 
-   for j in $(test_seq 0 9)
+   for j in $(test_seq 0 5)
do
p4 edit file.txt 
echo $i$j file.txt 
@@ -30,33 +33,68 @@ test_expect_success 'Create a repo with ~100 changes' '
 '
 
 test_expect_success 'Clone the repo' '
-   git p4 clone --dest=$git --changes-block-size=10 --verbose //depot@all
+   git p4 clone --dest=$git --changes-block-size=7 --verbose 
//depot/included@all
 '
 
 test_expect_success 'All files are present' '
echo file.txt expected 
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt 
expected 
-   test_write_lines outer5.txt outer6.txt outer7.txt outer8.txt outer9.txt 
expected 
+   test_write_lines outer5.txt expected 
ls $git current 
test_cmp expected current
 '
 
 test_expect_success 'file.txt is correct' '
-   echo 99 expected 
+   echo 55 expected 
test_cmp expected $git/file.txt
 '
 
 test_expect_success 'Correct number of commits' '
(cd $git  git log --oneline) log 
-   test_line_count = 111 log
+   wc -l log 
+   test_line_count = 43 log
 '
 
 test_expect_success 'Previous version of file.txt is correct' '
(cd $git  git checkout HEAD^^) 
-   echo 97 expected 
+   echo 53 expected 
test_cmp expected $git/file.txt
 '
 
+# Test git-p4 sync, with some files outside the client specification.
+
+p4_add_file() {
+   (cd $cli 
+   $1 
+   p4 add $1 
+   p4 submit -d Added a file $1
+   )
+}
+
+test_expect_success 'Add some more files' '
+   for i in $(test_seq 0 10)
+   do
+   p4_add_file included/x$i 
+   p4_add_file excluded/x$i
+   done 
+   for i in $(test_seq 0 10)
+   do
+   p4_add_file excluded/y$i
+   done
+'
+
+# This should pick up the 10 new files in included, but not be confused
+# by the additional files in excluded
+test_expect_success 'Syncing files' '
+   (
+   cd $git 
+   git p4 sync --changes-block-size=7 
+   git checkout p4/master 
+   ls -l x*  log 
+   test_line_count = 11 log
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
2.4.1.502.gb11c5ab

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 0/3] git-p4: fixing --changes-block-size handling

[Luke Diamand]

Updated per Lex's suggestion, so that git-p4 always uses the block mode,
and takes advantage of this to simplify the loop. This exposed a bug
in the termination condition.

One thing to note: 'git p4 sync' claims to support arbitrary p4 revision
specifications. I need to check that this is tested and hasn't been broken
by these changes.

Luke

Luke Diamand (3):
  git-p4: additional testing of --changes-block-size
  git-p4: test with limited p4 server results
  git-p4: fixing --changes-block-size handling

 git-p4.py   | 45 ++
 t/t9818-git-p4-block.sh | 73 +++--
 2 files changed, 92 insertions(+), 26 deletions(-)

-- 
2.4.1.502.gb11c5ab

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 3/3] git-p4: fixing --changes-block-size handling

[Luke Diamand]

The --changes-block-size handling was intended to help when
a user has a limited maxscanrows (see p4 group). It used
p4 changes -m $maxchanges to limit the number of results.

Unfortunately, it turns out that the maxscanrows and maxresults
limits are actually applied *before* the -m maxchanges parameter
is considered (experimentally).

Fix the block-size handling so that it gets blocks of changes
limited by revision number ($Start..$Start+$N, etc). This limits
the number of results early enough that both sets of tests pass.

Note that many other Perforce operations can fail for the same
reason (p4 print, p4 files, etc) and it's probably not possible
to workaround this. In the real world, this is probably not
usually a problem.

Signed-off-by: Luke Diamand l...@diamand.org
---
 git-p4.py   | 45 -
 t/t9818-git-p4-block.sh | 12 ++--
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 26ad4bc..4be0037 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -249,6 +249,10 @@ def p4_reopen(type, f):
 def p4_move(src, dest):
 p4_system([move, -k, wildcard_encode(src), wildcard_encode(dest)])
 
+def p4_last_change():
+results = p4CmdList([changes, -m, 1])
+return int(results[0]['change'])
+
 def p4_describe(change):
 Make sure it returns a valid result by checking for
the presence of field time.  Return a dict of the
@@ -746,39 +750,46 @@ def p4ChangesForPaths(depotPaths, changeRange, 
block_size):
 assert depotPaths
 assert block_size
 
+# We need the most recent change list number since we can't just
+# use #head in block mode.
+lastChange = p4_last_change()
+
 # Parse the change range into start and end
 if changeRange is None or changeRange == '':
-changeStart = '@1'
-changeEnd = '#head'
+changeStart = 1
+changeEnd = lastChange
 else:
 parts = changeRange.split(',')
 assert len(parts) == 2
-changeStart = parts[0]
-changeEnd = parts[1]
+changeStart = int(parts[0][1:])
+if parts[1] == '#head':
+changeEnd = lastChange
+else:
+changeEnd = int(parts[1])
 
 # Accumulate change numbers in a dictionary to avoid duplicates
 changes = {}
 
 for p in depotPaths:
 # Retrieve changes a block at a time, to prevent running
-# into a MaxScanRows error from the server.
-start = changeStart
-end = changeEnd
-get_another_block = True
-while get_another_block:
-new_changes = []
+# into a MaxResults/MaxScanRows error from the server.
+
+while True:
+end = min(changeEnd, changeStart + block_size)
+
 cmd = ['changes']
-cmd += ['-m', str(block_size)]
-cmd += [%s...%s,%s % (p, start, end)]
+cmd += [%s...@%d,%d % (p, changeStart, end)]
+
+new_changes = []
 for line in p4_read_pipe_lines(cmd):
 changeNum = int(line.split( )[1])
 new_changes.append(changeNum)
 changes[changeNum] = True
-if len(new_changes) == block_size:
-get_another_block = True
-end = '@' + str(min(new_changes))
-else:
-get_another_block = False
+
+if end = changeEnd:
+break
+
+changeStart = end + 1
 
 changelist = changes.keys()
 changelist.sort()
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index aae1121..3b3ae1f 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -49,11 +49,11 @@ test_expect_success 'Default user cannot fetch changes' '
! p4 changes -m 1 //depot/...
 '
 
-test_expect_failure 'Clone the repo' '
+test_expect_success 'Clone the repo' '
git p4 clone --dest=$git --changes-block-size=7 --verbose 
//depot/included@all
 '
 
-test_expect_failure 'All files are present' '
+test_expect_success 'All files are present' '
echo file.txt expected 
test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt 
expected 
test_write_lines outer5.txt expected 
@@ -61,18 +61,18 @@ test_expect_failure 'All files are present' '
test_cmp expected current
 '
 
-test_expect_failure 'file.txt is correct' '
+test_expect_success 'file.txt is correct' '
echo 55 expected 
test_cmp expected $git/file.txt
 '
 
-test_expect_failure 'Correct number of commits' '
+test_expect_success 'Correct number of commits' '
(cd $git  git log --oneline) log 
wc -l log 
test_line_count = 43 log
 '
 
-test_expect_failure 'Previous version of file.txt is correct' '
+test_expect_success 'Previous version of file.txt is correct' '
(cd $git  git checkout HEAD^^) 
echo 53 expected 
test_cmp expected $git/file.txt
@@ -102,7 +102,7 @@ test_expect_success 'Add

Re: Dependency Management

[Luke Diamand]

On 23/06/15 19:49, Josh Hagins wrote:

If neither git-submodule nor git-subtree is palatable to you, here are
a couple of alternatives you might try:

   * https://github.com/ingydotnet/git-subrepo
   * https://github.com/tdd/git-stree


You could also use Android's repo tool:

https://code.google.com/p/git-repo/

Luke



On Tue, Jun 23, 2015 at 1:36 PM Stefan Beller sbel...@google.com wrote:


On Tue, Jun 23, 2015 at 1:52 AM, Jean Audibert jaudib...@euronext.com wrote:

Hi,

Sorry to bother you with this question but I can't find any official answer or 
strong opinion from Git community.

In my company we recently started to use Git and we wonder how to share code 
and manage dependencies with Git?
Use case: in project P we need to include lib-a and lib-b (libraries shared by 
several projects)

In your opinion, what is the future proof solution?
* Use submodule
* Use subtree

We know there is lot of PRO/CONS but I feel that subtree is behind in the 
race and the latest version of submodule work fine


Use whatever works fine for your use case.

My personal opinion/expectation is to see submodules
improving/advancing more than subtrees advancing in the near future.
Though this is neither the official nor a strong opinion.

Stefan



Suggestions are very welcome.
Thanks in advance,

Jean Audibert



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: BUG: checkout won't checkout?

[Luke Diamand]

On 18 June 2015 at 23:28, Junio C Hamano gits...@pobox.com wrote:
 Luke Diamand l...@diamand.org writes:

 This is probably user error, but I'm not sure what I'm doing wrong.
 I'm posting here in case anyone else gets the same thing

 I'm using 2.4.4.598.gd7bed1d, i.e. 'next' as of today.

 I've somehow ended up with history skipping back in time, but git not
 prepared to let let me fix it, or something.

 $ git diff upstream/master -- subtree
  - lots of deltas, which look like I've reverted this back several
 revisions (which I haven't AFAIK)

 Are you on the right branch that you think you are working on?

Yes.


 $ git checkout upstream/master -- subtree
 $ git diff upstream/master -- subtree
 -- still lots of deltas

 Does this show _ONLY_ additions?  Or does it include modifications
 and removals?

There are indeed _ONLY_ additions.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: BUG: checkout won't checkout?

[Luke Diamand]

On 18 June 2015 at 23:53, Junio C Hamano gits...@pobox.com wrote:
 Luke Diamand l...@diamand.org writes:

 $ git checkout upstream/master -- subtree
 $ git diff upstream/master -- subtree
 -- still lots of deltas

 Does this show _ONLY_ additions?  Or does it include modifications
 and removals?

 There are indeed _ONLY_ additions.

 http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234912
 http://thread.gmane.org/gmane.comp.version-control.git/234903/focus=234924

 In short, it is an intended behaviour, both Peff and I consider that
 the intention is bad and the behaviour should be changed.

OK, thanks, it makes sense now!


 But nothing has happened yet (it is listed as one of the leftover
 bits http://git-blame.blogspot.com/p/leftover-bits.html).

OK, I'll keep it in mind.

Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

BUG: checkout won't checkout?

[Luke Diamand]

This is probably user error, but I'm not sure what I'm doing wrong.
I'm posting here in case anyone else gets the same thing

I'm using 2.4.4.598.gd7bed1d, i.e. 'next' as of today.

I've somehow ended up with history skipping back in time, but git not
prepared to let let me fix it, or something.

$ git diff upstream/master -- subtree
 - lots of deltas, which look like I've reverted this back several
revisions (which I haven't AFAIK)
$ git checkout upstream/master -- subtree
$ git diff upstream/master -- subtree
-- still lots of deltas
$ git checkout upstream/master -- subtree
$ git commit -m 'Revert unwanted changes'
$ git diff upstream/master -- subtree
-- still lots of deltas

What am I doing wrong? Have I ended up in the middle of some weird
state (cherry-pick and rebase isn't in progress AFAIK).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: BUG: checkout won't checkout?

[Luke Diamand]

The other thing about these files is that they were all deleted a few
weeks ago and have now come back.

On 18 June 2015 at 23:07, Luke Diamand l...@diamand.org wrote:
 This is probably user error, but I'm not sure what I'm doing wrong.
 I'm posting here in case anyone else gets the same thing

 I'm using 2.4.4.598.gd7bed1d, i.e. 'next' as of today.

 I've somehow ended up with history skipping back in time, but git not
 prepared to let let me fix it, or something.

 $ git diff upstream/master -- subtree
  - lots of deltas, which look like I've reverted this back several
 revisions (which I haven't AFAIK)
 $ git checkout upstream/master -- subtree
 $ git diff upstream/master -- subtree
 -- still lots of deltas
 $ git checkout upstream/master -- subtree
 $ git commit -m 'Revert unwanted changes'
 $ git diff upstream/master -- subtree
 -- still lots of deltas

 What am I doing wrong? Have I ended up in the middle of some weird
 state (cherry-pick and rebase isn't in progress AFAIK).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: format-patch and submodules

[Luke Diamand]

On 10/06/15 18:04, Christopher Dunn wrote:

Sorry. I thought empty patches were made to work in other cases.

'git-p4' needs to skip these. Wrong mailing list then.


Possibly the right mailing list - can you explain what you mean here 
w.r.t git-p4 please?


Thanks!
Luke






On Tue, Jun 9, 2015 at 1:52 PM, Jens Lehmann jens.lehm...@web.de wrote:

Am 05.06.2015 um 01:20 schrieb Christopher Dunn:


(Seen in git versions: 2.1.0 and 1.9.3 et al.)

$ git format-patch --stdout X^..X | git apply check -
fatal: unrecognized input

This fails when the commit consists of nothing but a submodule change
(as in 'git add submodule foo'), but it passes when a file change is
added to the same commit.

There used to be a similar problem for empty commits, but that was
fixed around git-1.8:


http://stackoverflow.com/questions/20775132/cannot-apply-git-patch-replacing-a-file-with-a-link

Now, 'git format-patch' outputs nothing for an empty commit. I suppose
that needs to be the behavior also when only submodules are changed,
since in that case there is no 'diff' section from 'format-patch'.

Use-case: git-p4

Of course, we do not plan to add the submodule into Perforce, but we
would like this particular command to behave the same whether there
are other diffs or not.



Hmm, I'm not sure that this is a bug. It looks to me like doing a

$ git format-patch --stdout X^..X | git apply check -

when nothing is changed except submodules and expecting it to work
is the cause of the problem.

I get the same error when I do:

$git format-patch --stdout master..master | git apply --check -
fatal: unrecognized input

No submodules involved, just an empty patch.

I assume you want to ignore all submodule changes, so you should
check if e.g. git diff --ignore-submodules X^..X returns anything
before applying that? (From the command you ran I assume you might
be able to drop the --ignore-submodules because you already did set
diff.ignoreSubmodules to all?)

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] p4: Retrieve the right revision of the UTF-16 file

[Luke Diamand]

On 27/05/15 23:31, Miguel Torroja wrote:

Fixing bug with UTF-16 files when they are retreived by git-p4.
It was always getting the tip version of the file and the history of the
file was lost.


This looks sensible to me, and seems to work in some simple testing, thanks!

Ack.

Luke



---
  git-p4.py |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index cdfa2df..be2c7da 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap):
  # them back too.  This is not needed to the cygwin windows 
version,
  # just the native NT type.
  #
-text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
+text = p4_read_pipe(['print', '-q', '-o', '-', %s@%s % 
(file['depotFile'], file['change']) ])
  if p4_version_string().find(/NT) = 0:
  text = text.replace(\r\n, \n)
  contents = [ text ]



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [git-p4] import with labels fails when commit is not transferred

[Luke Diamand]

Sorry for not replying earlier, and thanks for taking the time to 
investigate this!


It's a pretty subtle corner case: I think a test case would be useful. 
I'm going to try to put something together, unless you beat me to it!


(I think t9811-git-p4-label-import.sh is the one that needs extending).

Thanks!
Luke

On 30/06/15 09:45, Holl, Marcus wrote:

Hi,

I have an issue with the git p4 tooling regarding import of labels.

My git version is 2.4.5

I try to transform a perforce repository. My command line is:
git p4 clone --verbose --detect-branches --import-local --import-labels --destination 
DESTINATION //depot@all


The relevant parts in the gitconfig is:
[git-p4]
 branchUser = USERNAME


For that user there is a branch mapping defined with a lot of entries like:
//depot/trunk/... //depot/branches/ipro-status-8-2--branch/...
//depot/trunk/... //depot/branches/9-0-preview/...
//depot/trunk/... //depot/branches/release-8-0-0-branch/...
//depot/trunk/... //depot/branches/release-8-1-0-branch/...
//depot/trunk/... //depot/branches/release-8-2-0-branch/...
//depot/trunk/... //depot/branches/release-8-3-0-branch/...
//depot/trunk/... //depot/branches/release-8-4-branch/...
//depot/trunk/... //depot/branches/release-8-5-branch/...
...


The import fails with the log output that can be found at the bottom of this 
mail.

git log -all -grep \[git-p4:.*change\ =\ 69035\] reports nothing. The commit 
is not contained in the git repository.

p4 describe for changelist 69035 returns a reasonable result. This change 
contains one file located at a path in the perforce folder structure that comes 
without corresponding entry in the perforce branch mapping.

According to the given branch mapping it looks reasonable to me that the change 
is omitted in the git repository. But in my opinion the import should not fail 
in such a case.

A reasonable behavior would be to blacklist the label (add it to 
git-p4.ignoredP4Labels) and to continue with the next label.

Attached is a proposal for a fix that needs to be carefully reviews since I'm 
not that experienced with python.

Other proposals for resolving this issue are highly appreciated.

Thanks a lot and best regards,
Marcus Holl


Log output:

Reading pipe: ['git', 'rev-list', '--max-count=1', '--reverse', 
':/\\[git-p4:.*change = 69035\\]']
fatal: ambiguous argument ':/\[git-p4:.*change = 69035\]': unknown revision or 
path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git command [revision...] -- [file...]'
ied with change: 69078, original Date: 2010-04-22T09:07:24.00Z\n', 
'Update': '2013/11/02 07:40:31', 'Label': 'release-8-1-0-976', 'Access': 
'2015/06/26 14:50:15', 'Owner': 'svn_p4_converter', 'Options': 'unlocked 
noautoreload'}
p4 label release-8-1-0-976 mapped to git commit 
82a11809928b86a7bde03cf486428de52ab3380f
writing tag release-9-0-0-179 for commit 
fb8370cd04806686c567ad720d065436f2334b4a
labelDetails= {'code': 'stat', 'Description': 'Created or modified with change: 
96984, original Date: 2011-12-22T16:01:25.681427Z\n', 'Update': '2013/11/02 
15:15:50', 'Label': 'release-9-0-0-179', 'Access': '2015/06/26 14:50:16', 
'Owner': 'build', 'Options': 'unlocked noautoreload'}
p4 label release-9-0-0-179 mapped to git commit 
fb8370cd04806686c567ad720d065436f2334b4a
Traceback (most recent call last):
   File /usr/lib/git/git-p4, line 3297, in module
 main()
   File /usr/lib/git/git-p4, line 3291, in main
 if not cmd.run(args):
   File /usr/lib/git/git-p4, line 3165, in run
 if not P4Sync.run(self, depotPaths):
   File /usr/lib/git/git-p4, line 3045, in run
 self.importP4Labels(self.gitStream, missingP4Labels)
   File /usr/lib/git/git-p4, line 2421, in importP4Labels
 --reverse, :/\[git-p4:.*change = %d\] % changelist])
   File /usr/lib/git/git-p4, line 138, in read_pipe
 die('Command failed: %s' % str(c))
   File /usr/lib/git/git-p4, line 106, in die
 raise Exception(msg)
Exception: Command failed: ['git', 'rev-list', '--max-count=1', '--reverse', 
':/\\[git-p4:.*change = 69035\\]']



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] git-p4: fix handling of multi-word P4EDITOR

[Luke Diamand]

On 07/05/15 23:16, Junio C Hamano wrote:

Luke Diamand l...@diamand.org writes:



[Resurrecting old thread]



Looking at run-command.c, GIT_WINDOES_NATIVE and POSIX seems to use
pretty much the same construct, except that they use SHELL_PATH
instead of sh.


I think the state of git on Windows is a bit shaky (I'm happy to be 
proved wrong of course), but I think the only seriously active port is 
the msys one.


That, as far as I can tell, uses an msys version of 'sh', so it will be 
perfectly happy with the sh -c ... construct.


There may be a native windows port in existence, but I can't find how to 
build this, and I assume it's going to need Visual Studio, which makes 
it a lot more complex to get going.


The code you were looking at in run-command.c says this:

#ifndef GIT_WINDOWS_NATIVE
nargv[nargc++] = SHELL_PATH;   !GIT_WINDOWS_NATIVE
#else
nargv[nargc++] = sh; GIT_WINDOWS_NATIVE
#endif
nargv[nargc++] = -c;

To me, that seems to imply that for GIT_WINDOWS_NATIVE, we take the 
*second* branch and use sh, so again, the the code as it stands will 
be fine. msysgit uses that path.


(The next line, trying to use -c has no chance of working if Cmd is 
being used).





So something like this may be sufficient, perhaps?

  Makefile  | 1 +
  git-p4.py | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 20058f1..fda44bf 100644
--- a/Makefile
+++ b/Makefile
@@ -1776,6 +1776,7 @@ $(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX 
GIT-PYTHON-VARS
  $(SCRIPT_PYTHON_GEN): % : %.py
$(QUIET_GEN)$(RM) $@ $@+  \
sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
+   -e 's|SHELL_PATH|$(SHELL_PATH_SQ)|g' \
$ $@+  \
chmod +x $@+  \
mv $@+ $@
diff --git a/git-p4.py b/git-p4.py
index de06046..eb6d4b1 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1220,7 +1220,7 @@ class P4Submit(Command, P4UserMap):
  editor = os.environ.get(P4EDITOR)
  else:
  editor = read_pipe(git var GIT_EDITOR).strip()
-system([sh, -c, ('%s $@' % editor), editor, template_file])
+system(['''SHELL_PATH''', -c, ('%s $@' % editor), editor, 
template_file])


This seems to be expanded to '''sh''' which doesn't then work at all. I 
didn't take the time to investigate further though.




  # If the file was not saved, prompt to see if this patch should
  # be skipped.  But skip this verification step if configured so.


I don't think we need to do anything. msysgit works fine with the origin 
sh, -c, ... code.


Thanks!
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (May 2015, #06; Fri, 22)

[Luke Diamand]

On 22/05/15 23:14, 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'.


ry_matches(): inline function

  + is_ The fourth batch of topics have been merged to 'master'.



snip




* ld/p4-editor-multi-words (2015-05-20) 6 commits
  - SQUASH
  - git-p4: tests: use test-chmtime in place of touch
  - SQUASH
  - git-p4: fix handling of multi-word P4EDITOR
  - SQUASH
  - git-p4: add failing test for P4EDITOR handling

  Unlike $EDITOR and $GIT_EDITOR that can hold the path to the
  command and initial options (e.g. /path/to/emacs -nw), 'git p4'
  did not let the shell interpolate the contents of the environment
  variable that name the editor $P4EDITOR (and $EDITOR, too).
  Make it in line with the rest of Git, as well as with Perforce.


The latest versions in the branch (with the SQUASH) all look good to me.

The other thing still missing from this series is fixing Windows builds. 
I've been attempting to get a Windows build environment going to 
actually test it (if it hasn't been tested, it doesn't work :-).


I'm slowly getting there, but I'm not very familiar with this particular OS.

Thanks,
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git p4 clone - exclude file types

[Luke Diamand]

On 21/05/15 21:49, FusionX86 wrote:

I thought about that, but no. The box I'm running git-p4 on has the
following specs:

CentOS 6.6 64bit
1 CPU
8GB RAM
8GB Swap


Can you post the output, with -v added?

$ git-p4 clone //depot/some/dir -v

Also, what is your p4d server version?

$ p4 info

A quick test just cloning a repo with 4 files of 256MB each seems fine, 
FWIW.




It is also on the same physical network as the Perforce server. I
remember seeing someone else complain about this, but I can't find the
article/blog now.


On Wed, May 20, 2015 at 12:49 AM, Luke Diamand l...@diamand.org wrote:

On 19/05/15 08:38, FusionX86 wrote:


Thanks Luke, looks like this does work for excluding files when using
git p4. Great!

Unrelated question...

While using git p4 I have noticed that most of the time the clone/sync
operations hang and I have to keep retrying. The Perforce depot I'm
currently working with is larger than I'd like and has a lot of binary
files which might be the cause. The point it gets to in the clone/sync
is always random and doesn't ever stop on the same files or file
types. Sometimes it'll die soon after starting, but other times it
almost completes and then dies. If I keep retrying, it will eventually
complete. I haven't been able to narrow down the cause, but I do
notice that the git-fast-import stops right as the clone/sync dies.
I'm wondering if git is overwhelmed and terminates. Have you ever seen
this? Any suggestions?



Running out of memory?

Is this on a 32bit or 64bit system? How much virtual memory do you have?

Luke



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems

[Luke Diamand]

Lars - thanks for persisting with this!

I'm still trying to fully understand what's going on here - can you
point out where I've got it wrong below please!

The server is on Linux, and is case-sensitive. For whatever reason
(probably people committing changes on Windows in the first place)
we've ended up with a P4 repo that has differences in path case that
need to be ignored, with all upper-case paths being mapped to
lower-case.

e.g. the *real* depot might be:

//depot/path/File1
//depot/PATH/File2

git-p4 clone should return this case-folded on Windows (and Linux?)

$GIT_DIR/path/File1
$GIT_DIR/path/File2

(Am I right in thinking that this change folds the directory, but not
the filename?)

I don't really understand why a dictionary is required to do this
though - is there a reason we can't just lowercase all incoming paths?
Which is what the existing code in p4StartWith() is trying to do. That
code lowercases the *entire* path including the filename; this change
seems to leave the filename portion unchanged. I guess though that the
answer may be to do with your finding that P4 makes up the case of
directories based on lexicographic ordering - is that the basic
problem?

For a large repo, this approach is surely going to be really slow.

Additionally, could you update your patch to add some words to
Documentation/git-p4.txt please?

A few other comments inline.

Thanks!
Luke



On 21 August 2015 at 18:19,  larsxschnei...@gmail.com wrote:
 From: Lars Schneider larsxschnei...@gmail.com

 PROBLEM:
 We run P4 servers on Linux and P4 clients on Windows. For an unknown
 reason the file path for a number of files in P4 does not match the
 directory path with respect to case sensitivity.

 E.g. `p4 files` might return
 //depot/path/to/file1
 //depot/PATH/to/file2

 If you use P4/P4V then these files end up in the same directory, e.g.
 //depot/path/to/file1
 //depot/path/to/file2

 If you use git-p4 then all files not matching the correct file path
 (e.g. `file2`) will be ignored.

I'd like to think that the existing code that checks core.ignorecase
should handle this. I haven't tried it myself though.



 SOLUTION:
 Identify path names that are different with respect to case sensitivity.
 If there are any then run `p4 dirs` to build up a dictionary
 containing the correct cases for each path. It looks like P4
 interprets correct here as the existing path of the first file in a
 directory. The path dictionary is used later on to fix all paths.

 This is only applied if the parameter --ignore-path-case is passed to
 the git-p4 clone command.

 Signed-off-by: Lars Schneider larsxschnei...@gmail.com
 ---
  git-p4.py |  82 ++--
  t/t9821-git-p4-path-variations.sh | 109 
 ++
  2 files changed, 187 insertions(+), 4 deletions(-)
  create mode 100755 t/t9821-git-p4-path-variations.sh

 diff --git a/git-p4.py b/git-p4.py
 index 073f87b..61a587b 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -1931,7 +1931,7 @@ class View(object):
  (self.client_prefix, clientFile))
  return clientFile[len(self.client_prefix):]

 -def update_client_spec_path_cache(self, files):
 +def update_client_spec_path_cache(self, files, fixPathCase = None):
   Caching file paths by p4 where batch query 

  # List depot file paths exclude that already cached
 @@ -1950,6 +1950,8 @@ class View(object):
  if unmap in res:
  # it will list all of them, but only one not unmap-ped
  continue
 +if fixPathCase:
 +res['depotFile'] = fixPathCase(res['depotFile'])
  self.client_spec_path_cache[res['depotFile']] = 
 self.convert_client_path(res[clientFile])

  # not found files or unmap files set to 
 @@ -1987,6 +1989,7 @@ class P4Sync(Command, P4UserMap):
   help=Maximum number of changes to 
 import),
  optparse.make_option(--changes-block-size, 
 dest=changes_block_size, type=int,
   help=Internal block size to use when 
 iteratively calling p4 changes),
 +optparse.make_option(--ignore-path-case, 
 dest=ignorePathCase, action=store_true),
  optparse.make_option(--keep-path, dest=keepRepoPath, 
 action='store_true',
   help=Keep entire BRANCH/DIR/SUBDIR 
 prefix during import),
  optparse.make_option(--use-client-spec, 
 dest=useClientSpec, action='store_true',
 @@ -2017,6 +2020,7 @@ class P4Sync(Command, P4UserMap):
  self.maxChanges = 
  self.changes_block_size = None
  self.keepRepoPath = False
 +self.ignorePathCase = False
  self.depotPaths = None
  self.p4BranchesInGit = []
  self.cloneExclude = []
 @@ -2049,7 +2053,8 @@ class P4Sync(Command, P4UserMap):
  files = []
  fnum = 0
  while 

Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems

[Luke Diamand]

On 24 August 2015 at 13:43, Lars Schneider larsxschnei...@gmail.com wrote:

 https://github.com/luked99/quick-git-p4-case-folding-test
 As mentioned I realized that the problem occurs only if you use client specs. 
 Can you take a look at this test case / run it?
 https://github.com/larsxschneider/git/blob/d3a191cb5fb4d8f5f48ca9314c772169d5dbf65b/t/t9821-git-p4-path-variations.sh#L112-L127

 Does this make sense to you? If you want to I could also modify 
 “quick-git-p4-case-folding-test” to show the problem.

If you're able to fix my hacky test to show the problem that would be very kind.

If the problem only shows up when using client specs, is it possible
that the core.ignorecase logic is just missing a code path somewhere?

Glancing through the code, stripRepoPath() could perhaps be the
culprit? If self.useClientSpec is FALSE, it will do the
core.ignorecase trick by calling p4PathStartsWith, but if it is TRUE,
it won't.

Thanks!
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems

[Luke Diamand]

On 24 August 2015 at 10:51, Lars Schneider larsxschnei...@gmail.com wrote:

 I'm still trying to fully understand what's going on here - can you
 point out where I've got it wrong below please!
 Your welcome + sure :)


snip


 While I was working on the examples for this email reply I realized that the 
 problem is only present for paths given in a client-spec. I added a test case 
 to prove that. That means I don’t need to request *all* paths. I *think* it 
 is sufficient to request the paths mentioned in the client spec which would 
 usually be really fast. Stay tuned for PATCH v5.

I've just tried a small experiment with stock unaltered git-p4.

- Started p4d with -C1 option to case-fold.
- Add some files with different cases of directory (Path/File1,
PATH/File2, pATH/File3).
- git-p4 clone.

The result of the clone is separate directories if I do nothing
special (PATH/File1, Path/File2, etc). But if I set core.ignorecase, I
get a single case-folded directory, Path/File1, Path/File2, etc. I'm
still failing to get how that isn't what you need (other than being a
bit obscure to get to the right invocation).

I've put a script that shows this here:

https://github.com/luked99/quick-git-p4-case-folding-test

Thanks!
Luke
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [git-p4] import with labels fails when commit is not transferred

[Luke Diamand]

On 04/07/15 04:27, Luke Diamand wrote:

Sorry for not replying earlier, and thanks for taking the time to
investigate this!

It's a pretty subtle corner case: I think a test case would be useful.
I'm going to try to put something together, unless you beat me to it!

(I think t9811-git-p4-label-import.sh is the one that needs extending).


I've been looking into this a bit more, and I think that you don't need 
a try/except - it's enough just to add ignore_error=True to the call 
to read_lines.


It also looks like this is a general problem of importing a label where 
the revision doesn't exist in git. For example, if you create a p4 
label, then clone at the revision after that, you will get the problem.


I've got a test case and a modified git-p4.py, but I haven't yet got to 
the point where I'm convinced it works. I think the fix is pretty much 
what you've shown (replacing try-except with ignore_error) but I'd like 
my test case to be a bit more convincing.


Thanks!
Luke



On 30/06/15 09:45, Holl, Marcus wrote:

Hi,

I have an issue with the git p4 tooling regarding import of labels.

My git version is 2.4.5

I try to transform a perforce repository. My command line is:
git p4 clone --verbose --detect-branches --import-local
--import-labels --destination DESTINATION //depot@all


The relevant parts in the gitconfig is:
[git-p4]
 branchUser = USERNAME


For that user there is a branch mapping defined with a lot of entries
like:
//depot/trunk/... //depot/branches/ipro-status-8-2--branch/...
//depot/trunk/... //depot/branches/9-0-preview/...
//depot/trunk/... //depot/branches/release-8-0-0-branch/...
//depot/trunk/... //depot/branches/release-8-1-0-branch/...
//depot/trunk/... //depot/branches/release-8-2-0-branch/...
//depot/trunk/... //depot/branches/release-8-3-0-branch/...
//depot/trunk/... //depot/branches/release-8-4-branch/...
//depot/trunk/... //depot/branches/release-8-5-branch/...
...


The import fails with the log output that can be found at the bottom
of this mail.

git log -all -grep \[git-p4:.*change\ =\ 69035\] reports nothing.
The commit is not contained in the git repository.

p4 describe for changelist 69035 returns a reasonable result. This
change contains one file located at a path in the perforce folder
structure that comes without corresponding entry in the perforce
branch mapping.

According to the given branch mapping it looks reasonable to me that
the change is omitted in the git repository. But in my opinion the
import should not fail in such a case.

A reasonable behavior would be to blacklist the label (add it to
git-p4.ignoredP4Labels) and to continue with the next label.

Attached is a proposal for a fix that needs to be carefully reviews
since I'm not that experienced with python.

Other proposals for resolving this issue are highly appreciated.

Thanks a lot and best regards,
Marcus Holl


Log output:

Reading pipe: ['git', 'rev-list', '--max-count=1', '--reverse',
':/\\[git-p4:.*change = 69035\\]']
fatal: ambiguous argument ':/\[git-p4:.*change = 69035\]': unknown
revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git command [revision...] -- [file...]'
ied with change: 69078, original Date: 2010-04-22T09:07:24.00Z\n',
'Update': '2013/11/02 07:40:31', 'Label': 'release-8-1-0-976',
'Access': '2015/06/26 14:50:15', 'Owner': 'svn_p4_converter',
'Options': 'unlocked noautoreload'}
p4 label release-8-1-0-976 mapped to git commit
82a11809928b86a7bde03cf486428de52ab3380f
writing tag release-9-0-0-179 for commit
fb8370cd04806686c567ad720d065436f2334b4a
labelDetails= {'code': 'stat', 'Description': 'Created or modified
with change: 96984, original Date: 2011-12-22T16:01:25.681427Z\n',
'Update': '2013/11/02 15:15:50', 'Label': 'release-9-0-0-179',
'Access': '2015/06/26 14:50:16', 'Owner': 'build', 'Options':
'unlocked noautoreload'}
p4 label release-9-0-0-179 mapped to git commit
fb8370cd04806686c567ad720d065436f2334b4a
Traceback (most recent call last):
   File /usr/lib/git/git-p4, line 3297, in module
 main()
   File /usr/lib/git/git-p4, line 3291, in main
 if not cmd.run(args):
   File /usr/lib/git/git-p4, line 3165, in run
 if not P4Sync.run(self, depotPaths):
   File /usr/lib/git/git-p4, line 3045, in run
 self.importP4Labels(self.gitStream, missingP4Labels)
   File /usr/lib/git/git-p4, line 2421, in importP4Labels
 --reverse, :/\[git-p4:.*change = %d\] % changelist])
   File /usr/lib/git/git-p4, line 138, in read_pipe
 die('Command failed: %s' % str(c))
   File /usr/lib/git/git-p4, line 106, in die
 raise Exception(msg)
Exception: Command failed: ['git', 'rev-list', '--max-count=1',
'--reverse', ':/\\[git-p4:.*change = 69035\\]']





--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-p4: fix faulty paths for case insensitive systems

[Luke Diamand]

On 02/08/15 16:15, larsxschnei...@gmail.com wrote:

From: Lars Schneider larsxschnei...@gmail.com

Hi,

I want to propose this patch as it helped us to migrate a big source code base
successfully from P4 to Git. I am sorry that I don't provide a test case, yet.


Case sensitivity is a pretty tricky area with p4 - it's very brave of 
you to have a go at fixing it!



I would like to get advise on the patch and on the best strategy to provide a
test. Do you only run git-p4 integration tests in t/t98??-git-p4-*.sh? If yes,
which version of start_p4d should I use?


Only the t98* tests relate to git-p4 so if you just copy one of those it 
should do the right thing.


t9819-git-p4-case-folding.sh already has a few failing tests for this 
problem. I wrote it a while back just to illustrate the problem, so it 
might be of use to you, or you might need to start again.


Won't your change make importing much slower for people with this problem?

Also, I'm not sure you can use core.ignorecase to trigger this: the 
problem will arise if the *server* is ignoring case as well (which I 
think you can detect by querying the server).


I'm not trying to be negative - but this problem does have some annoying 
pitfalls! Let me know if you think I can help though.


Regards!
Luke

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] git-p4: Add option to ignore empty commits

[Luke Diamand]

On 24/10/15 17:28, Lars Schneider wrote:





Also I have this suspicion that those who do want to use client spec
to get a narrowed view into the history would almost always want
this "ignore empty" behaviour (I'd even say the current behaviour to
leave empty commits by default is a bug).  What are the advantages
of keeping empty commits?  If there aren't many, perhaps git-p4
should by the default skip empties and require p4.keepEmpty
configuration to keep them?


I agree.
@Luke: What option do you prefer? "git-p4.keepEmptyCommits" or 
"git-p4.ignoreEmptyCommits" ?


keepEmptyCommits.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 0/2] git-p4: Fix tests on OS X

[Luke Diamand]

On 12/10/15 18:03, larsxschnei...@gmail.com wrote:

From: Lars Schneider 

I extracted this patch series from "[PATCH v3 0/3] Add Travis CI support" as
suggested by Junio.


All seems sensible to me.

Luke




Thanks,
Lars

Lars Schneider (2):
   git-p4: Improve test case portability for t9815 git-p4-submit-fail
   git-p4: Skip t9819 test case on case insensitive file systems

  t/t9815-git-p4-submit-fail.sh  | 7 ++-
  t/t9819-git-p4-case-folding.sh | 6 ++
  2 files changed, 8 insertions(+), 5 deletions(-)

--
2.5.1



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] git-p4: Add option to ignore empty commits

[Luke Diamand]

On 24/10/15 19:08, Lars Schneider wrote:


On 21 Oct 2015, at 08:32, Luke Diamand <l...@diamand.org> wrote:


On 19/10/15 19:43, larsxschnei...@gmail.com wrote:

From: Lars Schneider <larsxschnei...@gmail.com>



This seems to be adding a new function in the middle of an existing function. 
Is that right?

That is true. I could move these functions into the P4Sync class if you don't 
like them here. I added them right there because it is the only place where 
they are used/useful.


It just seemed a bit confusing, but I'm ok with them here as well.







+if not self.clientSpecDirs:
+return True
+inClientSpec = self.clientSpecDirs.map_in_client(path)
+if not inClientSpec and self.verbose:
+print '\n  Ignoring file outside of client spec' % path
+return inClientSpec


Any particular reason for putting a \n at the start of the message?

I did this because "sys.stdout.write("\rImporting revision ..." (line 2724) does not add a newline. However, I agree that this 
looks stupid. I will remove the "\n" and fix the "Import revision" print. Speaking of that one: this is only printed if 
"not self.silent". Is there a particular reason to have "self.silent" and "self.verbose"? Should we merge the two?


self.silent and self.verbose seem like two slightly different things. I 
would expect silent to prevent any output at all. But actually it doesn't.


If we wanted to implement it properly, I think we'd need a new function 
(p4print?) which did the obvious right thing. Maybe something for a 
rainy day in the future.







Also, could you use python3 style print stmnts, print("whatever") ?

Sure. How do you prefer the formatting? Using "format" would be true Python 3 
style I think:
print('Ignoring file outside of client spec: {}'.format(path))


Will that breaker older versions of python? There's a statement 
somewhere about how far back we support.



OK?




+
+def hasBranchPrefix(path):
+if not self.branchPrefixes:
+return True
+hasPrefix = [p for p in self.branchPrefixes
+if p4PathStartsWith(path, p)]
+if hasPrefix and self.verbose:
+print '\n  Ignoring file outside of prefix: %s' % path
+return hasPrefix
+
+files = [f for f in files
+if inClientSpec(f['path']) and hasBranchPrefix(f['path'])]
+
+if not files and gitConfigBool('git-p4.ignoreEmptyCommits'):
+print '\n  Ignoring change %s as it would produce an empty commit.'
+return


As with Junio's comment elsewhere, I worry about deletion here.

I believe this is right. See my answer to Junio.



+
  self.gitStream.write("commit %s\n" % branch)
  #gitStream.write("mark :%s\n" % details["change"])
  self.committedChanges.add(int(details["change"]))
@@ -2403,7 +2412,7 @@ class P4Sync(Command, P4UserMap):
  print "parent %s" % parent
  self.gitStream.write("from %s\n" % parent)

-self.streamP4Files(new_files)
+self.streamP4Files(files)
  self.gitStream.write("\n")

  change = int(details["change"])
diff --git a/t/t9826-git-p4-ignore-empty-commits.sh 
b/t/t9826-git-p4-ignore-empty-commits.sh
new file mode 100755
index 000..5ddccde
--- /dev/null
+++ b/t/t9826-git-p4-ignore-empty-commits.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+
+test_description='Clone repositories and ignore empty commits'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+test_expect_success 'Create a repo' '
+   client_view "//depot/... //client/..." &&
+   (
+   cd "$cli" &&
+
+   mkdir -p subdir &&
+
+   >subdir/file1.txt &&
+   p4 add subdir/file1.txt &&
+   p4 submit -d "Add file 1" &&
+
+   >file2.txt &&
+   p4 add file2.txt &&
+   p4 submit -d "Add file 2" &&
+
+   >subdir/file3.txt &&
+   p4 add subdir/file3.txt &&
+   p4 submit -d "Add file 3"
+   )
+'
+
+test_expect_success 'Clone repo root path with all history' '
+   client_view "//depot/... //client/..." &&
+   test_when_finished cleanup_git &&
+   (
+   cd "$git" &&
+   git init . &&
+   git p4 clone --use-client-spec --destination="$git" //depot@all 
&&
+   cat >expect <<-\EOF &&
+Add file 3
+[git-p4: depot-paths = "//depot/": change = 3]
+
+Add file 2
+[git-p4: 

Re: [PATCHv1 2/2] git-p4: work with a detached head

[Luke Diamand]

On 28/10/15 17:44, Junio C Hamano wrote:

Luke Diamand <l...@diamand.org> writes:


On 9 September 2015 at 22:52, Junio C Hamano <gits...@pobox.com> wrote:

Luke Diamand <l...@diamand.org> writes:
...
 def currentGitBranch():
 return read_pipe("git name-rev HEAD").split(" ")[1].strip()

Yuck.  I know it is not entirely the fault of this patch, but
shouldn't it be reading from

 $ git symbolic-ref HEAD

and catch the error "fatal: ref HEAD is not a symbolic ref" and use
it as a signal to tell that the HEAD is detached?


That sounds much nicer. I'll redo the patch accordingly.


No need to rush, but should I expect a reroll of this sometime, or
have things around this topic changed to make this topic no longer
necessary?  I am only asking so that I can decide to either keep or
drop ld/p4-detached-head topic that is listed in the [Stalled]
section for quite some time [*1*].


I was waiting for the other git-p4 changes to go through before starting 
this up again.


It definitely needs fixing - it was annoying me a lot today, as I kept 
on having to invent temporary branch names to needlessly keep git-p4 happy.


After getting to "for-p4-9", I'm now onto "x". I'll see if I can 
sort something out in the next few days.


Luke




Thanks.


[Footnote]

*1* Not that my dropping a topic from 'pu' means very much; a
 dropped topic can still be submitted and requeued after all.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout

[Luke Diamand]

On 15/11/15 13:08, larsxschnei...@gmail.com wrote:

From: Lars Schneider 

In rare cases kill/cleanup operations in tests fail. Retry these
operations with a timeout to make the test less flaky.


Should there be a sleep in that retry_until_success loop so that it 
doesn't spin sending signals to p4d?


Do we need to worry about the time offset being updated (e.g. NTP) while 
this is running?




Signed-off-by: Lars Schneider 
---
  t/lib-git-p4.sh | 31 +++
  1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7548225..8d6b48f 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -6,6 +6,10 @@
  # a subdirectory called "$git"
  TEST_NO_CREATE_REPO=NoThanks

+# Some operations require multiple attempts to be successful. Define
+# here the maximal retry timeout in seconds.
+RETRY_TIMEOUT=60
+
  . ./test-lib.sh

  if ! test_have_prereq PYTHON
@@ -121,22 +125,33 @@ p4_add_user() {
EOF
  }

+retry_until_success() {
+timeout=$(($(date +%s) + $RETRY_TIMEOUT))
+until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
+do :


 sleep here?


+done
+}
+
+retry_until_fail() {
+timeout=$(($(date +%s) + $RETRY_TIMEOUT))
+until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
+do :


 sleep here?


+done
+}
+
  kill_p4d() {
pid=$(cat "$pidfile")
-   # it had better exist for the first kill
-   kill $pid &&
-   for i in 1 2 3 4 5 ; do
-   kill $pid >/dev/null 2>&1 || break
-   sleep 1
-   done &&
+   retry_until_fail kill $pid
+   retry_until_fail kill -9 $pid
# complain if it would not die
test_must_fail kill $pid >/dev/null 2>&1 &&
rm -rf "$db" "$cli" "$pidfile"
  }

  cleanup_git() {
-   rm -rf "$git" &&
-   mkdir "$git"
+   retry_until_success rm -r "$git"
+   test_must_fail test -d "$git" &&
+   retry_until_success mkdir "$git"
  }

  marshal_dump() {



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html