Re: [PATCH v2 3/4] git-p4: Fix t9815 git-p4-submit-fail test case on OS X

[Pete Wyckoff]

larsxschnei...@gmail.com wrote on Sun, 04 Oct 2015 11:44 -0700:
> 
> On 04 Oct 2015, at 11:23, Junio C Hamano <gits...@pobox.com> wrote:
> 
> > larsxschnei...@gmail.com writes:
> > 
> >> +  if test_have_prereq CYGWIN; then
> >> +  : # NOOP
> >> +  elif test_have_prereq DARWIN; then
> >> +  stat -f %Sp text | egrep ^-r-- &&
> >> +  stat -f %Sp text+x | egrep ^-r-x
> >> +  else
> >>stat --format=%A text | egrep ^-r-- &&
> >>stat --format=%A text+x | egrep ^-r-x
> >>fi
> > 
> > Not a new problem but why do we need "stat" here?
> > 
> > Shouldn't "test -r", "! test -x", and their usual friends be
> > sufficient for the purpose of the test and are more portable?
> 
> Good question. The stat call was introduced with df9c545 by Pete Wyckoff.
> @Pete, @Luke: Are you aware of any particular reason for stat?

I think you could do this all with test. The key is to make
sure the files are readable, not writable, and either executable
or not. Cygwin and darwin oddities were not on my radar 3 years ago.

See also 4cea4d6 (git p4 test: use test_chmod for cygwin,
2013-01-26) for the description I wrote about what this test is
trying to verify.

-- 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

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

[Pete Wyckoff]

l...@diamand.org wrote on Fri, 23 Jan 2015 09:15 +:
 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
  

Looks obviously good here.  Ack!

-- 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

git-p4 maintainership change

[Pete Wyckoff]

Hi Junio. I'm fortunate enough to need no longer any git
integration with Perforce (p4). I work only in git these days.
Thus you might expect my interest in improving git-p4 would
be waning.

Luke, on the other hand, continues to need git-p4 and is
active in improving it. I think you should consider accepting
patches in that area from him directly. He's contributed many
patches over the years and has helped users to debug their issues
too.

I'll certainly be available to comment on any dodgy code in there
already, and can help with archeological, but will not likely do
any substantive work to git-p4 in the near future.

Luke: I think you have a couple patches outstanding; hope these
can get merged to mainline soon.

Thanks,

--  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

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

[Pete Wyckoff]

l...@diamand.org wrote on Sat, 17 Jan 2015 20:56 +:
 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 :-(

So sorry. I on the other hand have been fortunate enough to
switch to using only git.

Nevertheless, I read through the patch and it looks good and
makes sense. You've got my ack on this for what it's worth.
Hopefully someone else starts picking up the git-p4 maintenance
work. Hint.

-- 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

Re: [PATCH] t0090: mark add-interactive test with PERL prerequisite

[Pete Wyckoff]

jrnie...@gmail.com wrote on Tue, 18 Nov 2014 10:43 -0800:
 ... and here's a patch on top to give git-p4 the same treatment.
 
 -- 8 --
 Subject: Makefile: have python scripts depend on NO_PYTHON setting
 
 Like the perl scripts, python scripts need a dependency to ensure they
 are rebuilt when switching between the dummy versions that run
 without Python and the real thing.
 
 Signed-off-by: Jonathan Nieder jrnie...@gmail.com
 ---
  Makefile | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/Makefile b/Makefile
 index 8f980e0..7482a4d 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1736,6 +1736,9 @@ $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
   mv $@+ $@
  endif # NO_PERL
  
 +# This makes sure we depend on the NO_PYTHON setting itself.
 +$(SCRIPT_PYTHON_GEN): GIT-BUILD-OPTIONS
 +
  ifndef NO_PYTHON
  $(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
  $(SCRIPT_PYTHON_GEN): % : %.py
 -- 
 2.1.0.rc2.206.gedb03e5

Looks obviously correct, thanks for remembering the other
scripting languages.  :)

Acked-by: Pete Wyckoff p...@padd.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

Re: [PATCH] git p4 test: fix failure in 9814-git-p4-rename.sh Was: Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Pete Wyckoff]

ml.christophbon...@gmail.com wrote on Wed, 23 Jul 2014 23:28 +0200:
 The scenario in the rename test makes unnecessary assumptions about
 which file git file-tree will detect as a source for a copy-operations.
 Furthermore, copy detection is not tested by checking the resulting
 perforce revision history via p4 filelog, but via git diff-tree.
 
 This patch makes the test more robust by accepting each of the possible
 sources, and more rigorous by doing so via p4 filelog.

I see what you're doing here, and it all looks good.  The
diff-tree checks were mainly debugging, and if p4 filelog
shows the right branch from, that means it works.

You might be able to firm up the branch from lines for file8
and file9 too, but those aren't likely to fail.

Indeed, as noted in the other thread, it would be better to make
these not so flaky, but your change here fixes a problem, and
doesn't do any harm.  And gives you an opportunity to fix it more
later.  :)

Be sure to fix the word-wrapping you have on two of the lines
below.  And be careful not to top post.

Here's my ack for when you decide to send it back to the list,
cc junio.

Acked-by: Pete Wyckoff p...@padd.com

 ---
  t/t9814-git-p4-rename.sh | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)
 
 diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
 index 1fc1f5f..4068510 100755
 --- a/t/t9814-git-p4-rename.sh
 +++ b/t/t9814-git-p4-rename.sh
 @@ -156,18 +156,16 @@ test_expect_success 'detect copies' '
   git diff-tree -r -C HEAD 
   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 
 
   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) 
 - test $src = file10 
   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 -E branch from //depot/file(2|10) 
 
   cp file2 file12 
   echo some text file12 
 @@ -177,7 +175,7 @@ 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) 
 - test $src = file10 || test $src = file11 
 + test $src = file2 || test $src = file10 || test $src = file11 
   git config git-p4.detectCopies $(($level + 2)) 
   git p4 submit 
   p4 filelog //depot/file12 
 @@ -190,12 +188,10 @@ test_expect_success 'detect copies' '
   git diff-tree -r -C --find-copies-harder HEAD 
   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) 
 - test $src = file10 || test $src = file11 || test $src = file12 
   git config git-p4.detectCopies $(($level - 2)) 
   git p4 submit 
   p4 filelog //depot/file13 
 - p4 filelog //depot/file13 | grep -q branch from //depot/file
 + p4 filelog //depot/file13 | grep -q -E branch from 
 //depot/file(2|10|11|12)
   )
  '
 
 -- 
 2.0.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: git-p4 and initial import

[Pete Wyckoff]

lcharri...@promptu.com wrote on Thu, 10 Jul 2014 15:45 +0200:
 I've used git-p4 for several years now and it's generally working well for
 me.
 
 The only thing that bugs me at this time is having to re-clone regularly.
 Here is how this happens:
 
 * Say my p4 client maps //foo/bar/... to /home/jdoe/perforce/foo/bar/... (I
 don't want to clone the entire repo, because it's too big).
 * I do git p4 clone --use-client-spec //foo /home/jdoe/git/foo, work with
 it, all goes well.
 * Meanwhile, at some point somebody else adds //foo/baz.
 * Eventually I need //foo/baz. I add it to my p4 client.
 * Naturally, git-p4 won't pick up the changes, because they happened before
 I added //foo/baz to my client.
 * So I git reset --hard to the first commit, delete even that using git
 update-ref -d HEAD, then again I do git p4 clone //foo /home/jdoe/git/foo.
 When the repo gets big, this takes a lot of time.
 
 So, I have a few questions:
 1. Am I doing this wrong? Is there another way I could proceed?

What you observe makes sense.  You do need somehow to sync in the
change that added the files in //foo/baz.  Say that path was
added in change 5, and you already synced change 5 but did not 
have //foo/baz in your client spec.  It would not be in the git
commit corresponding to change 5.

Now you don't have to reset all the way back to the first commit,
you can just rewind back to the one that introduced //foo/baz.
This still is disruptive and takes a while, depending on how long
between when //foo/baz got added and when you decided you needed
it.

Say there's just a few changes to //foo/baz, among the thousands
of ones you've already synced.  You can surgically go in and
re-sync just those few changes, rebasing the rest of the changes
after each one.  Something like:

p4change=3227  ;# want to reimport this one as it added baz
sha=$(git rev-parse :/change = $p4change)
git update-ref refs/remotes/p4/master $sha^
git p4 sync @$p4change  ;# re-sync 3227, now with baz
git rebase --onto p4/master $sha  ;# put everything back on top

Now you don't have to re-import change 3228 etc.  Repeat until
you've fixed up all the baz.  Of course the rebases might be
slow...

If you don't care about your history, you could make a new branch
and import just //foo/baz into the new branch.

git p4 sync --branch refs/remotes/p4/baz //foo/baz

Then merge it into your main branch.  You may have to use a
different client spec to put the baz files in the right subdir
name.  Or filter-branch --subdirectory-filter.  Now adjust your
main client spec and future git p4 sync will grab both //foo/bar
and //foo/baz.  Of course you have this odd wart in your history
where they got glued together.

If you do script up something cool, be sure to send it in for
contrib/ or even a magic option in git-p4.

 2. It occurred to me that when I re-clone a repository using
 --use-client-spec, I already have everything I need in my local copy of the
 p4 client. Why does git-p4 need to redownload everything from the
 repository? Could we find a way to tell it to p4 sync, then fetch the files
 from the local copy? Or is there a way I can copy everything over from my
 local client and pretend this is the initial import?

That should work in theory.  We've got all the blobs (file
objects) already. If we had a database that mapped each p4
file#revision to a blob, git p4 sync could look at that and see
if it already has the blob.  Possibly with
.git/objects/info/alternates to grab them from somewhere nearby.

But we don't have that database.  Git p4 knew the mapping when it
did the syncs, but didn't write them down.  You could script up
something to recreate this by asking p4 for the revision of each
file for each change already in git.  And git already knows the
blob for each of files in those changes.  With this mapping, you
could modify git p4 to check for the blob first, before doing p4
print on the file#rev.  See also git-p4raw from Sam Vilain that
builds up SQL from a raw p4 db.

Fun bit of work maybe if you're motivated.

-- 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

Re: git p4 diff-tree ambiguous argument error

[Pete Wyckoff]

duanemur...@mac.com wrote on Thu, 10 Jul 2014 12:19 -0700:
 Some additional investigation. 
 
 I am working in a copy of a repository that was originally used to pull the 
 data 
 from Perforce. As part of my experiments to figure out this problem, I 
 deleted 
 the contents of .git/git-p4-tmp/. 
 
 I noticed that git-p4 would continue if those files were present. I have now 
 copied the files that were in .git/git-p4-tmp/ from the other repository. 
 
 git-p4 is not crashing now, but I also noticed that none of the dates on 
 these files
 have changed. These files should have been touched each time that a branch is 
 taken,
 but these files have not changed while the sync is running.
 
 That seems significant. 
 
 I expect git-p4 to crash again on a new commit that is not in 
 .git/git-p4-tmp/. 
 Then I have to start the 8-12 hour process over again (did I mention 70k 
 commits?).

Bizarre.  That directory is really supposed to be temporary, and
live only during a single git p4 invocation.  It's just a bunch
of branch heads for the temporary commits.  I don't know why
those branches, and the git-p4-tmp directory, hang around after
you run git p4.  Might be worth your investigation.

The second weirdness is why a new run doesn't create the branch.
This maybe points to self.checkpoint() not really checkpointing.
It does send a checkpoint down the git fast-import stream,
which is supposed to make it write the branches out.  You might
consider grabbing the fast-import process in a debugger and see
why it's not writing out the branch head.

There's lots of changes since v1.7.12.4, but nothing obvious I
can see that would cause this.  Sorry,

-- 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

Re: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Pete Wyckoff]

ml.christophbon...@gmail.com wrote on Sun, 06 Jul 2014 16:32 +0200:
 I'm trying to get the git p4 tests to pass on my machine (OS X
 Mavericks) from master before making some changes. I'm experiencing a
 test failure in detect copies of the rename test.
 
 The test creates file2 with some content, creates a few copies (each
 with a commit), then does the following (no git write operations
 omitted):
 echo file2 file2 
 cp file2 file10 
 git add file2 file10 
 git commit -a -m Modify and copy file2 to file10 
 ... (some non-write-operations) ...
 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) 
 test $src = file10 
 
 This is where it fails on my machine. The git diff-tree output is this
 :100644 100644 22a35c17c4c0779f75142036beef6ccd58525b9c
 22a35c17c4c0779f75142036beef6ccd58525b9c C100 file2 file11
 so git diff-tree sees file2 as the copy source, not file10. In my
 opinion, the diff-tree result is legitimate (at that point, file2,
 file10 and file11 are identical). Later in the tests, after making
 more copies of file2, the conditions are more flexible, e.g.
 test $src = file10 || test $src = file11 || test $src = file12 
 
 IMO, the test discounts the legitimate possibility of diff-tree
 detecting file2 as source, making unnecessary assumptions about
 implementation details. Is this correct, or do I misunderstand the
 workings of diff-tree?
 
 I'd be grateful for advice, both on whether this is a bug, and if so,
 which branch to base a patch on.

I think your analysis is correct.  And I agree that later tests
have noticed this ambiguity and added multiple comparisons like
you quote.

I'm not sure how to robustify this.  At least doing the multiple
comparisons should make the tests work again.  The goal of this
series of tests is to make sure that copy detection is working,
not to verify that the correct copy choice was made.  That should
be in other (non-p4) tests.

Do send patches based on Junio's master.  I can ack, and they'll
show up in a future git release.

Thanks!

-- 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

Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode

[Pete Wyckoff]

frrr...@gmail.com wrote on Wed, 11 Jun 2014 14:06 +0100:
 On Tue, Jun 10, 2014 at 06:39:58PM -0400, Pete Wyckoff wrote:
  frrr...@gmail.com wrote on Tue, 10 Jun 2014 13:14 +0100:
   b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here
   is a proper fix, including proper handling for windows end of lines.
  
  I guess we don't have test coverage for these cases?  Is this
  something that should get put into a maintenance release, quickly?
 
 We have test cases for that, however we need to create a link to git-p4.py
 named git-p4 in order for them to work. I did not run the first patch through
 the tests (see my previous email) because of that. Sorry about that.

The secret is to build the code before running tests, just like
when working on .c files.  I tend to do something like:

make git-p4  (cd t ; make T=$(echo t98*)) ; pkill p4d

Thanks for catching the problem quickly and fixing it.

-- 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

Re: [PATCH] Fix git-p4 submit in non --prepare-p4-only mode

[Pete Wyckoff]

frrr...@gmail.com wrote on Tue, 10 Jun 2014 13:14 +0100:
 b4073bb387ef303c9ac3c044f46d6a8ae6e190f0 broke git p4 submit, here
 is a proper fix, including proper handling for windows end of lines.

I guess we don't have test coverage for these cases?  Is this
something that should get put into a maintenance release, quickly?

The fix looks good.  It's surprising that none of the tests
managed to add a file and trigger the failure.

I'll ack this again, as it looks okay, but hope you ran all the
unit tests successfully on your machine.

-- Pete

 Signed-off-by: Maxime Coste frrr...@gmail.com
 ---
  git-p4.py | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/git-p4.py b/git-p4.py
 index 7bb0f73..ff132b2 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -1238,7 +1238,7 @@ class P4Submit(Command, P4UserMap):
  if response == 'n':
  return False
  
 -def get_diff_description(self, editedFiles):
 +def get_diff_description(self, editedFiles, filesToAdd):
  # diff
  if os.environ.has_key(P4DIFF):
  del(os.environ[P4DIFF])
 @@ -1258,7 +1258,7 @@ class P4Submit(Command, P4UserMap):
  newdiff += + + line
  f.close()
  
 -return diff + newdiff
 +return (diff + newdiff).replace('\r\n', '\n')
  
  def applyCommit(self, id):
  Apply one commit, return True if it succeeded.
 @@ -1422,10 +1422,10 @@ class P4Submit(Command, P4UserMap):
  separatorLine =  everything below this line is just the 
 diff ###\n
  if not self.prepare_p4_only:
  submitTemplate += separatorLine
 -submitTemplate += self.get_diff_description(editedFiles)
 +submitTemplate += self.get_diff_description(editedFiles, 
 filesToAdd)
  
  (handle, fileName) = tempfile.mkstemp()
 -tmpFile = os.fdopen(handle, w+)
 +tmpFile = os.fdopen(handle, w+b)
  if self.isWindows:
  submitTemplate = submitTemplate.replace(\n, \r\n)
  tmpFile.write(submitTemplate)
 @@ -1475,9 +1475,9 @@ class P4Submit(Command, P4UserMap):
  tmpFile = open(fileName, rb)
  message = tmpFile.read()
  tmpFile.close()
 -submitTemplate = message[:message.index(separatorLine)]
  if self.isWindows:
 -submitTemplate = submitTemplate.replace(\r\n, \n)
 +message = message.replace(\r\n, \n)
 +submitTemplate = message[:message.index(separatorLine)]
  p4_write_pipe(['submit', '-i'], submitTemplate)
  
  if self.preserveUser:
 -- 
 2.0.0
 
 
--
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: Do not include diff in spec file when just preparing p4

[Pete Wyckoff]

frrr...@gmail.com wrote on Sat, 24 May 2014 02:39 +0100:
 The diff information render the spec file unusable as is by p4,
 do not include it when run with --prepare-p4-only so that the
 given file can be directly passed to p4.
 
 With --prepare-p4-only, git-p4 already tells the user it can use
 p4 submit with the generated spec file. This fails because of the
 diff being present in the file. Not including the diff fixes that.
 
 Without --prepare-p4-only, keeping the diff makes sense for a
 quick review of the patch before submitting it. And does not cause
 problems with p4 as we remove it programmatically.
 
 Signed-off-by: Maxime Coste frrr...@gmail.com

Hi Maxime.  This looks really good.  Even the Windows section
is fine; thanks for paying attention there too.

I'm not particularly worried about having a new test for this.
Your tweak to the existing 9807 is fine.  Unless of course you
have one ready to go.

Acked-by: Pete Wyckoff p...@padd.com

You might add my ack and send it directly to Junio + CC the list.
It'll be a nice improvement for the next available release.

-- 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

Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

[Pete Wyckoff]

tolga.cey...@gmail.com wrote on Fri, 02 May 2014 22:40 -0700:
 
 
 This is the error message git-apply emits in this case:
 
 error: cannot apply binary patch to 'filename' without full index line
 error: filename: patch does not apply
 
 Cheers,
 Tolga
 
 Any feedback is appreciated.

Sorry, travel delay.  This explanation is pretty
straight-forward, thanks.

Suggest you include it in the commit message along with the
other text you had, and resend to the list, cc me and junio.
Oh, and include an ack:

Acked-by: Pete Wyckoff p...@padd.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

Re: git p4: bug - branch detection broken on empty branches

[Pete Wyckoff]

dpr...@gmail.com wrote on Tue, 22 Apr 2014 10:20 +0100:
 As part of my work to help get git-p4 close to bug-free before Git
 2.0, I'm posting all bugs and patches to this mailing list. Please
 direct me elsewhere if this is incorrect.
 
 When trying to clone a particular directory from a depot, that
 contains one or more branches that contain no commits for that
 directory, branch detection is broken and results in a failed clone.
 
 fatal: ambiguous argument
 'refs/remotes/p4/silly_project_branch/trunk': unknown revision or path
 not in the working tree.
[..]
   File /home/dreid/bin/git-p4, line 2678, in importChanges
 blob = self.searchParent(parent, branch, tempBranch)
   File /home/dreid/bin/git-p4, line 2600, in searchParent
 for blob in read_pipe_lines([git, rev-list, --reverse,
   File /home/dreid/bin/git-p4, line 155, in read_pipe_lines
 die('Command failed: %s' % str(c))
   File /home/dreid/bin/git-p4, line 106, in die
 raise Exception(msg)
 Exception: Command failed: ['git', 'rev-list', '--reverse',
 '--no-merges', 'refs/remotes/p4/silly_project_branch/trunk']
 
 Original command:
 $ git-p4 clone 
 //insane_depot/projects/Exchange/CompanyName/silly_project_branch@all
 silly-project --detect-branches -v

Yes, this is a good bug.  You could do git rev-parse -q --verify
on parent before trying to read the rev-list.

But then what should happen?  I suspect git-p4 will just create
that ref when it commits the change it is considering.

-- 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

Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

[Pete Wyckoff]

tolga.cey...@gmail.com wrote on Thu, 24 Apr 2014 21:46 -0700:
 When applying binary patches a full index is required. format-patch
 already handles this, but diff-tree needs '--full-index' argument
 to always output full index.
 
 Signed-off-by: Tolga Ceylan tolga.cey...@gmail.com
 ---
  git-p4.py |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/git-p4.py b/git-p4.py
 index cdfa2df..4ee6739 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -1311,7 +1311,7 @@ class P4Submit(Command, P4UserMap):
  else:
  die(unknown modifier %s for %s % (modifier, path))
  
 -diffcmd = git diff-tree -p \%s\ % (id)
 +diffcmd = git diff-tree --full-index -p \%s\ % (id)
  patchcmd = diffcmd +  | git apply 
  tryPatchCmd = patchcmd + --check -
  applyPatchCmd = patchcmd + --check --apply -
 -- 

This looks like a straightforward change, but can you give a
bit more background on why a full index is required?  Do you
mean that git apply will reject a patch with abbreviated
blob object names?

-- 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

Re: Fwd: git p4: feature request - branch check filtering

[Pete Wyckoff]

dpr...@gmail.com wrote on Tue, 22 Apr 2014 10:29 +0100:
 There is a patch viewable at this link:
 https://github.com/Stealthii/git/commit/f7a2e611262fd977ac99e066872d3d0743b7df3c
 
 For the use case this works perfectly - if I define branch mappings
 with git config, followed by setting 'git-p4.skipBranchScan' to true,
 git-p4 will skip scanning of all remote branches and limit to what's
 defined in the map.  An example config:
 
 [git-p4]
 skipBranchScan = true
 branchList = release_1.0.0:release_1.1.0
 branchList = release_1.1.0:release_1.2.0
 
 If there is any more information I need to provide let me know. I have
 been using this patch for over two months, testing both use cases with
 and without git-p4.skipBranchScan and I have noticed no issues.  Logic
 of git-p4 is not changed from default behaviour, unless the user
 explicitly sets the boolean flag to skip scanning.

Thanks, Dan.  This looks good and is a fine compromise
considering the various choices we discussed earlier.

Junio's comments about 2.0 non-withstanding, I think this change
should go into the next convenient release.  So 2.1 or 2.0.1;
however the numbers end up working post-2.0.

If you could take a look at Documentation/SubmittingPatches,
and do a few things:

1.  Write a nice commit message, say:

git p4: add skipBranchScan to avoid p4 branch scan

Some more useful text.

2.  Include at the bottom of that message:

Acked-by: Pete Wyckoff p...@padd.com

3.  Inline the text of your patch, not just a link to github.

4.  Consider adding a t98xx test.  This isn't required for
a fairly minor change like yours, but if you think TDD
is fun, have at it.  Might protect your feature against
future hackers who would try to break it.  :)

Then send it to vger, cc junio (and me), and he will be kind
enough to queue it up appropriately.

Thanks!

-- 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

Re: [PATCH] git-p4: explicitly specify that HEAD is a revision

[Pete Wyckoff]

vdog...@ixiacom.com wrote on Mon, 07 Apr 2014 16:19 +0300:
 'git p4 rebase' fails with the following message if there is a file
 named HEAD in the current directory:
 
   fatal: ambiguous argument 'HEAD': both revision and filename
   Use '--' to separate paths from revisions, like this:
   'git command [revision...] -- [file...]'
 
 Take the suggestion above and explicitly state that HEAD should be
 treated as a revision.
 
 Signed-off-by: Vlad Dogaru vdog...@ixiacom.com

This looks obviously good to me, thanks!

Junio, could you carry it into the next release?  As a trivial
fixup.

Acked-by: Pete Wyckoff p...@padd.com

 ---
  git-p4.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/git-p4.py b/git-p4.py
 index cdfa2df..8d11b25 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -3086,7 +3086,7 @@ class P4Rebase(Command):
  print Rebasing the current branch onto %s % upstream
  oldHead = read_pipe(git rev-parse HEAD).strip()
  system(git rebase %s % upstream)
 -system(git diff-tree --stat --summary -M %s HEAD % oldHead)
 +system(git diff-tree --stat --summary -M %s HEAD -- % oldHead)
  return True
  
  class P4Clone(P4Sync):
 -- 
 1.8.5.2
 
 
--
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: Fwd: git p4: feature request - branch check filtering

[Pete Wyckoff]

dpr...@gmail.com wrote on Tue, 18 Feb 2014 12:42 +:
 I work at a company that has recently moved all CVS, SVN, and git
 repositories to Perforce.  Depots have not been setup correctly in
 every case, and there is one depot that contains literally hundreds of
 projects under commercial development (and hundreds of branches as a
 result)

My condolences.

 My project may be in //stupid_depot/commercial/teamporter/rok.  This
 is the path I clone with git-p4.  The only branches in this depot that
 contain files at this path are titled as
 'rok_porter_branch/release_1.x' or similar.
 
 When using '--detect-branches' git-p4 checks each key of branches to
 see if any of them have files in the path I've cloned.  Whilst this is
 good in practice there is unfortunately 6,809 branches, git-p4
 processes about 2 a second and just under an hour to perform any
 git-p4 rebase, submit, or similar operation.

This is in getBranchMapping() presumably.  Where it loops
over each branch doing p4 branch -o.  Yuk.

You could always avoid the --detect-branches if you don't really
need it, instead doing, say, multiple git p4 sync for the
different areas of the repo that interest you, each with its own
destination branch in git (p4/depot-part1, p4/depot-part3,
...).  Or --use-client-spec to cobble together an exact mapping
of where p4 files should land in git, all in a single git branch
then.

 I propose the addition of a branch list filtering option
 (--filter-branches) that takes either a regular expression or list of
 branches it should check.  This may be useful in sane situations where
 you don't want to scan every branch in a Perforce repository, or
 blacklist branches that have undesirable content (for example, one of
 the branches is called 'svn-backup'.  It contains a single, multi-GB
 tarball.)

There is the existing git-p4.branchList option that explicitly
adds (or overrides) branch information, beyond the ones auto-discovered.

You might be able to use that option, but change its behavior
to avoid the scan.  So that if that option is set in the config,
p4 is not asked anything about its branches.  Not sure if this
would break anyone's setup though.

Another approach would be to add a config option
git-p4.branchScan that defaults to True.  You could turn it off
and use branchList.

 It would be ideal to have this information (after initial clone or
 sync) stored somewhere in the git config where is appropriate so that
 future submit/rebase operations adhere to this list.
 
 Has something like this been worked on, or has been considered in the
 past?  If not I will consider implementing this after reading up on
 the Git code guidelines.
 
 Thanks for keeping the Git workflow accessible in painful areas.

It would be great if you could get something like this to work.
Start in getBranchMapping() and don't forget to write up your
work in Documentation/git-p4.txt.  Also, this is sort of a messy
area of the code, unfortunately.  t/t9801 tries to make sure some
of it keeps working.

-- 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

Re: [PATCH 00/11] git p4 tests and a few bug fixes

[Pete Wyckoff]

gits...@pobox.com wrote on Tue, 21 Jan 2014 16:03 -0800:
 Pete Wyckoff p...@padd.com writes:
[..]
  Patch 03 is a regression fix, found and narrowed down thanks to
  much work by Damien Gérard.  But it is obscure enough that I'm
  not proposing it for a maintenance release.
 
 Thanks.
 
 I am inclined to say that we should queue this on a fork from
 'maint, merge the result to 'master' before 1.9-rc1 and ship the
 result as part of the upcoming release, and then possibly merging
 the topic to 1.8.5.x maintenance release after that.
 
 This is primarily because I personally do not have p4 expertise to
 test or properly judge this (iow, you are the area maintainer, the
 authority), and I somehow have this feeling that parking in 'next'
 for extended period of time would not give meaningfully larger
 exposure to the code.
 
 What do you think?
 
 If you feel uneasy about such a fast-track, I wouldn't push it,
 though.

I think you're right that fast-track is the best choice, and low
risk.  The diffs came out identical, and it merges cleanly to
master, and passes all tests in both.

Thanks Eric for the commit message fixes too!

Here comes a v2 that is otherwise identical, but based on
origin/maint from a couple weeks ago.

-- 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

[PATCHv2 01/11] git p4 test: wildcards are supported

[Pete Wyckoff]

Since 9d57c4a (git p4: implement view spec wildcards with p4
where, 2013-08-30), all the wildcard types should be supported.
Change must-fail tests to mark that they now pass.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9809-git-p4-client-view.sh | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 77f6349..23a827f 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -76,28 +76,28 @@ test_expect_success 'init depot' '
 '
 
 # double % for printf
-test_expect_success 'unsupported view wildcard %%n' '
+test_expect_success 'view wildcard %%n' '
client_view //depot/1/sub/... //client/sub/1/... 
test_when_finished cleanup_git 
-   test_must_fail git p4 clone --use-client-spec --dest=$git //depot
+   git p4 clone --use-client-spec --dest=$git //depot
 '
 
-test_expect_success 'unsupported view wildcard *' '
+test_expect_success 'view wildcard *' '
client_view //depot/*/bar/... //client/*/bar/... 
test_when_finished cleanup_git 
-   test_must_fail git p4 clone --use-client-spec --dest=$git //depot
+   git p4 clone --use-client-spec --dest=$git //depot
 '
 
-test_expect_success 'wildcard ... only supported at end of spec 1' '
+test_expect_success 'wildcard ... in the middle' '
client_view //depot/.../file11 //client/.../file11 
test_when_finished cleanup_git 
-   test_must_fail git p4 clone --use-client-spec --dest=$git //depot
+   git p4 clone --use-client-spec --dest=$git //depot
 '
 
-test_expect_success 'wildcard ... only supported at end of spec 2' '
+test_expect_success 'wildcard ... in the middle and at the end' '
client_view //depot/.../a/... //client/.../a/... 
test_when_finished cleanup_git 
-   test_must_fail git p4 clone --use-client-spec --dest=$git //depot
+   git p4 clone --use-client-spec --dest=$git //depot
 '
 
 test_expect_success 'basic map' '
-- 
1.8.5.2.364.g6ac45cd

--
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 04/11] git p4 test: explicitly check p4 wildcard delete

[Pete Wyckoff]

There was no test where p4 deleted a file with a wildcard
character.  Make sure git p4 applies the wildcard decoding
properly when importing a delete that includes a wildcard.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9812-git-p4-wildcards.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh
index 6763325..f2ddbc5 100755
--- a/t/t9812-git-p4-wildcards.sh
+++ b/t/t9812-git-p4-wildcards.sh
@@ -161,6 +161,33 @@ test_expect_success 'wildcard files submit back to p4, 
delete' '
)
 '
 
+test_expect_success 'p4 deleted a wildcard file' '
+   (
+   cd $cli 
+   echo wild delete test wild@delete 
+   p4 add -f wild@delete 
+   p4 submit -d add wild@delete
+   ) 
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   test_path_is_file wild@delete
+   ) 
+   (
+   cd $cli 
+   # must use its encoded name
+   p4 delete wild%40delete 
+   p4 submit -d delete wild@delete
+   ) 
+   (
+   cd $git 
+   git p4 sync 
+   git merge --ff-only p4/master 
+   test_path_is_missing wild@delete
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.5.2.364.g6ac45cd

--
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 03/11] git p4: work around p4 bug that causes empty symlinks

[Pete Wyckoff]

Damien Gérard highlights an interesting problem.  Some p4
repositories end up with symlinks that have an empty target.  It
is not possible to create this with current p4, but they do
indeed exist.

The effect in git p4 is that p4 print on the symlink returns an
empty string, confusing the curret symlink-handling code.

Such broken repositories cause problems in p4 as well, even with
no git involved.  In p4, syncing to a change that includes a
bogus symlink causes errors:

//depot/empty-symlink - updating /home/me/p4/empty-symlink
rename: /home/me/p4/empty-symlink: No such file or directory

and leaves no symlink.

In git, replicate the p4 behavior by ignoring these bad symlinks.
If, in a later p4 revision, the symlink happens to point to
something non-null, the symlink will be replaced properly.

Add a big test for all this too.

This happens to be a regression introduced by 1292df1 (git-p4:
Fix occasional truncation of symlink contents., 2013-08-08) and
appeared first in 1.8.5.  But it shows up only in p4
repositories of dubious character, so can wait for a proper
release.

Tested-by: Damien Gérard dam...@iwi.me
Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py  |  9 ++-
 t/t9802-git-p4-filetype.sh | 66 ++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 06a3cc6..3a20d15 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2075,7 +2075,14 @@ class P4Sync(Command, P4UserMap):
 # p4 print on a symlink sometimes contains target\n;
 # if it does, remove the newline
 data = ''.join(contents)
-if data[-1] == '\n':
+if not data:
+# Some version of p4 allowed creating a symlink that pointed
+# to nothing.  This causes p4 errors when checking out such
+# a change, and errors here too.  Work around it by ignoring
+# the bad symlink; hopefully a future change fixes it.
+print \nIgnoring empty symlink in %s % file['depotFile']
+return
+elif data[-1] == '\n':
 contents = [data[:-1]]
 else:
 contents = [data]
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 94d7be9..66d3fc9 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -267,6 +267,72 @@ test_expect_success SYMLINKS 'ensure p4 symlink parsed 
correctly' '
)
 '
 
+test_expect_success SYMLINKS 'empty symlink target' '
+   (
+   # first create the file as a file
+   cd $cli 
+   empty-symlink 
+   p4 add empty-symlink 
+   p4 submit -d add empty-symlink as a file
+   ) 
+   (
+   # now change it to be a symlink to target1
+   cd $cli 
+   p4 edit empty-symlink 
+   p4 reopen -t symlink empty-symlink 
+   rm empty-symlink 
+   ln -s target1 empty-symlink 
+   p4 add empty-symlink 
+   p4 submit -d make empty-symlink point to target1
+   ) 
+   (
+   # Hack the p4 depot to make the symlink point to nothing;
+   # this should not happen in reality, but shows up
+   # in p4 repos in the wild.
+   #
+   # The sed expression changes this:
+   # @@
+   # text
+   # @target1
+   # @
+   # to this:
+   # @@
+   # text
+   # @@
+   #
+   cd $db/depot 
+   sed /@target1/{; s/target1/@/; n; d; } \
+   empty-symlink,v empty-symlink,v.tmp 
+   mv empty-symlink,v.tmp empty-symlink,v
+   ) 
+   (
+   # Make sure symlink really is empty.  Asking
+   # p4 to sync here will make it generate errors.
+   cd $cli 
+   p4 print -q //depot/empty-symlink#2 out 
+   test ! -s out
+   ) 
+   test_when_finished cleanup_git 
+
+   # make sure git p4 handles it without error
+   git p4 clone --dest=$git //depot@all 
+
+   # fix the symlink, make it point to target2
+   (
+   cd $cli 
+   p4 open empty-symlink 
+   rm empty-symlink 
+   ln -s target2 empty-symlink 
+   p4 submit -d make empty-symlink point to target2
+   ) 
+   cleanup_git 
+   git p4 clone --dest=$git //depot@all 
+   (
+   cd $git 
+   test $(readlink empty-symlink) = target2
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.5.2.364.g6ac45cd

--
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

[PATCHv2 05/11] git p4 test: is_cli_file_writeable succeeds

[Pete Wyckoff]

Commit e9df0f9 (git p4: cygwin p4 client does not mark read-only,
2013-01-26) fixed a problem with test -w on cygwin, but mistakenly
marked the new test as failing.  Fix this.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9807-git-p4-submit.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 1fb7bc7..4caf36e 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -17,7 +17,7 @@ test_expect_success 'init depot' '
)
 '
 
-test_expect_failure 'is_cli_file_writeable function' '
+test_expect_success 'is_cli_file_writeable function' '
(
cd $cli 
echo a a 
-- 
1.8.5.2.364.g6ac45cd

--
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 06/11] git p4 test: run as user author

[Pete Wyckoff]

The tests use aut...@example.com as the canonical submitter, but
he does not have an entry in the p4 users database.  This causes
the generated change description to complain that the git and p4
users disagree.  The complaint message is still valid, just isn't
useful in tests.  It was introduced in 848de9c (git-p4: warn if
git authorship won't be retained, 2011-05-13).

Fix t9813 to use @example.com instead of @localhost due to change
in p4_add_user().  Move the function into the git p4 test library
so author can be added at initialization time.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/lib-git-p4.sh  | 15 ++-
 t/t9813-git-p4-preserve-users.sh | 38 ++
 2 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index ccd918e..4ff2bb1 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -47,9 +47,10 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
 
 P4PORT=localhost:$P4DPORT
 P4CLIENT=client
+P4USER=author
 P4EDITOR=:
 unset P4CHARSET
-export P4PORT P4CLIENT P4EDITOR P4CHARSET
+export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET
 
 db=$TRASH_DIRECTORY/db
 cli=$TRASH_DIRECTORY/cli
@@ -96,12 +97,24 @@ start_p4d() {
return 1
fi
 
+   # build a p4 user so aut...@example.com has an entry
+   p4_add_user author
+
# build a client
client_view //depot/... //client/... 
 
return 0
 }
 
+p4_add_user() {
+   name=$1 
+   p4 user -f -i -EOF
+   User: $name
+   Email: $n...@example.com
+   FullName: Dr. $name
+   EOF
+}
+
 kill_p4d() {
pid=$(cat $pidfile)
# it had better exist for the first kill
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index f2e85e5..166b840 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -19,16 +19,6 @@ test_expect_success 'create files' '
)
 '
 
-p4_add_user() {
-   name=$1 fullname=$2 
-   p4 user -f -i -EOF 
-   User: $name
-   Email: $name@localhost
-   FullName: $fullname
-   EOF
-   p4 passwd -P secret $name
-}
-
 p4_grant_admin() {
name=$1 
{
@@ -51,8 +41,8 @@ make_change_by_user() {
 
 # Test username support, submitting as user 'alice'
 test_expect_success 'preserve users' '
-   p4_add_user alice Alice 
-   p4_add_user bob Bob 
+   p4_add_user alice 
+   p4_add_user bob 
p4_grant_admin alice 
git p4 clone --dest=$git //depot 
test_when_finished cleanup_git 
@@ -60,8 +50,8 @@ test_expect_success 'preserve users' '
cd $git 
echo username: a change by alice file1 
echo username: a change by bob file2 
-   git commit --author Alice alice@localhost -m a change by 
alice file1 
-   git commit --author Bob bob@localhost -m a change by bob 
file2 
+   git commit --author Alice al...@example.com -m a change by 
alice file1 
+   git commit --author Bob b...@example.com -m a change by 
bob file2 
git config git-p4.skipSubmitEditCheck true 
P4EDITOR=touch P4USER=alice P4PASSWD=secret git p4 commit 
--preserve-user 
p4_check_commit_author file1 alice 
@@ -78,7 +68,7 @@ test_expect_success 'refuse to preserve users without perms' '
cd $git 
git config git-p4.skipSubmitEditCheck true 
echo username-noperms: a change by alice file1 
-   git commit --author Alice alice@localhost -m perms: a 
change by alice file1 
+   git commit --author Alice al...@example.com -m perms: a 
change by alice file1 
P4EDITOR=touch P4USER=bob P4PASSWD=secret 
export P4EDITOR P4USER P4PASSWD 
test_must_fail git p4 commit --preserve-user 
@@ -94,9 +84,9 @@ test_expect_success 'preserve user where author is unknown to 
p4' '
cd $git 
git config git-p4.skipSubmitEditCheck true 
echo username-bob: a change by bob file1 
-   git commit --author Bob bob@localhost -m preserve: a 
change by bob file1 
+   git commit --author Bob b...@example.com -m preserve: a 
change by bob file1 
echo username-unknown: a change by charlie file1 
-   git commit --author Charlie charlie@localhost -m preserve: 
a change by charlie file1 
+   git commit --author Charlie char...@example.com -m 
preserve: a change by charlie file1 
P4EDITOR=touch P4USER=alice P4PASSWD=secret 
export P4EDITOR P4USER P4PASSWD 
test_must_fail git p4 commit --preserve-user 
@@ -121,24 +111,24 @@ test_expect_success 'not preserving user with mixed 
authorship' '
(
cd $git 
git config git-p4.skipSubmitEditCheck true 
-   p4_add_user derek

[PATCHv2 08/11] git p4: handle files with wildcards when doing RCS scrubbing

[Pete Wyckoff]

Commit 9d7d446 (git p4: submit files with wildcards, 2012-04-29)
fixed problems with handling files that had p4 wildcard
characters, like @ and *.  But it missed one case, that of
RCS keyword scrubbing, which uses p4 fstat to extract type
information.  Fix it by calling wildcard_encode() on the raw
filename.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py   |  4 ++--
 t/t9812-git-p4-wildcards.sh | 23 +++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index f0a327d..39a0fa0 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -310,8 +310,8 @@ def split_p4_type(p4type):
 #
 # return the raw p4 type of a file (text, text+ko, etc)
 #
-def p4_type(file):
-results = p4CmdList([fstat, -T, headType, file])
+def p4_type(f):
+results = p4CmdList([fstat, -T, headType, wildcard_encode(f)])
 return results[0]['headType']
 
 #
diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh
index f2ddbc5..c7472cb 100755
--- a/t/t9812-git-p4-wildcards.sh
+++ b/t/t9812-git-p4-wildcards.sh
@@ -188,6 +188,29 @@ test_expect_success 'p4 deleted a wildcard file' '
)
 '
 
+test_expect_success 'wildcard files requiring keyword scrub' '
+   (
+   cd $cli 
+   cat -\EOF scrub@wild 
+   $Id$
+   line2
+   EOF
+   p4 add -t text+k -f scrub@wild 
+   p4 submit -d scrub at wild
+   ) 
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   git config git-p4.skipSubmitEdit true 
+   git config git-p4.attemptRCSCleanup true 
+   sed s/^line2/line2 edit/ scrub@wild sc...@wild.tmp 
+   mv -f sc...@wild.tmp scrub@wild 
+   git commit -m scrub at wild line2 edit scrub@wild 
+   git p4 submit
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.5.2.364.g6ac45cd

--
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 07/11] git p4 test: do not pollute /tmp

[Pete Wyckoff]

Generating the submit template for p4 uses tempfile.mkstemp(),
which by default puts files in /tmp.  For a test that fails,
possibly on purpose, this is not cleaned up.  Run with TMPDIR
pointing into the trash directory so the temp files go away
with the test results.

To do this required some other minor changes.  First, the editor
is launched using system(editor +   + template_file), using
shell expansion to build the command string.  This doesn't work
if editor has a space in it.  And is generally unwise as it's
easy to fool the shell into doing extra work.  Exec the args
directly, without shell expansion.

Second, without shell expansion, the trick of P4EDITOR=: used
in the tests doesn't work.  Use a real command, true, as the
non-interactive editor for testing.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py  | 2 +-
 t/lib-git-p4.sh| 8 +++-
 t/t9805-git-p4-skip-submit-edit.sh | 6 --
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 3a20d15..f0a327d 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(editor +   + template_file)
+system([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/lib-git-p4.sh b/t/lib-git-p4.sh
index 4ff2bb1..5aa8adc 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -48,7 +48,7 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
 P4PORT=localhost:$P4DPORT
 P4CLIENT=client
 P4USER=author
-P4EDITOR=:
+P4EDITOR=true
 unset P4CHARSET
 export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET
 
@@ -57,6 +57,12 @@ cli=$TRASH_DIRECTORY/cli
 git=$TRASH_DIRECTORY/git
 pidfile=$TRASH_DIRECTORY/p4d.pid
 
+# git p4 submit generates a temp file, which will
+# not get cleaned up if the submission fails.  Don't
+# clutter up /tmp on the test machine.
+TMPDIR=$TRASH_DIRECTORY
+export TMPDIR
+
 start_p4d() {
mkdir -p $db $cli $git 
rm -f $pidfile 
diff --git a/t/t9805-git-p4-skip-submit-edit.sh 
b/t/t9805-git-p4-skip-submit-edit.sh
index ff2cc79..8931188 100755
--- a/t/t9805-git-p4-skip-submit-edit.sh
+++ b/t/t9805-git-p4-skip-submit-edit.sh
@@ -17,7 +17,7 @@ test_expect_success 'init depot' '
)
 '
 
-# this works because EDITOR is set to :
+# this works because P4EDITOR is set to true
 test_expect_success 'no config, unedited, say yes' '
git p4 clone --dest=$git //depot 
test_when_finished cleanup_git 
@@ -90,7 +90,9 @@ test_expect_success 'no config, edited' '
cd $git 
echo line file1 
git commit -a -m change 5 
-   P4EDITOR= EDITOR=\$TRASH_DIRECTORY/ed.sh\ git p4 submit 
+   P4EDITOR=$TRASH_DIRECTORY/ed.sh 
+   export P4EDITOR 
+   git p4 submit 
p4 changes //depot/... wc 
test_line_count = 5 wc
)
-- 
1.8.5.2.364.g6ac45cd

--
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 10/11] git p4 test: examine behavior with locked (+l) files

[Pete Wyckoff]

The p4 server can enforce file locking, so that only one user
can edit a file at a time.  Git p4 is unable to submit changes
to locked files.  Currently it exits poorly.  Ideally it would
notice the locked condition and clean up nicely.

Add a bunch of tests that describe the problem, hoping that
fixes appear in the future.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9816-git-p4-locked.sh | 145 +++
 1 file changed, 145 insertions(+)
 create mode 100755 t/t9816-git-p4-locked.sh

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
new file mode 100755
index 000..e71e543
--- /dev/null
+++ b/t/t9816-git-p4-locked.sh
@@ -0,0 +1,145 @@
+#!/bin/sh
+
+test_description='git p4 locked file behavior'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+# See
+# 
http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.html#1088563
+# for suggestions on how to configure sitewide pessimistic locking
+# where only one person can have a file open for edit at a time.
+test_expect_success 'init depot' '
+   (
+   cd $cli 
+   echo TypeMap: +l //depot/... | p4 typemap -i 
+   echo file1 file1 
+   p4 add file1 
+   p4 submit -d add file1
+   )
+'
+
+test_expect_success 'edit with lock not taken' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line2 file1 
+   git add file1 
+   git commit -m line2 in file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit
+   )
+'
+
+test_expect_failure 'add with lock not taken' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line1 add-lock-not-taken 
+   git add file2 
+   git commit -m add add-lock-not-taken 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+lock_in_another_client() {
+   # build a different client
+   cli2=$TRASH_DIRECTORY/cli2 
+   mkdir -p $cli2 
+   test_when_finished p4 client -f -d client2  rm -rf \$cli2\ 
+   (
+   cd $cli2 
+   P4CLIENT=client2 
+   cli=$cli2 
+   client_view //depot/... //client2/... 
+   p4 sync 
+   p4 open file1
+   )
+}
+
+test_expect_failure 'edit with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line3 file1 
+   git add file1 
+   git commit -m line3 in file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'delete with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   git rm file1 
+   git commit -m delete file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'chmod with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   chmod +x file1 
+   git add file1 
+   git commit -m chmod +x file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'copy with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 revert file2  rm -f file2 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   cp file1 file2 
+   git add file2 
+   git commit -m cp file1 to file2 
+   git config git-p4.skipSubmitEdit true 
+   git config git-p4.detectCopies true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'move with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync file1  rm -f file2 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   git mv file1 file2 
+   git commit -m mv file1 to file2 
+   git config git-p4.skipSubmitEdit true 
+   git config git-p4.detectRenames true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_success 'kill p4d

[PATCHv2 09/11] git p4: fix an error message when p4 where fails

[Pete Wyckoff]

When p4 where fails, for whatever reason, the error message tries to
show an undefined variable.  This minor bug applies only when using a
client spec, and was introduced recently in 9d57c4a (git p4: implement
view spec wildcards with p4 where, 2013-08-30).

Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 39a0fa0..db43629 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1871,7 +1871,7 @@ class View(object):
 # assume error is ... file(s) not in client view
 continue
 if clientFile not in res:
-die(No clientFile from 'p4 where %s' % depot_path)
+die(No clientFile in 'p4 where' output)
 if unmap in res:
 # it will list all of them, but only one not unmap-ped
 continue
-- 
1.8.5.2.364.g6ac45cd

--
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 11/11] git p4 doc: use two-line style for options with multiple spellings

[Pete Wyckoff]

Thomas Rast noticed the docs have a mix of styles when
it comes to options with multiple spellings.  Standardize
the couple in git-p4.txt that are odd.

Instead of:
  -n, --dry-run::

Do this:
  -n::
  --dry-run::

See
http://thread.gmane.org/gmane.comp.version-control.git/219936/focus=219945

Signed-off-by: Pete Wyckoff p...@padd.com
---
 Documentation/git-p4.txt | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 8cba16d..6ab5f94 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -168,7 +168,8 @@ All commands except clone accept these options.
 --git-dir dir::
Set the 'GIT_DIR' environment variable.  See linkgit:git[1].
 
---verbose, -v::
+-v::
+--verbose::
Provide more progress information.
 
 Sync options
@@ -279,7 +280,8 @@ These options can be used to modify 'git p4 submit' 
behavior.
Export tags from Git as p4 labels. Tags found in Git are applied
to the perforce working directory.
 
---dry-run, -n::
+-n::
+--dry-run::
Show just what commits would be submitted to p4; do not change
state in Git or p4.
 
-- 
1.8.5.2.364.g6ac45cd

--
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 01/11] git p4 test: wildcards are supported

[Pete Wyckoff]

Since 9d57c4a (git p4: implement view spec wildcards with p4
where, 2013-08-30), all the wildcard types should be supported.
Change must-fail tests to mark that they now pass.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9809-git-p4-client-view.sh | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 77f6349..23a827f 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -76,28 +76,28 @@ test_expect_success 'init depot' '
 '
 
 # double % for printf
-test_expect_success 'unsupported view wildcard %%n' '
+test_expect_success 'view wildcard %%n' '
client_view //depot/1/sub/... //client/sub/1/... 
test_when_finished cleanup_git 
-   test_must_fail git p4 clone --use-client-spec --dest=$git //depot
+   git p4 clone --use-client-spec --dest=$git //depot
 '
 
-test_expect_success 'unsupported view wildcard *' '
+test_expect_success 'view wildcard *' '
client_view //depot/*/bar/... //client/*/bar/... 
test_when_finished cleanup_git 
-   test_must_fail git p4 clone --use-client-spec --dest=$git //depot
+   git p4 clone --use-client-spec --dest=$git //depot
 '
 
-test_expect_success 'wildcard ... only supported at end of spec 1' '
+test_expect_success 'wildcard ... in the middle' '
client_view //depot/.../file11 //client/.../file11 
test_when_finished cleanup_git 
-   test_must_fail git p4 clone --use-client-spec --dest=$git //depot
+   git p4 clone --use-client-spec --dest=$git //depot
 '
 
-test_expect_success 'wildcard ... only supported at end of spec 2' '
+test_expect_success 'wildcard ... in the middle and at the end' '
client_view //depot/.../a/... //client/.../a/... 
test_when_finished cleanup_git 
-   test_must_fail git p4 clone --use-client-spec --dest=$git //depot
+   git p4 clone --use-client-spec --dest=$git //depot
 '
 
 test_expect_success 'basic map' '
-- 
1.8.5.2.320.g99957e5

--
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 00/11] git p4 tests and a few bug fixes

[Pete Wyckoff]

Most of this is work on tests for git p4.

Patch 03 is a regression fix, found and narrowed down thanks to
much work by Damien Gérard.  But it is obscure enough that I'm
not proposing it for a maintenance release.

There are a couple other behavior fixes, but again, these
are quite minor and can wait for the next release.

Pete Wyckoff (11):
  git p4 test: wildcards are supported
  git p4 test: ensure p4 symlink parsing works
  git p4: work around p4 bug that causes empty symlinks
  git p4 test: explicitly check p4 wildcard delete
  git p4 test: is_cli_file_writeable succeeds
  git p4 test: run as user author
  git p4 test: do not pollute /tmp
  git p4: handle files with wildcards when doing RCS scrubbing
  git p4: fix an error message when p4 where fails
  git p4 test: examine behavior with locked (+l) files
  git p4 doc: use two-line style for options with multiple spellings

 Documentation/git-p4.txt   |   6 +-
 git-p4.py  |  17 +++--
 t/lib-git-p4.sh|  23 +-
 t/t9802-git-p4-filetype.sh |  83 +
 t/t9805-git-p4-skip-submit-edit.sh |   6 +-
 t/t9807-git-p4-submit.sh   |   2 +-
 t/t9809-git-p4-client-view.sh  |  16 ++--
 t/t9812-git-p4-wildcards.sh|  50 +
 t/t9813-git-p4-preserve-users.sh   |  38 --
 t/t9816-git-p4-locked.sh   | 145 +
 10 files changed, 342 insertions(+), 44 deletions(-)
 create mode 100755 t/t9816-git-p4-locked.sh

-- 
1.8.5.2.320.g99957e5

--
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 02/11] git p4 test: ensure p4 symlink parsing works

[Pete Wyckoff]

While this happens to work, there was no test to make sure
that the basic importing of a symlink from p4 to git functioned.

Add a simple test to create a symlink in p4 and import it into git,
then verify that the symlink exists and has the correct target.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9802-git-p4-filetype.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index a82744b..94d7be9 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -250,6 +250,23 @@ test_expect_success 'ignore apple' '
)
 '
 
+test_expect_success SYMLINKS 'create p4 symlink' '
+   cd $cli 
+   ln -s symlink-target symlink 
+   p4 add symlink 
+   p4 submit -d add symlink
+'
+
+test_expect_success SYMLINKS 'ensure p4 symlink parsed correctly' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot@all 
+   (
+   cd $git 
+   test -L symlink 
+   test $(readlink symlink) = symlink-target
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.5.2.320.g99957e5

--
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 04/11] git p4 test: explicitly check p4 wildcard delete

[Pete Wyckoff]

There was no test where p4 deleted a file with a wildcard
character.  Make sure git p4 applies the wildcard decoding
properly when importing a delete that includes a wildcard.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9812-git-p4-wildcards.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh
index 6763325..f2ddbc5 100755
--- a/t/t9812-git-p4-wildcards.sh
+++ b/t/t9812-git-p4-wildcards.sh
@@ -161,6 +161,33 @@ test_expect_success 'wildcard files submit back to p4, 
delete' '
)
 '
 
+test_expect_success 'p4 deleted a wildcard file' '
+   (
+   cd $cli 
+   echo wild delete test wild@delete 
+   p4 add -f wild@delete 
+   p4 submit -d add wild@delete
+   ) 
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   test_path_is_file wild@delete
+   ) 
+   (
+   cd $cli 
+   # must use its encoded name
+   p4 delete wild%40delete 
+   p4 submit -d delete wild@delete
+   ) 
+   (
+   cd $git 
+   git p4 sync 
+   git merge --ff-only p4/master 
+   test_path_is_missing wild@delete
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.5.2.320.g99957e5

--
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 03/11] git p4: work around p4 bug that causes empty symlinks

[Pete Wyckoff]

Damien Gérard highlights an interesting problem.  Some p4
repositories end up with symlinks that have an empty target.  It
is not possible to create this with current p4, but they do
indeed exist.

The effect in git p4 is that p4 print on the symlink returns an
empty string, confusing the curret symlink-handling code.

Such broken repositories cause problems in p4 as well, even with
no git involved.  In p4, syncing to a change that includes a
bogus symlink causes errors:

//depot/empty-symlink - updating /home/me/p4/empty-symlink
rename: /home/me/p4/empty-symlink: No such file or directory

and leaves no symlink.

In git, replicate the p4 behavior by ignoring these bad symlinks.
If, in a later p4 revision, the symlink happens to point to
something non-null, the symlink will be replaced properly.

Add a big test for all this too.

This happens to be a regression introduced by 1292df1 (git-p4:
Fix occasional truncation of symlink contents., 2013-08-08) and
appeared first in 1.8.5.  But it only shows up only in p4
repositories of dubious character, so can wait for a proper
release.

Tested-by: Damien Gérard dam...@iwi.me
Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py  |  9 ++-
 t/t9802-git-p4-filetype.sh | 66 ++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 5ea8bb8..e798ecf 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2075,7 +2075,14 @@ class P4Sync(Command, P4UserMap):
 # p4 print on a symlink sometimes contains target\n;
 # if it does, remove the newline
 data = ''.join(contents)
-if data[-1] == '\n':
+if not data:
+# Some version of p4 allowed creating a symlink that pointed
+# to nothing.  This causes p4 errors when checking out such
+# a change, and errors here too.  Work around it by ignoring
+# the bad symlink; hopefully a future change fixes it.
+print \nIgnoring empty symlink in %s % file['depotFile']
+return
+elif data[-1] == '\n':
 contents = [data[:-1]]
 else:
 contents = [data]
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 94d7be9..66d3fc9 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -267,6 +267,72 @@ test_expect_success SYMLINKS 'ensure p4 symlink parsed 
correctly' '
)
 '
 
+test_expect_success SYMLINKS 'empty symlink target' '
+   (
+   # first create the file as a file
+   cd $cli 
+   empty-symlink 
+   p4 add empty-symlink 
+   p4 submit -d add empty-symlink as a file
+   ) 
+   (
+   # now change it to be a symlink to target1
+   cd $cli 
+   p4 edit empty-symlink 
+   p4 reopen -t symlink empty-symlink 
+   rm empty-symlink 
+   ln -s target1 empty-symlink 
+   p4 add empty-symlink 
+   p4 submit -d make empty-symlink point to target1
+   ) 
+   (
+   # Hack the p4 depot to make the symlink point to nothing;
+   # this should not happen in reality, but shows up
+   # in p4 repos in the wild.
+   #
+   # The sed expression changes this:
+   # @@
+   # text
+   # @target1
+   # @
+   # to this:
+   # @@
+   # text
+   # @@
+   #
+   cd $db/depot 
+   sed /@target1/{; s/target1/@/; n; d; } \
+   empty-symlink,v empty-symlink,v.tmp 
+   mv empty-symlink,v.tmp empty-symlink,v
+   ) 
+   (
+   # Make sure symlink really is empty.  Asking
+   # p4 to sync here will make it generate errors.
+   cd $cli 
+   p4 print -q //depot/empty-symlink#2 out 
+   test ! -s out
+   ) 
+   test_when_finished cleanup_git 
+
+   # make sure git p4 handles it without error
+   git p4 clone --dest=$git //depot@all 
+
+   # fix the symlink, make it point to target2
+   (
+   cd $cli 
+   p4 open empty-symlink 
+   rm empty-symlink 
+   ln -s target2 empty-symlink 
+   p4 submit -d make empty-symlink point to target2
+   ) 
+   cleanup_git 
+   git p4 clone --dest=$git //depot@all 
+   (
+   cd $git 
+   test $(readlink empty-symlink) = target2
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.5.2.320.g99957e5

--
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

[PATCH 06/11] git p4 test: run as user author

[Pete Wyckoff]

The tests use aut...@example.com as the canonical submitter,
but he does not have an entry in the p4 users database.
This causes the generated change description to complain
that the git and p4 users disagree.  The complaint message
is still valid, just isn't useful in tests.  It was was
introduced in 848de9c (git-p4: warn if git authorship won't
be retained, 2011-05-13).

Fix t9813 to use @example.com instead of @localhost due to
change in p4_add_user().  Move the function into the git p4
test library so author can be added at initialization time.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/lib-git-p4.sh  | 15 ++-
 t/t9813-git-p4-preserve-users.sh | 38 ++
 2 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index ccd918e..4ff2bb1 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -47,9 +47,10 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
 
 P4PORT=localhost:$P4DPORT
 P4CLIENT=client
+P4USER=author
 P4EDITOR=:
 unset P4CHARSET
-export P4PORT P4CLIENT P4EDITOR P4CHARSET
+export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET
 
 db=$TRASH_DIRECTORY/db
 cli=$TRASH_DIRECTORY/cli
@@ -96,12 +97,24 @@ start_p4d() {
return 1
fi
 
+   # build a p4 user so aut...@example.com has an entry
+   p4_add_user author
+
# build a client
client_view //depot/... //client/... 
 
return 0
 }
 
+p4_add_user() {
+   name=$1 
+   p4 user -f -i -EOF
+   User: $name
+   Email: $n...@example.com
+   FullName: Dr. $name
+   EOF
+}
+
 kill_p4d() {
pid=$(cat $pidfile)
# it had better exist for the first kill
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index f2e85e5..166b840 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -19,16 +19,6 @@ test_expect_success 'create files' '
)
 '
 
-p4_add_user() {
-   name=$1 fullname=$2 
-   p4 user -f -i -EOF 
-   User: $name
-   Email: $name@localhost
-   FullName: $fullname
-   EOF
-   p4 passwd -P secret $name
-}
-
 p4_grant_admin() {
name=$1 
{
@@ -51,8 +41,8 @@ make_change_by_user() {
 
 # Test username support, submitting as user 'alice'
 test_expect_success 'preserve users' '
-   p4_add_user alice Alice 
-   p4_add_user bob Bob 
+   p4_add_user alice 
+   p4_add_user bob 
p4_grant_admin alice 
git p4 clone --dest=$git //depot 
test_when_finished cleanup_git 
@@ -60,8 +50,8 @@ test_expect_success 'preserve users' '
cd $git 
echo username: a change by alice file1 
echo username: a change by bob file2 
-   git commit --author Alice alice@localhost -m a change by 
alice file1 
-   git commit --author Bob bob@localhost -m a change by bob 
file2 
+   git commit --author Alice al...@example.com -m a change by 
alice file1 
+   git commit --author Bob b...@example.com -m a change by 
bob file2 
git config git-p4.skipSubmitEditCheck true 
P4EDITOR=touch P4USER=alice P4PASSWD=secret git p4 commit 
--preserve-user 
p4_check_commit_author file1 alice 
@@ -78,7 +68,7 @@ test_expect_success 'refuse to preserve users without perms' '
cd $git 
git config git-p4.skipSubmitEditCheck true 
echo username-noperms: a change by alice file1 
-   git commit --author Alice alice@localhost -m perms: a 
change by alice file1 
+   git commit --author Alice al...@example.com -m perms: a 
change by alice file1 
P4EDITOR=touch P4USER=bob P4PASSWD=secret 
export P4EDITOR P4USER P4PASSWD 
test_must_fail git p4 commit --preserve-user 
@@ -94,9 +84,9 @@ test_expect_success 'preserve user where author is unknown to 
p4' '
cd $git 
git config git-p4.skipSubmitEditCheck true 
echo username-bob: a change by bob file1 
-   git commit --author Bob bob@localhost -m preserve: a 
change by bob file1 
+   git commit --author Bob b...@example.com -m preserve: a 
change by bob file1 
echo username-unknown: a change by charlie file1 
-   git commit --author Charlie charlie@localhost -m preserve: 
a change by charlie file1 
+   git commit --author Charlie char...@example.com -m 
preserve: a change by charlie file1 
P4EDITOR=touch P4USER=alice P4PASSWD=secret 
export P4EDITOR P4USER P4PASSWD 
test_must_fail git p4 commit --preserve-user 
@@ -121,24 +111,24 @@ test_expect_success 'not preserving user with mixed 
authorship' '
(
cd $git 
git config git-p4.skipSubmitEditCheck true 
-   p4_add_user

[PATCH 05/11] git p4 test: is_cli_file_writeable succeeds

[Pete Wyckoff]

Commit e9df0f9 (git p4: cygwin p4 client does not mark read-only,
2013-01-26) fixed a problem with test -w on cygwin, but mistakenly
marked the new test as failing.  Fix this.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9807-git-p4-submit.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 1fb7bc7..4caf36e 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -17,7 +17,7 @@ test_expect_success 'init depot' '
)
 '
 
-test_expect_failure 'is_cli_file_writeable function' '
+test_expect_success 'is_cli_file_writeable function' '
(
cd $cli 
echo a a 
-- 
1.8.5.2.320.g99957e5

--
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 07/11] git p4 test: do not pollute /tmp

[Pete Wyckoff]

Generating the submit template for p4 uses tempfile.mkstemp(),
which by default puts files in /tmp.  For a test that fails,
possibly on purpose, this is not cleaned up.  Run with TMPDIR
pointing into the trash directory so the temp files go away
with the test results.

To do this required some other minor changes.  First, the editor
is launched using system(editor +   + template_file), using
shell expansion to build the command string.  This doesn't work
if editor has a space in it.  And is generally unwise as it's
easy to fool the shell into doing extra work.  Exec the args
directly, without shell expansion.

Second, without shell expansion, the trick of P4EDITOR=: used
in the tests doesn't work.  Use a real command, true, as the
non-interactive editor for testing.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py  | 2 +-
 t/lib-git-p4.sh| 8 +++-
 t/t9805-git-p4-skip-submit-edit.sh | 6 --
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index e798ecf..a4414b5 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(editor +   + template_file)
+system([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/lib-git-p4.sh b/t/lib-git-p4.sh
index 4ff2bb1..5aa8adc 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -48,7 +48,7 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
 P4PORT=localhost:$P4DPORT
 P4CLIENT=client
 P4USER=author
-P4EDITOR=:
+P4EDITOR=true
 unset P4CHARSET
 export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET
 
@@ -57,6 +57,12 @@ cli=$TRASH_DIRECTORY/cli
 git=$TRASH_DIRECTORY/git
 pidfile=$TRASH_DIRECTORY/p4d.pid
 
+# git p4 submit generates a temp file, which will
+# not get cleaned up if the submission fails.  Don't
+# clutter up /tmp on the test machine.
+TMPDIR=$TRASH_DIRECTORY
+export TMPDIR
+
 start_p4d() {
mkdir -p $db $cli $git 
rm -f $pidfile 
diff --git a/t/t9805-git-p4-skip-submit-edit.sh 
b/t/t9805-git-p4-skip-submit-edit.sh
index ff2cc79..8931188 100755
--- a/t/t9805-git-p4-skip-submit-edit.sh
+++ b/t/t9805-git-p4-skip-submit-edit.sh
@@ -17,7 +17,7 @@ test_expect_success 'init depot' '
)
 '
 
-# this works because EDITOR is set to :
+# this works because P4EDITOR is set to true
 test_expect_success 'no config, unedited, say yes' '
git p4 clone --dest=$git //depot 
test_when_finished cleanup_git 
@@ -90,7 +90,9 @@ test_expect_success 'no config, edited' '
cd $git 
echo line file1 
git commit -a -m change 5 
-   P4EDITOR= EDITOR=\$TRASH_DIRECTORY/ed.sh\ git p4 submit 
+   P4EDITOR=$TRASH_DIRECTORY/ed.sh 
+   export P4EDITOR 
+   git p4 submit 
p4 changes //depot/... wc 
test_line_count = 5 wc
)
-- 
1.8.5.2.320.g99957e5

--
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 08/11] git p4: handle files with wildcards when doing RCS scrubbing

[Pete Wyckoff]

Commit 9d7d446 (git p4: submit files with wildcards, 2012-04-29)
fixed problems with handling files that had p4 wildcard
characters, like @ and *.  But it missed one case, that of
RCS keyword scrubbing, which uses p4 fstat to extract type
information.  Fix it by calling wildcard_encode() on the raw
filename.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py   |  4 ++--
 t/t9812-git-p4-wildcards.sh | 23 +++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index a4414b5..26b874f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -310,8 +310,8 @@ def split_p4_type(p4type):
 #
 # return the raw p4 type of a file (text, text+ko, etc)
 #
-def p4_type(file):
-results = p4CmdList([fstat, -T, headType, file])
+def p4_type(f):
+results = p4CmdList([fstat, -T, headType, wildcard_encode(f)])
 return results[0]['headType']
 
 #
diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh
index f2ddbc5..c7472cb 100755
--- a/t/t9812-git-p4-wildcards.sh
+++ b/t/t9812-git-p4-wildcards.sh
@@ -188,6 +188,29 @@ test_expect_success 'p4 deleted a wildcard file' '
)
 '
 
+test_expect_success 'wildcard files requiring keyword scrub' '
+   (
+   cd $cli 
+   cat -\EOF scrub@wild 
+   $Id$
+   line2
+   EOF
+   p4 add -t text+k -f scrub@wild 
+   p4 submit -d scrub at wild
+   ) 
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   git config git-p4.skipSubmitEdit true 
+   git config git-p4.attemptRCSCleanup true 
+   sed s/^line2/line2 edit/ scrub@wild sc...@wild.tmp 
+   mv -f sc...@wild.tmp scrub@wild 
+   git commit -m scrub at wild line2 edit scrub@wild 
+   git p4 submit
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.5.2.320.g99957e5

--
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 09/11] git p4: fix an error message when p4 where fails

[Pete Wyckoff]

When p4 where fails, for whatever reason, the error message tries to
show an undefined variable.  This minor bug applies only when using a
client spec, and was introduced recently in 9d57c4a (git p4: implement
view spec wildcards with p4 where, 2013-08-30).

Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 26b874f..cdfa2df 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1871,7 +1871,7 @@ class View(object):
 # assume error is ... file(s) not in client view
 continue
 if clientFile not in res:
-die(No clientFile from 'p4 where %s' % depot_path)
+die(No clientFile in 'p4 where' output)
 if unmap in res:
 # it will list all of them, but only one not unmap-ped
 continue
-- 
1.8.5.2.320.g99957e5

--
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 10/11] git p4 test: examine behavior with locked (+l) files

[Pete Wyckoff]

The p4 server can enforce file locking, so that only one user
can edit a file at a time.  Git p4 is unable to submit changes
to locked files.  Currently it exits poorly.  Ideally it would
notice the locked condition and clean up nicely.

Add a bunch of tests that describe the problem, hoping that
fixes appear in the future.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9816-git-p4-locked.sh | 145 +++
 1 file changed, 145 insertions(+)
 create mode 100755 t/t9816-git-p4-locked.sh

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
new file mode 100755
index 000..e71e543
--- /dev/null
+++ b/t/t9816-git-p4-locked.sh
@@ -0,0 +1,145 @@
+#!/bin/sh
+
+test_description='git p4 locked file behavior'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+# See
+# 
http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.html#1088563
+# for suggestions on how to configure sitewide pessimistic locking
+# where only one person can have a file open for edit at a time.
+test_expect_success 'init depot' '
+   (
+   cd $cli 
+   echo TypeMap: +l //depot/... | p4 typemap -i 
+   echo file1 file1 
+   p4 add file1 
+   p4 submit -d add file1
+   )
+'
+
+test_expect_success 'edit with lock not taken' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line2 file1 
+   git add file1 
+   git commit -m line2 in file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit
+   )
+'
+
+test_expect_failure 'add with lock not taken' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line1 add-lock-not-taken 
+   git add file2 
+   git commit -m add add-lock-not-taken 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+lock_in_another_client() {
+   # build a different client
+   cli2=$TRASH_DIRECTORY/cli2 
+   mkdir -p $cli2 
+   test_when_finished p4 client -f -d client2  rm -rf \$cli2\ 
+   (
+   cd $cli2 
+   P4CLIENT=client2 
+   cli=$cli2 
+   client_view //depot/... //client2/... 
+   p4 sync 
+   p4 open file1
+   )
+}
+
+test_expect_failure 'edit with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line3 file1 
+   git add file1 
+   git commit -m line3 in file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'delete with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   git rm file1 
+   git commit -m delete file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'chmod with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   chmod +x file1 
+   git add file1 
+   git commit -m chmod +x file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'copy with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 revert file2  rm -f file2 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   cp file1 file2 
+   git add file2 
+   git commit -m cp file1 to file2 
+   git config git-p4.skipSubmitEdit true 
+   git config git-p4.detectCopies true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'move with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync file1  rm -f file2 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   git mv file1 file2 
+   git commit -m mv file1 to file2 
+   git config git-p4.skipSubmitEdit true 
+   git config git-p4.detectRenames true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_success 'kill p4d

[PATCH 11/11] git p4 doc: use two-line style for options with multiple spellings

[Pete Wyckoff]

Thomas Rast noticed the docs have a mix of styles when
it comes to options with multiple spellings.  Standardize
the couple in git-p4.txt that are odd.

Instead of:
  -n, --dry-run::

Do this:
  -n::
  --dry-run::

See
http://thread.gmane.org/gmane.comp.version-control.git/219936/focus=219945

Signed-off-by: Pete Wyckoff p...@padd.com
---
 Documentation/git-p4.txt | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 8cba16d..6ab5f94 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -168,7 +168,8 @@ All commands except clone accept these options.
 --git-dir dir::
Set the 'GIT_DIR' environment variable.  See linkgit:git[1].
 
---verbose, -v::
+-v::
+--verbose::
Provide more progress information.
 
 Sync options
@@ -279,7 +280,8 @@ These options can be used to modify 'git p4 submit' 
behavior.
Export tags from Git as p4 labels. Tags found in Git are applied
to the perforce working directory.
 
---dry-run, -n::
+-n::
+--dry-run::
Show just what commits would be submitted to p4; do not change
state in Git or p4.
 
-- 
1.8.5.2.320.g99957e5

--
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: exception when cloning a perforce repository

[Pete Wyckoff]

dam...@iwi.me wrote on Thu, 16 Jan 2014 17:02 +0100:
 
 On 16 Jan 2014, at 15:45, Pete Wyckoff p...@padd.com wrote:
 
  Oh cool, that helps a lot.  P4 is just broken here, so we can get
  away with being a bit sloppy in git.  I'll try just pretending
  empty symlinks are not in the repo.  Hopefully you'll have a
  future commit in your p4 repo that brings back bn.h properly.
 
 Thanks !
 I would love to use git instead of perforce if possible :)
 
  Still not sure about how I'll test this.
 
 I can test for you, no probleme with that.

Any chance you can give this a go?  I've a bigger patch in
a longer series, but this should be the minimal fix.  If it
works, I'll ship it to Junio.

Thanks,

-- Pete

8

From 8556ab04dd126184e26a380b7ed08998fd33debe Mon Sep 17 00:00:00 2001
From: Pete Wyckoff p...@padd.com
Date: Thu, 16 Jan 2014 18:34:09 -0500
Subject: [PATCH] git p4: work around p4 bug that causes empty symlinks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Damien Gérard highlights an interesting problem.  Some p4
repositories end up with symlinks that have an empty target.  It
is not possible to create this with current p4, but they do
indeed exist.

The effect in git p4 is that p4 print on the symlink returns an
empty string, confusing the curret symlink-handling code.

In p4, syncing to a change that includes such a bogus symlink
creates errors:

//depot/empty-symlink - updating /home/me/p4/empty-symlink
rename: /home/me/p4/empty-symlink: No such file or directory

and leaves no symlink.

Replicate the p4 behavior by ignoring these bogus symlinks.  If
they are fixed in later revisions, the symlink will be replaced
properly.

Reported-by: Damien Gérard dam...@iwi.me
Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 5ea8bb8..e798ecf 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2075,7 +2075,14 @@ class P4Sync(Command, P4UserMap):
 # p4 print on a symlink sometimes contains target\n;
 # if it does, remove the newline
 data = ''.join(contents)
-if data[-1] == '\n':
+if not data:
+# Some version of p4 allowed creating a symlink that pointed
+# to nothing.  This causes p4 errors when checking out such
+# a change, and errors here too.  Work around it by ignoring
+# the bad symlink; hopefully a future change fixes it.
+print \nIgnoring empty symlink in %s % file['depotFile']
+return
+elif data[-1] == '\n':
 contents = [data[:-1]]
 else:
 contents = [data]
-- 
1.8.5.2.320.g99957e5

--
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: exception when cloning a perforce repository

[Pete Wyckoff]

dam...@iwi.me wrote on Wed, 15 Jan 2014 09:56 +0100:
 p4 fstat  //depot/openssl/0.9.8j/openssl/include/openssl/bn.h@59702 
 ... depotFile //depot/openssl/0.9.8j/openssl/include/openssl/bn.h
 ... headAction edit
 ... headType symlink
 ... headTime 1237906419
 ... headRev 2
 ... headChange 59702
 ... headModTime 1231329423
 
 p4 print -q //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 | od -c
 000
 
 p4 print  //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1
 //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1 - add change 59574 
 (text)
  p4 print  //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2
 //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 - edit change 59702 
 (symlink)

That's interesting.  When I do the equivalent p4 print commands
it shows something like this.

arf-git-test$ p4 fstat //depot/bn.h
... depotFile //depot/bn.h
... clientFile /dev/shm/trash directory.t9802-git-p4-filetype/cli/bn.h
... isMapped 
... headAction edit
... headType symlink
... headTime 1389876870
... headRev 2
... headChange 8
... headModTime 1389876870
... haveRev 2

arf-git-test$ p4 print //depot/bn.h#1
//depot/bn.h#1 - add change 7 (text)
file-text

arf-git-test$ p4 print //depot/bn.h#2
//depot/bn.h#2 - edit change 8 (symlink)
/elsewhere/bn.h

I don't know how you manage to get a symlink with an empty
destination like that.

I'll work on a way to hack around this failure.  In the mean time,
if you're game, it might be fun to see what p4 does with such a
repository.  You could make a client for just that little subdir,
check out at 59702 and see what is there:

mkdir testmess
cd testmess
cat EOF | p4 client -i
Client: testmess
Description: testmess
Root: $(pwd)
View: //depot/openssl/0.9.8j/openssl/include/openssl/... //testmess/...
EOF

then take a look at how p4 represents the empty symlink
in the filesystem:

p4 sync @59702
ls -la bn.h

-- 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

Re: git-p4: exception when cloning a perforce repository

[Pete Wyckoff]

dam...@iwi.me wrote on Thu, 16 Jan 2014 14:46 +0100:
 
 On 16 Jan 2014, at 14:08, Pete Wyckoff p...@padd.com wrote:
 
  dam...@iwi.me wrote on Wed, 15 Jan 2014 09:56 +0100:
  p4 fstat  //depot/openssl/0.9.8j/openssl/include/openssl/bn.h@59702 
  ... depotFile //depot/openssl/0.9.8j/openssl/include/openssl/bn.h
  ... headAction edit
  ... headType symlink
  ... headTime 1237906419
  ... headRev 2
  ... headChange 59702
  ... headModTime 1231329423
  
  p4 print -q //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 | od -c
  000
  
  p4 print  //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1
  //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#1 - add change 59574 
  (text)
  p4 print  //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2
  //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 - edit change 59702 
  (symlink)
  
  That's interesting.  When I do the equivalent p4 print commands
  it shows something like this.
  
  arf-git-test$ p4 fstat //depot/bn.h
  ... depotFile //depot/bn.h
  ... clientFile /dev/shm/trash directory.t9802-git-p4-filetype/cli/bn.h
  ... isMapped 
  ... headAction edit
  ... headType symlink
  ... headTime 1389876870
  ... headRev 2
  ... headChange 8
  ... headModTime 1389876870
  ... haveRev 2
  
  arf-git-test$ p4 print //depot/bn.h#1
  //depot/bn.h#1 - add change 7 (text)
  file-text
  
  arf-git-test$ p4 print //depot/bn.h#2
  //depot/bn.h#2 - edit change 8 (symlink)
  /elsewhere/bn.h
  
  I don't know how you manage to get a symlink with an empty
  destination like that.
  
  I'll work on a way to hack around this failure.  In the mean time,
  if you're game, it might be fun to see what p4 does with such a
  repository.  You could make a client for just that little subdir,
  check out at 59702 and see what is there:
  
  mkdir testmess
  cd testmess
  cat EOF | p4 client -i
  Client: testmess
  Description: testmess
  Root: $(pwd)
  View: //depot/openssl/0.9.8j/openssl/include/openssl/... //testmess/...
  EOF
  
  then take a look at how p4 represents the empty symlink
  in the filesystem:
  
  p4 sync @59702
  ls -la bn.h
 
 I’ve tried exactly your commands, and I’ve got an empty folder..
 
 {14:38}~/p4/testmess ➭ p4 sync @59702
 //depot/openssl/0.9.8j/openssl/include/openssl/aes.h#2 - refreshing 
 /home/dgerard/p4/testmess/aes.h
 //depot/openssl/0.9.8j/openssl/include/openssl/asn1.h#2 - refreshing 
 /home/dgerard/p4/testmess/asn1.h
 //depot/openssl/0.9.8j/openssl/include/openssl/asn1_mac.h#2 - refreshing 
 /home/dgerard/p4/testmess/asn1_mac.h
 //depot/openssl/0.9.8j/openssl/include/openssl/asn1t.h#2 - refreshing 
 /home/dgerard/p4/testmess/asn1t.h
 //depot/openssl/0.9.8j/openssl/include/openssl/bio.h#2 - refreshing 
 /home/dgerard/p4/testmess/bio.h
 //depot/openssl/0.9.8j/openssl/include/openssl/blowfish.h#2 - refreshing 
 /home/dgerard/p4/testmess/blowfish.h
 //depot/openssl/0.9.8j/openssl/include/openssl/bn.h#2 - refreshing 
 /home/dgerard/p4/testmess/bn.h
 //depot/openssl/0.9.8j/openssl/include/openssl/buffer.h#2 - refreshing 
 /home/dgerard/p4/testmess/buffer.h
 […]
 
 
 {14:39}~/p4/testmess ➭ ls -la
 total 12
 drwxr-xr-x 2 dgerard dgerard 4096 janv. 16 14:37 .
 drwxr-xr-x 4 dgerard dgerard 4096 janv. 16 14:34 ..
 -rw-r--r-- 1 dgerard dgerard   93 janv. 16 14:37 .perforce
 
 
 Then I tried to sync the previous changeset, which is ok :
 
 {14:44}~/p4/testmess ➭ p4 sync -f @59701
 //depot/openssl/0.9.8j/openssl/include/openssl/aes.h#1 - updating 
 /home/dgerard/p4/testmess/aes.h
 […]
 
 {14:44}~/p4/testmess ➭ l
 total 0
 -r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 aes.h
 -r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1.h
 -r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1_mac.h
 -r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 asn1t.h
 -r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 bio.h
 -r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 blowfish.h
 -r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 bn.h
 -r--r--r-- 1 dgerard dgerard 0 janv. 16 14:44 buffer.h
 […]
 
 
 
 However, when trying to sync to the appropriate changeset :
 
 {14:44}~/p4/testmess ➭ p4 sync -f @59702
 //depot/openssl/0.9.8j/openssl/include/openssl/aes.h#2 - updating 
 /home/dgerard/p4/testmess/aes.h
 rename: /home/dgerard/p4/testmess/aes.h: No such file or directory
 //depot/openssl/0.9.8j/openssl/include/openssl/asn1.h#2 - updating 
 /home/dgerard/p4/testmess/asn1.h
 rename: /home/dgerard/p4/testmess/asn1.h: No such file or directory
 //depot/openssl/0.9.8j/openssl/include/openssl/asn1_mac.h#2 - updating 
 /home/dgerard/p4/testmess/asn1_mac.h
 rename: /home/dgerard/p4/testmess/asn1_mac.h: No such file or directory
 //depot/openssl/0.9.8j/openssl/include/openssl/asn1t.h#2 - updating 
 /home/dgerard/p4/testmess/asn1t.h
 rename: /home/dgerard/p4/testmess/asn1t.h: No such file or directory
 //depot/openssl/0.9.8j/openssl/include/openssl/bio.h#2 - updating 
 /home/dgerard/p4/testmess/bio.h
 rename: /home/dgerard/p4/testmess/bio.h: No such file or directory
 //depot/openssl/0.9.8j

Re: git-p4: exception when cloning a perforce repository

[Pete Wyckoff]

p...@padd.com wrote on Mon, 13 Jan 2014 19:18 -0500:
 dam...@iwi.me wrote on Mon, 13 Jan 2014 14:37 +0100:
  I am trying to clone a perforce repository via git and I am having the 
  following backtrace :
  
  {14:20}~/projects/:master ✗ ➭ git p4 clone //depot/@all .
  Importing revision …
  [...]
  Importing revision 59702 (45%)Traceback (most recent call last):
 [..]
File /opt/git/libexec/git-core/git-p4, line 2078, in streamOneP4File
  if data[-1] == '\n':
  IndexError: string index out of range
  
  git —version: git version 1.8.5.2.309.ga25014b [last commit from master 
  from github.com/git/git]
  os : ubuntu 13.10
 
 This code:
 
 if type_base == symlink:
 git_mode = 12
 # p4 print on a symlink sometimes contains target\n;
 # if it does, remove the newline
 data = ''.join(contents)
   ==   if data[-1] == '\n':
 contents = [data[:-1]]
 else:
 contents = [data]
 
 means that data is an empty string.  Implies you've got a
 symlink pointing to nothing.  Is that even possible?
 
 It could be this is a regression introduced at 1292df1 (git-p4:
 Fix occasional truncation of symlink contents., 2013-08-08).  The
 old way of doing data[:-1] unconditionally would have worked but
 was broken for other reasons.
 
 Could you investigate the symlink a bit?  We're looking for
 one in change 59702 that points to nowhere.  Maybe do:
 
 $ p4 describe -s 59702
 
 and see if you can figure out which of those could be a symlink, then
 inspect it:
 
 $ p4 fstat //depot/symlink@59702
 (probably shows it is headRev 1)
 
 $ p4 print -q //depot/symlink#1
 
 $ p4 print -q //depot/symlink#1 | od -c
 
 Thanks for checking this depot info first.

I've tried to hack a test that produces a null symlink,
and having done so, find an error later on trying to
generate a symlink that points to .  So the easy
fix of checking for an empty string is unlikely to work
for your repo.

Curious as to how you managed to generate such a thing.
If you find the file, and can get at the p4 depot, the
full ,v file would be interesting too.

-- 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

Re: [PATCH] git-p4: Do not include diff in spec file when just preparing p4

[Pete Wyckoff]

frrr...@gmail.com wrote on Mon, 13 Jan 2014 12:10 +:
 Hello,
 
 On Sun, Jan 12, 2014 at 05:29:46PM -0500, Pete Wyckoff wrote:
  Thanks for the patch, but I'm curious how you'd like this to
  work.  I never use the option myself.
  
  As it is, --prepare-p4-only generates a file in /tmp/ that has
  exactly the contents you'd see in the editor during git p4
  submit.  It includes the diff of the change, presumably to help
  with writing the description.
 
 Yes, I believe it makes sense to display the diff in this case, as we
 can remove it later programmatically.
  
  Now you can't actually feed this file directly to p4 submit
  without deleting the diff.  That's the part you don't like?
 
 Yes, I do not use that for submitting, but for shelving. I can run
 git p4 submit --prepare-p4-only followed by p4 shelve -i  /tmp/...
 and perforce will shelve the corresponding change.
 
 Removing the diff could be done externally, however git-p4 itself
 tells the user it can submit using the generated file, which is
 not the case if we keep the diff in it.

I'm convinced.  That explanation makes sense, thanks.

It would be nice to do a few more things with this patch.  Here's
some ideas, sorted in priority order.

1.  Put slightly more text into the commit message, possibly
from your email above.

2.  Refactor out that big chunk of code instead of just
moving it.  Selectively call it only if not prepare_p4_only.

3.  Modify the t9807 test 'submit --prepare-p4-only' to make
sure the diff isn't there.

4.  Documentation update?  Probably not necessary.

Let me know if you're interested in doing any of this.

-- 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

Re: git-p4: exception when cloning a perforce repository

[Pete Wyckoff]

dam...@iwi.me wrote on Mon, 13 Jan 2014 14:37 +0100:
 I am trying to clone a perforce repository via git and I am having the 
 following backtrace :
 
 {14:20}~/projects/:master ✗ ➭ git p4 clone //depot/@all .
 Importing revision …
 [...]
 Importing revision 59702 (45%)Traceback (most recent call last):
[..]
   File /opt/git/libexec/git-core/git-p4, line 2078, in streamOneP4File
 if data[-1] == '\n':
 IndexError: string index out of range
 
 git —version: git version 1.8.5.2.309.ga25014b [last commit from master from 
 github.com/git/git]
 os : ubuntu 13.10

This code:

if type_base == symlink:
git_mode = 12
# p4 print on a symlink sometimes contains target\n;
# if it does, remove the newline
data = ''.join(contents)
  ==   if data[-1] == '\n':
contents = [data[:-1]]
else:
contents = [data]

means that data is an empty string.  Implies you've got a
symlink pointing to nothing.  Is that even possible?

It could be this is a regression introduced at 1292df1 (git-p4:
Fix occasional truncation of symlink contents., 2013-08-08).  The
old way of doing data[:-1] unconditionally would have worked but
was broken for other reasons.

Could you investigate the symlink a bit?  We're looking for
one in change 59702 that points to nowhere.  Maybe do:

$ p4 describe -s 59702

and see if you can figure out which of those could be a symlink, then
inspect it:

$ p4 fstat //depot/symlink@59702
(probably shows it is headRev 1)

$ p4 print -q //depot/symlink#1

$ p4 print -q //depot/symlink#1 | od -c

Thanks for checking this depot info first.

-- 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

Re: [PATCH] git-p4: Do not include diff in spec file when just preparing p4

[Pete Wyckoff]

frrr...@gmail.com wrote on Fri, 10 Jan 2014 18:18 +:
 The diff information render the spec file unusable as is by p4,
 do not include it when run with --prepare-p4-only so that the
 given file can be directly passed to p4.

Thanks for the patch, but I'm curious how you'd like this to
work.  I never use the option myself.

As it is, --prepare-p4-only generates a file in /tmp/ that has
exactly the contents you'd see in the editor during git p4
submit.  It includes the diff of the change, presumably to help
with writing the description.

Now you can't actually feed this file directly to p4 submit
without deleting the diff.  That's the part you don't like?

-- 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

Re: [PATCH 4/9] contrib: remove git-p4import

[Pete Wyckoff]

jrnie...@gmail.com wrote on Mon, 25 Nov 2013 12:58 -0800:
 The git p4import documentation has suggested git p4 as a better
 alternative for more than 6 years.  (According to the mailing list
 discussion when it was moved to contrib/, git-p4import has serious
 bugs --- e.g., its incremental mode just doesn't work.) Since then,
 git p4 has been actively developed and was promoted to a standard git
 command alongside git svn.
 
 Searches on google.com/trends and stackoverflow suggest that no one is
 looking for git-p4import any more.  Remove it.
 
 Noticed while considering marking the contrib/p4import/git-p4import.py
 script executable as part of a wider sweep.

I haven't seen git-p4import mentioned for the last 6 years either.
Thanks,

Acked-by: Pete Wyckoff p...@padd.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

Re: [PATCH] git p4: Use git diff-tree instead of format-patch

[Pete Wyckoff]

gits...@pobox.com wrote on Thu, 21 Nov 2013 11:47 -0800:
 Crestez Dan Leonard cdleon...@gmail.com writes:
 
  The output of git format-patch can vary with user preferences. In
  particular setting diff.noprefix will break the git apply that
  is done as part of git p4 submit.
 
  Signed-off-by: Crestez Dan Leonard cdleon...@gmail.com
  ---
   git-p4.py | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff --git a/git-p4.py b/git-p4.py
  index 31e71ff..fe988ce 100755
  --- a/git-p4.py
  +++ b/git-p4.py
  @@ -1308,7 +1308,7 @@ class P4Submit(Command, P4UserMap):
   else:
   die(unknown modifier %s for %s % (modifier, path))
   
  -diffcmd = git format-patch -k --stdout \%s^\..\%s\ % (id, id)
  +diffcmd = git diff-tree -p \%s\ % (id)
   patchcmd = diffcmd +  | git apply 
   tryPatchCmd = patchcmd + --check -
   applyPatchCmd = patchcmd + --check --apply -
 
 I do not do p4 myself, but from a cursory reading it looks like the
 right thing to do.  Thanks.
 
 The output of git shortlog --no-merges --since=1.year git-p4.py
 tells me that Pete should be the person much more familiar with the
 code than myself, so I'll Cc him just in case...

This looks great, and passes all my tests.

Acked-by: Pete Wyckoff p...@padd.com

Thanks,

-- 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

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

[Pete Wyckoff]

cmt...@gmail.com wrote on Fri, 06 Sep 2013 15:03 -0400:
 Finally, I claim success!  Unfortunately I did not try either of the OOM
 score or strace suggestions - sorry!  After spending so much time on
 this, I've gotten to the point that I'm more interested in getting it to
 work than in figuring out why the direct approach isn't working; it
 sounds like you're both pretty confident that git is working as it
 should, and I don't maintain the system I'm doing this on so I don't
 doubt that there might be some artificial limit or other quirk here that
 we just aren't seeing.
 
 Anyway, what I found is that Pete's incremental method does work, I just
 have to know how to do it properly!  This is what I WAS doing to
 generate the error message I pasted several posts ago:
 
 git clone //path/to/branch@begin,stage1
 cd branch
 git sync //path/to/branch@stage2
 # ERROR!
 # (I also tried //path/to/branch@stage1+1,stage2, same error)
 
 Eventually what happened is that I downloaded the free 20-user p4d, set
 up a very small repository with only 4 changes, and started some old
 fashioned trial-and-error.  Here's what I should have been doing all
 along:
 
 git clone //path/to/branch@begin,stage1
 cd branch
 git sync //path/to/branch@begin,stage2
 git sync //path/to/branch@begin,stage3
 # and so on...
 
 And syncing a few thousand changes every day over the course of the past
 week, my git repo is finally up to the Perforce HEAD.  So I suppose
 ultimately this was my own misunderstanding, partly because when you
 begin your range at the original first change number the output looks
 suspiciously like it's importing changes again that it's already
 imported.  Maybe this is all documented somewhere, and if it is I just
 failed to find it.
 
 Thanks to both of you for all your help!

That you got it to work is the most important thing.  Amazing all
the effort you put into it; a lesser hacker would have walked
away much earlier.

The changes don't overlap.  If you give it a range that includes
changes already synced, git-p4 makes sure to start only at the
lowest change it has not yet seen.  I'll see if I can update the
docs somewhere.

-- 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

Re: [PATCH v2] git-p4: Ask p4 to interpret View setting

[Pete Wyckoff]

ksaitoh...@gmail.com wrote on Tue, 27 Aug 2013 11:43 +0900:
  Do you have an updated patch?  Want to take some time to clean up and
  resubmit the entire series?  The batching should be incorporated with
  the last 2/2 that I sent out.
 
 I don't have other update.
 I'm satisfied because able to want to do and it became better than my
 original modification thanks to your cooperation.
 ( a few-hundred-thousand file repo
 I didn't think that it work with so HUGE repo.)
 
 How should I do?
 Should I create one patch mail that incorporated your sent one?
 Or nothing to do?

It would be good if you could fold the one I sent in with yours,
and clean up any stylistic issues as you go.

I'll play with it a bit more, then send on to Junio for
the next release.

Thanks, this is a good addition!

-- Pete


 2013/8/25 Pete Wyckoff p...@padd.com:
  p...@padd.com wrote on Thu, 15 Aug 2013 21:24 -0400:
  ksaitoh...@gmail.com wrote on Wed, 14 Aug 2013 09:59 +0900:
My only concern is in the commit message, about performance.  A
change that has lots of files in it will cause many roundtrips to
p4d to do p4 where on each.  When the files don't have much
edited content, this new approach will make the import take twice
as long, I'll guess.  Do you have a big repository where you
could test that?
  
   I measured performance of git p4 clone  --use-client-spec with a
   repository it has 1925 files, 50MB.
 Original:8.05s user 32.02s system 15% cpu 4:25.34 total
 Apply patch:9.02s user 53.19s system 14% cpu 6:56.41 total
  
   It is acceptable in my situation, but looks quite slow...
  
   Then I implemented one batch query version
  7.92s user 33.03s system 15% cpu 4:25.59 total
  
   It is same as original
  
   My additional patch is below.
   I investigate call graph (attached rough sketch) and
   implement batch query in commit() and splitFilesIntoBranches().
   In addition, modified map_in_client to just search cache value.
  
   Could you accept?
 
  This looks good.  I've started my own performance testing
  on a few-hundred-thousand file repo to confirm your findings.
 
  If it seems to work out, we can clean up the patch.  Otherwise
  maybe need to think about having both implementations and use
  the by-hand one for   I don't like that approach.  Let's
  hope it's not needed.
 
  I tried with a few repos:
 
  Small repo, single-commit clone:
 
  Current: 0m0.35s user 0m0.30s sys 0m11.52s elapsed 5.69 %CPU
  No batching: 0m0.66s user 0m0.77s sys 0m34.42s elapsed 4.17 %CPU
  Batching:0m0.28s user 0m0.29s sys 0m10.85s elapsed 5.27 %CPU
 
  Big repo, single-commit clone:
 
  Current: 6m21.38s user 1m35.36s sys 19m44.83s elapsed 40.23 %CPU
  No batching: 1m53.13s user 24m34.35s sys 146m13.80s elapsed 18.09 %CPU 
  (*)
  Batching:6m22.01s user 1m44.23s sys 21m19.73s elapsed 37.99 %CPU
 
  The no batching run died with an unrelated p4 timeout.
 
  Big repo, 1000 incremental changes:
 
  Current: 0m13.43s user 0m19.82s sys 11m12.58s elapsed 4.94 %CPU
  No batching: 0m20.29s user 0m39.94s sys 38m44.69s elapsed 2.59 %CPU (*)
  Batching:0m16.15s user 0m26.60s sys 13m55.69s elapsed 5.11 %CPU
 
  The no batching run died at 28% of the way through.
 
  There is probably a 20%-ish slowdown in my environment with this
  approach.  But given that the timescale for these operations is
  measured in the tens of minutes, I don't think a couple more matters
  too much to anybody.
 
  The attractiveness of the simplicity and increased client spec feature
  coverage weighs in its favor.  Let's go ahead and inflict this on the
  world and see what they think.
 
  Do you have an updated patch?  Want to take some time to clean up and
  resubmit the entire series?  The batching should be incorporated with
  the last 2/2 that I sent out.
 
  -- 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

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

[Pete Wyckoff]

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

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

[Pete Wyckoff]

cmt...@gmail.com wrote on Fri, 23 Aug 2013 07:48 -0400:
 On Fri, Aug 23, 2013 at 08:16:58AM +0100, Luke Diamand wrote:
  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.
[..]
 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.

Modern git, including your version, do streaming reads from p4,
so the git-p4 python process never even holds a whole file's
worth of data.  You're seeing git-fast-import die, it seems.  It
will hold onto the entire file contents.  But just one, not the
entire repo.  How big is the single largest file?

You can import in pieces.  See the change numbers like this:

p4 changes -m 1000 //depot/big/...
p4 changes -m 1000 //depot/big/...@some-old-change

Import something far enough back in history so that it seems
to work:

git p4 clone --destination=big //depot/big@60602
cd big

Sync up a bit at a time:

git p4 sync @60700
git p4 sync @60800
...

I don't expect this to get around the problem you describe,
however.  Sounds like there is one gigantic file that is causing
git-fast-import to fill all of memory.  You will at least isolate
the change.

There are options to git-fast-import to limit max pack size
and to cause it to skip importing files that are too big, if
that would help.

You can also use a client spec to hide the offending files
from git.

Can you watch with top?  Hit M to sort by memory usage, and
see how big the processes get before falling over.

-- 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

Re: [PATCH v2] git-p4: Ask p4 to interpret View setting

[Pete Wyckoff]

p...@padd.com wrote on Thu, 15 Aug 2013 21:24 -0400:
 ksaitoh...@gmail.com wrote on Wed, 14 Aug 2013 09:59 +0900:
   My only concern is in the commit message, about performance.  A
   change that has lots of files in it will cause many roundtrips to
   p4d to do p4 where on each.  When the files don't have much
   edited content, this new approach will make the import take twice
   as long, I'll guess.  Do you have a big repository where you
   could test that?
  
  I measured performance of git p4 clone  --use-client-spec with a
  repository it has 1925 files, 50MB.
Original:8.05s user 32.02s system 15% cpu 4:25.34 total
Apply patch:9.02s user 53.19s system 14% cpu 6:56.41 total
  
  It is acceptable in my situation, but looks quite slow...
  
  Then I implemented one batch query version
 7.92s user 33.03s system 15% cpu 4:25.59 total
  
  It is same as original
  
  My additional patch is below.
  I investigate call graph (attached rough sketch) and
  implement batch query in commit() and splitFilesIntoBranches().
  In addition, modified map_in_client to just search cache value.
  
  Could you accept?
 
 This looks good.  I've started my own performance testing
 on a few-hundred-thousand file repo to confirm your findings.
 
 If it seems to work out, we can clean up the patch.  Otherwise
 maybe need to think about having both implementations and use
 the by-hand one for   I don't like that approach.  Let's
 hope it's not needed.

I tried with a few repos:

Small repo, single-commit clone:

Current: 0m0.35s user 0m0.30s sys 0m11.52s elapsed 5.69 %CPU
No batching: 0m0.66s user 0m0.77s sys 0m34.42s elapsed 4.17 %CPU
Batching:0m0.28s user 0m0.29s sys 0m10.85s elapsed 5.27 %CPU

Big repo, single-commit clone:

Current: 6m21.38s user 1m35.36s sys 19m44.83s elapsed 40.23 %CPU
No batching: 1m53.13s user 24m34.35s sys 146m13.80s elapsed 18.09 %CPU (*)
Batching:6m22.01s user 1m44.23s sys 21m19.73s elapsed 37.99 %CPU

The no batching run died with an unrelated p4 timeout.

Big repo, 1000 incremental changes:

Current: 0m13.43s user 0m19.82s sys 11m12.58s elapsed 4.94 %CPU
No batching: 0m20.29s user 0m39.94s sys 38m44.69s elapsed 2.59 %CPU (*)
Batching:0m16.15s user 0m26.60s sys 13m55.69s elapsed 5.11 %CPU

The no batching run died at 28% of the way through.

There is probably a 20%-ish slowdown in my environment with this
approach.  But given that the timescale for these operations is
measured in the tens of minutes, I don't think a couple more matters
too much to anybody.

The attractiveness of the simplicity and increased client spec feature
coverage weighs in its favor.  Let's go ahead and inflict this on the
world and see what they think.

Do you have an updated patch?  Want to take some time to clean up and
resubmit the entire series?  The batching should be incorporated with
the last 2/2 that I sent out.

-- 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

Re: What's cooking in git.git (Aug 2013, #03; Tue, 13)

[Pete Wyckoff]

gits...@pobox.com wrote on Tue, 13 Aug 2013 15:06 -0700:
 * ks/p4-view-spec (2013-08-11) 3 commits
  - WAITING FOR ACK
  - git p4: implement view spec wildcards with p4 where
  - git p4 test: sanitize P4CHARSET
 
  Waiting for an ack.

I'm still running perf tests on the 3-patch version.  It
looks good.  You should expect a reroll of the 2nd and 3rd
commits, combining them into one patch.

Thanks for tracking these.

-- 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

Re: [PATCH v2] git-p4: Ask p4 to interpret View setting

[Pete Wyckoff]

ksaitoh...@gmail.com wrote on Wed, 14 Aug 2013 09:59 +0900:
  My only concern is in the commit message, about performance.  A
  change that has lots of files in it will cause many roundtrips to
  p4d to do p4 where on each.  When the files don't have much
  edited content, this new approach will make the import take twice
  as long, I'll guess.  Do you have a big repository where you
  could test that?
 
 I measured performance of git p4 clone  --use-client-spec with a
 repository it has 1925 files, 50MB.
   Original:8.05s user 32.02s system 15% cpu 4:25.34 total
   Apply patch:9.02s user 53.19s system 14% cpu 6:56.41 total
 
 It is acceptable in my situation, but looks quite slow...
 
 Then I implemented one batch query version
7.92s user 33.03s system 15% cpu 4:25.59 total
 
 It is same as original
 
 My additional patch is below.
 I investigate call graph (attached rough sketch) and
 implement batch query in commit() and splitFilesIntoBranches().
 In addition, modified map_in_client to just search cache value.
 
 Could you accept?

This looks good.  I've started my own performance testing
on a few-hundred-thousand file repo to confirm your findings.

If it seems to work out, we can clean up the patch.  Otherwise
maybe need to think about having both implementations and use
the by-hand one for   I don't like that approach.  Let's
hope it's not needed.

-- Pete

 Subject: [PATCH] git p4: Implement as one batch p4 where query to interpret
  view spec
 
 Query for each file is decrese performance.
 So I implement query to get client file path as one batch query.
 The query must called before use client path (map_in_client() ).
 
 Result of performance measurement, about 40% speed up
 
 Signed-off-by: KazukiSaitoh ksaitoh...@gmail.com
 ---
  git-p4.py | 70 
 ++-
  1 file changed, 51 insertions(+), 19 deletions(-)
 
 diff --git a/git-p4.py b/git-p4.py
 index 40522f7..8cbee24 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -1849,37 +1849,46 @@ class View(object):
  if not exclude:
  self.mappings.append(depot_side)
 
 -def map_in_client(self, depot_path):
 -Return the relative location in the client where this
 -   depot file should live.  Returns  if the file should
 -   not be mapped in the client.
 +def convert_client_path(self, clientFile):
 +# chop off //client/ part to make it relative
 +if not clientFile.startswith(self.client_prefix):
 +die(No prefix '%s' on clientFile '%s' %
 +(self.client_prefix, clientFile))
 +return clientFile[len(self.client_prefix):]
 
 -if depot_path in self.client_spec_path_cache:
 -return self.client_spec_path_cache[depot_path]
 +def update_client_spec_path_cache(self, files):
 +fileArgs = [f for f in files if f not in self.client_spec_path_cache]
 
 -where_result = p4CmdList(['where', depot_path])
 -if len(where_result) == 0:
 -die(No result from 'p4 where %s' % depot_path)
 -client_path = 
 +if len(fileArgs) == 0:
 +return  # All files in cache
 +
 +where_result = p4CmdList([-x, -, where], stdin=fileArgs)
  for res in where_result:
  if code in res and res[code] == error:
  # assume error is ... file(s) not in client view
 -client_path = 
  continue
  if clientFile not in res:
  die(No clientFile from 'p4 where %s' % depot_path)
  if unmap in res:
  # it will list all of them, but only one not unmap-ped
  continue
 -# chop off //client/ part to make it relative
 -clientFile = res[clientFile]
 -if not clientFile.startswith(self.client_prefix):
 -die(No prefix '%s' on clientFile '%s' %
 -(self.client_prefix, clientFile))
 -client_path = clientFile[len(self.client_prefix):]
 +self.client_spec_path_cache[res['depotFile']] =
 self.convert_client_path(res[clientFile])
 +
 +# not found files or unmap files set to 
 +for depotFile in fileArgs:
 +if depotFile not in self.client_spec_path_cache:
 +self.client_spec_path_cache[depotFile] = 
 +
 +def map_in_client(self, depot_path):
 +Return the relative location in the client where this
 +   depot file should live.  Returns  if the file should
 +   not be mapped in the client.
 +
 +if depot_path in self.client_spec_path_cache:
 +return self.client_spec_path_cache[depot_path]
 
 -self.client_spec_path_cache[depot_path] = client_path
 -return client_path
 +die( Error: %s is not found in client spec path % depot_path )
 +return 
 
  class P4Sync(Command, P4UserMap):
  delete_actions = ( delete, 

Re: [PATCH] git-p4: Fix occasional truncation of symlink contents.

[Pete Wyckoff]

al...@rosedu.org wrote on Mon, 12 Aug 2013 10:46 +0300:
 On 11 August 2013 14:57, Pete Wyckoff p...@padd.com wrote:
  al...@rosedu.org wrote on Thu, 08 Aug 2013 16:17 +0300:
  Symlink contents in p4 print sometimes have a trailing
  new line character, but sometimes it doesn't. git-p4
  should only remove the last character if that character
  is '\n'.
 
  Your patch looks fine, and harmless if symlinks continue
  to have \n on the end.  I'd like to understand a bit why
  this behavior is different for you, though.  Could you do
  this test on a symlink in your depot?
 
  Here //depot/symlink points to symlink-target.  You can
  see the \n in position 0o332 below.  What happens on a
  symlink in your repo?
 
  arf-git-test$ p4 fstat //depot/symlink
  ... depotFile //depot/symlink
  ... clientFile /dev/shm/trash 
  directory.t9802-git-p4-filetype/cli/symlink
  ... isMapped
  ... headAction add
  ... headType symlink
  ... headTime 1376221978
  ... headRev 1
  ... headChange 6
  ... headModTime 1376221978
  ... haveRev 1
 
  arf-git-test$ p4 -G print //depot/symlink | od -c
  000   {   s 004  \0  \0  \0   c   o   d   e   s 004  \0  \0  \0   s
  020   t   a   t   s  \t  \0  \0  \0   d   e   p   o   t   F   i   l
  040   e   s 017  \0  \0  \0   /   /   d   e   p   o   t   /   s   y
  060   m   l   i   n   k   s 003  \0  \0  \0   r   e   v   s 001  \0
  100  \0  \0   1   s 006  \0  \0  \0   c   h   a   n   g   e   s 001
  120  \0  \0  \0   6   s 006  \0  \0  \0   a   c   t   i   o   n   s
  140 003  \0  \0  \0   a   d   d   s 004  \0  \0  \0   t   y   p   e
  160   s  \a  \0  \0  \0   s   y   m   l   i   n   k   s 004  \0  \0
  200  \0   t   i   m   e   s  \n  \0  \0  \0   1   3   7   6   2   2
  220   1   9   7   8   s  \b  \0  \0  \0   f   i   l   e   S   i   z
  240   e   s 002  \0  \0  \0   1   5   0   {   s 004  \0  \0  \0   c
  260   o   d   e   s 006  \0  \0  \0   b   i   n   a   r   y   s 004
  300  \0  \0  \0   d   a   t   a   s 017  \0  \0  \0   s   y   m   l
  320   i   n   k   -   t   a   r   g   e   t  \n   0   {   s 004  \0
  340  \0  \0   c   o   d   e   s 006  \0  \0  \0   b   i   n   a   r
  360   y   s 004  \0  \0  \0   d   a   t   a   s  \0  \0  \0  \0   0
  400
 
  Also, what version is your server, from p4 info:
 
  Server version: P4D/LINUX26X86_64/2013.1/610569 (2013/03/19)
 
  -- Pete
 
 
 Hello!
 
 Let me give you an example. Here are the links as resulted in the git
 p4 clone (one was correct, one wasn't):
 
 ./lib/update.sh - ../update.sh
 ./apps/update.sh - ../update.s
 
 
 alexj@ixro-alexj ~/perforce $ p4 print path/lib/update.sh
 //path/update.sh#6 - edit change 119890 (symlink)
 ../update.sh
 alexj@ixro-alexj ~/perforce $ p4 print path/apps/update.sh
 //path/apps/update.sh#144 - edit change 116063 (symlink)
 ../update.shalexj@ixro-alexj ~/perforce/unstable $
 
 Notice the output and the prompt.
 
 (I removed the exact path to the file from the output)
 
 The fstat output is kind of irrelevant, but the hexdump shows the missing \n:
 
 p4 -G print lib/update.sh|od -c
 
 360   t   e   .   s   h  \n   0
 
 p4 -G print apps/update.sh|od -c
 360   p   d   a   t   e   .   s   h   0

I had forgotten, in fact, another thread on this very topic:

http://thread.gmane.org/gmane.comp.version-control.git/221500

Now with your evidence, I think we can decide that no matter how
the symlink managed to sneak into p4d, git p4 should be able to
handle it.

The only problem is that due to the \n ambiguity, in your setup
where p4d loses the \n, git p4 will not handle symlinks with a
target that ends with a newline, e.g.:

symlink(foo\n, bar);

But the small chance of someone actually doing that, coupled with
the fact that for you git p4 is unusable with these symlinks,
says we should go ahead and make the change.

You should send the patch to junio for inclusion in pu/ for the
next release, with:

Acked-by: Pete Wyckoff p...@padd.com

Thanks for fixing this!

-- Pete

 Server version: P4D/LINUX26X86_64/2013.1/610569
 
  Signed-off-by: Alex Juncu aju...@ixiacom.com
  Signed-off-by: Alex Badea aba...@ixiacom.com
  ---
   git-p4.py | 8 ++--
   1 file changed, 6 insertions(+), 2 deletions(-)
 
  diff --git a/git-p4.py b/git-p4.py
  index 31e71ff..a53a6dc 100755
  --- a/git-p4.py
  +++ b/git-p4.py
  @@ -2180,9 +2180,13 @@ class P4Sync(Command, P4UserMap):
   git_mode = 100755
   if type_base == symlink:
   git_mode = 12
  -# p4 print on a symlink contains target\n; remove the 
  newline
  +# p4 print on a symlink sometimes contains target\n;
  +# if it does, remove the newline
   data = ''.join(contents)
  -contents = [data[:-1]]
  +if data[-1] == '\n

Re: [PATCH] git-p4: Fix occasional truncation of symlink contents.

[Pete Wyckoff]

al...@rosedu.org wrote on Thu, 08 Aug 2013 16:17 +0300:
 Symlink contents in p4 print sometimes have a trailing
 new line character, but sometimes it doesn't. git-p4
 should only remove the last character if that character
 is '\n'.

Your patch looks fine, and harmless if symlinks continue
to have \n on the end.  I'd like to understand a bit why
this behavior is different for you, though.  Could you do
this test on a symlink in your depot?

Here //depot/symlink points to symlink-target.  You can
see the \n in position 0o332 below.  What happens on a
symlink in your repo?

arf-git-test$ p4 fstat //depot/symlink
... depotFile //depot/symlink
... clientFile /dev/shm/trash directory.t9802-git-p4-filetype/cli/symlink
... isMapped 
... headAction add
... headType symlink
... headTime 1376221978
... headRev 1
... headChange 6
... headModTime 1376221978
... haveRev 1

arf-git-test$ p4 -G print //depot/symlink | od -c
000   {   s 004  \0  \0  \0   c   o   d   e   s 004  \0  \0  \0   s
020   t   a   t   s  \t  \0  \0  \0   d   e   p   o   t   F   i   l
040   e   s 017  \0  \0  \0   /   /   d   e   p   o   t   /   s   y
060   m   l   i   n   k   s 003  \0  \0  \0   r   e   v   s 001  \0
100  \0  \0   1   s 006  \0  \0  \0   c   h   a   n   g   e   s 001
120  \0  \0  \0   6   s 006  \0  \0  \0   a   c   t   i   o   n   s
140 003  \0  \0  \0   a   d   d   s 004  \0  \0  \0   t   y   p   e
160   s  \a  \0  \0  \0   s   y   m   l   i   n   k   s 004  \0  \0
200  \0   t   i   m   e   s  \n  \0  \0  \0   1   3   7   6   2   2
220   1   9   7   8   s  \b  \0  \0  \0   f   i   l   e   S   i   z
240   e   s 002  \0  \0  \0   1   5   0   {   s 004  \0  \0  \0   c
260   o   d   e   s 006  \0  \0  \0   b   i   n   a   r   y   s 004
300  \0  \0  \0   d   a   t   a   s 017  \0  \0  \0   s   y   m   l
320   i   n   k   -   t   a   r   g   e   t  \n   0   {   s 004  \0
340  \0  \0   c   o   d   e   s 006  \0  \0  \0   b   i   n   a   r
360   y   s 004  \0  \0  \0   d   a   t   a   s  \0  \0  \0  \0   0
400

Also, what version is your server, from p4 info:

Server version: P4D/LINUX26X86_64/2013.1/610569 (2013/03/19)

-- Pete


 Signed-off-by: Alex Juncu aju...@ixiacom.com
 Signed-off-by: Alex Badea aba...@ixiacom.com
 ---
  git-p4.py | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/git-p4.py b/git-p4.py
 index 31e71ff..a53a6dc 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -2180,9 +2180,13 @@ class P4Sync(Command, P4UserMap):
  git_mode = 100755
  if type_base == symlink:
  git_mode = 12
 -# p4 print on a symlink contains target\n; remove the newline
 +# p4 print on a symlink sometimes contains target\n;
 +# if it does, remove the newline
  data = ''.join(contents)
 -contents = [data[:-1]]
 +if data[-1] == '\n':
 +contents = [data[:-1]]
 +else:
 +contents = [data]
  
  if type_base == utf16:
  # p4 delivers different text in the python output to -G
 -- 
 1.8.4.rc0.1.g8f6a3e5
--
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 v2] git-p4: Ask p4 to interpret View setting

[Pete Wyckoff]

ksaitoh...@gmail.com wrote on Tue, 06 Aug 2013 15:45 +0900:
 In Perforce, View setting of p4 client can describe
   -//depot/project/files/*.xls //client/project/files/*.xls
 to exclude Excel files.
 But git p4 --use-client-spec cannot support '*'.
 
 In git-p4.py, map_in_client method analyzes View setting and return
 client file path.
 So I modify the method to just ask p4.
 
 
  Let me play with this for a bit.  I wonder about the performance
  aspects of doing a p4 fstat for every file.  Would it be
  possible to do one or a few batch queries higher up somewhere?
 To reduce p4 access, it cache result of asking client path.
 And addition, fstat depends on sync status, so modify to use p4
 where instead of fstat.

I played around with your patch a bit, ending up with this
teensy series.

I redid the code to use clientFile, not path, as that
will work better with AltRoots.  Also I simplified your
test and added a couple more for the now-supported wildcards.
And deleted a bunch of newly dead code.

My only concern is in the commit message, about performance.  A
change that has lots of files in it will cause many roundtrips to
p4d to do p4 where on each.  When the files don't have much
edited content, this new approach will make the import take twice
as long, I'll guess.  Do you have a big repository where you
could test that?

Tell me what you think.

-- 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

[PATCH 1/2] git p4 test: sanitize P4CHARSET

[Pete Wyckoff]

From: kazuki saitoh ksaitoh...@gmail.com

In the tests, p4d is started without using internationalized
mode.  Make sure this environment variable is unset, otherwise
a mis-matched user setting would break the tests.  The error
message would be Unicode clients require a unicode enabled server.

[pw: use unset, add commit text]

Signed-off-by: Kazuki Saitoh ksaitoh...@gmail.com
Signed-off-by: Pete Wyckoff p...@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 2098b9b..ccd918e 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -48,7 +48,8 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
 P4PORT=localhost:$P4DPORT
 P4CLIENT=client
 P4EDITOR=:
-export P4PORT P4CLIENT P4EDITOR
+unset P4CHARSET
+export P4PORT P4CLIENT P4EDITOR P4CHARSET
 
 db=$TRASH_DIRECTORY/db
 cli=$TRASH_DIRECTORY/cli
-- 
1.8.4.rc2.88.ga5463da

--
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 p4 fstat to interpret View setting

[Pete Wyckoff]

ksaitoh...@gmail.com wrote on Fri, 02 Aug 2013 17:02 +0900:
 I trying clone Perforce project and I found git-p4. It's a great tool!
 
 And I don't know how to exclude special extension file in a directory?
 (Practically, I want to exclude Excel files at git p4 clone/sync.)
 
 In Perforce, View setting of p4 client can describe
   -//depot/project/files/*.xls //client/project/files/*.xls
 to exclude Excel files.
 But git p4 --use-client-spec cannot support '*'.
 
 In git-p4.py, map_in_client method analyzes View setting and return
 client file path.
 So I modify the method to just use p4 command, p4 fstat -T clientFile.
 
 I'd like to know your opinions about that and what you think about the
 suggestion.

This is either incredibly brilliant or fundamentally broken.  I'm
hoping for the former.  :)

Your theory is:  there is a client spec, and p4 knows how to
interpret these things, so instead of figuring out and
implementing the algorithms for %% and * and ... in git-p4, just
ask p4 directly.

Let me play with this for a bit.  I wonder about the performance
aspects of doing a p4 fstat for every file.  Would it be
possible to do one or a few batch queries higher up somewhere?

A few nit-picky questions below, just on the test bits, while I
play with your code.

 diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
 index 2098b9b..fbda1ad 100644
 --- a/t/lib-git-p4.sh
 +++ b/t/lib-git-p4.sh
 @@ -48,6 +48,9 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
  P4PORT=localhost:$P4DPORT
  P4CLIENT=client
  P4EDITOR=:
 +P4USER=
 +P4PASS=
 +P4CHARSET=

Why do you need these?

 diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
 index 2bf142d..b97bdda 100755
 --- a/t/t9801-git-p4-branch.sh
 +++ b/t/t9801-git-p4-branch.sh
 @@ -480,6 +480,7 @@ test_expect_success 'use-client-spec detect-branches
 skips branches setup' '
  test_expect_success 'use-client-spec detect-branches skips branches' '
   client_view //depot/usecs/... //client/... \
  -//depot/usecs/b3/... //client/b3/... 
 +( p4 sync ) 
   test_when_finished cleanup_git 

How does the p4 sync help here?

 +test_expect_success 'view wildcard *' '
 + client_view //depot/... //client/... \
 + -//depot/dir1/*.junk //client/dir1/*.junk \
 + -//depot/dir2/*.junk //client/dir2/*.junk 
 + (p4 sync ) 
 + files=dir1/file11 dir1/file12 dir2/file21 dir2/file22 
 + client_verify $files 
 + git p4 clone --use-client-spec --dest=$git //depot 
 + git_verify $files
 +'

It works!  Cool.

-- 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

Re: Git-P4 Bug With Filename Case Change

[Pete Wyckoff]

aaron.dw...@imgtec.com wrote on Wed, 17 Jul 2013 22:11 +:
   We recently have moved our project from Git to Perforce and those of us 
 who prefer Git still are using Git p4 to stay in Git land.  One of the files 
 in our repository was renamed while still in Git, but the rename only 
 consisted of a case change of a character in the name.  Now, on an OS X box 
 with a case insensitive file system (not sure if that matters), one of our 
 guys cloned from perforce with Git p4 and used @all to get all history.  When 
 this operation is finished, the file name is in its original state, not the 
 newer renamed state.

So original file Foo, new file foo, to make it concrete.

The git p4 clone command generates an internal .git/ history of
the entire p4 repository, before checking out any files in the
workspace.  It does this without touching the filesystem, so I
would expect it never to mangle case, even on OSX.

You should be able to verify this with:

mkdir test1
cd test1
git init
git p4 clone --bare --destination . //depot/proj@all

git ls-tree HEAD

and see that foo is there, not Foo.

Then check that the rename really did happen:

git log --stat --summary --follow -- foo

should show a rename Foo = foo in there somewhere.

Does this all work?  I'd like to clear up this confusing part
first.

   Perforce doesn't respect that file as being in the repository.  We 
 noticed this after making a local Git commit and upon issuing a Git p4 
 submit, things go haywire with file(s) not opened on this client and 
 nothing getting submitted.

Yep, it's all bad from there-on, I'm sure.

I'm a bit out of my depth on case-insensitive file systems.  Do
check if the cloner in question has core.ignorecase config option
set:

git config --get core.ignorecase


-- 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

Re: [PATCH] git p4 test: Check ignore files with client spec

[Pete Wyckoff]

vitor@gmail.com wrote on Fri, 19 Jul 2013 00:04 +0100:
 This test confirms that a file can be ignored during git p4 sync if if is
 excluded in P4 client specification.

This is a good check to have, and I'm glad it happens to work.  :)

I'd forgotten during your conversation with Matthieu that we
did indeed have tests for detect-branches with use-client-spec.
This test sure seems like it should cover that situation though.

Acked-by: Pete Wyckoff p...@padd.com

 ---
  t/t9801-git-p4-branch.sh | 23 ---
  1 file changed, 20 insertions(+), 3 deletions(-)
 
 diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
 index 9730821..2bf142d 100755
 --- a/t/t9801-git-p4-branch.sh
 +++ b/t/t9801-git-p4-branch.sh
 @@ -469,9 +469,11 @@ test_expect_success 'use-client-spec detect-branches 
 skips branches setup' '
   View: //depot/usecs/b1/... //depot/usecs/b3/...
   EOF
  
 - echo b3/b3-file3 b3/b3-file3 
 - p4 add b3/b3-file3 
 - p4 submit -d b3/b3-file3
 + echo b3/b3-file3_1 b3/b3-file3_1 
 + echo b3/b3-file3_2 b3/b3-file3_2 
 + p4 add b3/b3-file3_1 
 + p4 add b3/b3-file3_2 
 + p4 submit -d b3/b3-file3_1 b3/b3-file3_2
   )
  '
  
 @@ -487,6 +489,21 @@ test_expect_success 'use-client-spec detect-branches 
 skips branches' '
   )
  '
  
 +test_expect_success 'use-client-spec detect-branches skips files in 
 branches' '
 + client_view //depot/usecs/... //client/... \
 + -//depot/usecs/b3/b3-file3_1 //client/b3/b3-file3_1 
 + test_when_finished cleanup_git 
 + test_create_repo $git 
 + (
 + cd $git 
 + git p4 sync --detect-branches --use-client-spec 
 //depot/usecs@all 
 + git checkout -b master p4/usecs/b3 
 + test_path_is_file b1-file1 
 + test_path_is_file b3-file3_2 
 + test_path_is_missing b3-file3_1
 + )
 +'
 +
  test_expect_success 'kill p4d' '
   kill_p4d
  '
 -- 
 1.8.3.2
 
 --
 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] Change remote tracking to remote-tracking

[Pete Wyckoff]

gits...@pobox.com wrote on Wed, 03 Jul 2013 13:33 -0700:
 Jonathan Nieder jrnie...@gmail.com writes:
 
  Michael Schubert wrote:
 
  --- a/Documentation/git-p4.txt
  +++ b/Documentation/git-p4.txt
  @@ -180,7 +180,7 @@ subsequent 'sync' operations.
 Import changes into given branch.  If the branch starts with
 'refs/', it will be used as is.  Otherwise if it does not start
 with 'p4/', that prefix is added.  The branch is assumed to
  -  name a remote tracking, but this can be modified using
  +  name a remote-tracking, but this can be modified using
 '--import-local', or by giving a full ref name.  The default
 branch is 'master'.
 
  This is confusing both before and after the patch.  What is a remote
  tracking?
 
  Perhaps:
 
  --branch ref::
  Import changes into ref instead of refs/remotes/p4/master.
  If ref starts with refs/, it is used as is.  Otherwise, if
  it does not start with p4/, that prefix is added.
  +
  By default a ref not starting with refs/ is treated as the
  name of a remote-tracking branch (under refs/remotes/).  This
  behavior can be modified using the --import-local option.
  +
  The default ref is master.
 
  The rest of the patch looks good.
 
 Myy reading did hiccup at the same remote-tracking used as if it
 were a noun, and your rewritten version reads much better.

Yes, very clear and complete rewrite; thanks.  The final
paragraph is perhaps duplicative of the first sentence, but adds
clarity, so I'm happy as it stands.

Acked-by: Pete Wyckoff p...@padd.com

-- 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

Re: [PATCH 2/2] t/t9802: explicitly name the upstream branch to use as a base

[Pete Wyckoff]

bca...@nvidia.com wrote on Mon, 17 Jun 2013 18:40 -0700:
 From: Brandon Casey draf...@gmail.com
 
 Prior to commit fa83a33b, the 'git checkout' DWIMery would create a
 new local branch if the specified branch name did not exist and it
 matched exactly one ref in the remotes namespace.  It searched
 the remotes namespace for matching refs using a simple comparison
 of the trailing portion of the remote ref names.  This approach
 could sometimes produce false positives or negatives.
 
 Since fa83a33b, the DWIMery more strictly excludes the remote name
 from the ref comparison by iterating through the remotes that are
 configured in the .gitconfig file.  This has the side-effect that
 any refs that exist in the remotes namespace, but do not match
 the destination side of any remote refspec, will not be used by
 the DWIMery.
 
 This change in behavior breaks the tests in t9802 which relied on
 the old behavior of searching all refs in the remotes namespace,
 since the git-p4 script does not configure any remotes in the
 .gitconfig.  Let's work around this in these tests by explicitly
 naming the upstream branch to base the new local branch on when
 calling 'git checkout'.

Thanks for finding and fixing this.  Great explanation.  I
tested it locally too.

Acked-by: Pete Wyckoff p...@padd.com

-- 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

Re: Bug: git-p4: Sometimes p4 generates Windows-style output on OS X

[Pete Wyckoff]

davidf...@gmail.com wrote on Mon, 06 May 2013 10:59 -0700:
 I've observed that the p4 command that git-p4 delegates to
 occasionally outputs Windows-style line endings even on the OS X
 platform. When this happens, git-p4 gets very confused and crashes
 out.
 
 I've attached a patch which seems to fix the issue in my case. Now
 this patch is a pretty bad hack, and I don't recommend that it be
 accepted as-is. It is just a starting point.
 
 A real fix would determine in advance whether Perforce was going to
 emit Windows-style output. Since I don't know the circumstances
 under which this happens on non-Windows platforms, I can't provide a
 better patch. Someone who has intimate knowledge of p4's operating
 modes would be best to examine what's really going on with p4.

You've changed the part where git-p4 reads the submit message
back from the text editor.  There has been no interaction with p4
yet.  The self.isWindows check after your changes is just to
remove \r from newlines that many windows editors produce.

Now could be that you're worried not about this message, but
about failing in the later apply when it tries to put the
\n-terminated patch onto a workspace full of \r\n.  There
was a recent thread:

http://thread.gmane.org/gmane.comp.version-control.git/221664/focus=223625

suggesting that core.autocrlf was to blame.  Would be interesting
if this turns out to be your problem too.  Maybe we could look
for that and do something sensible.

The other thing to check is p4 client -o and see what LineEnd
setting exists for the backing p4 workspace.

-- Pete

 From aef963f0c45dea81f3e6f30d3b4185a0983ca4de Mon Sep 17 00:00:00 2001
 From: David Foster davidf...@gmail.com
 Date: Mon, 6 May 2013 10:50:01 -0700
 Subject: [PATCH] Compensate for Windows-style output from the p4 command on
  non-Windows systems.
 
 ---
  git-p4.py |   18 ++
  1 files changed, 18 insertions(+), 0 deletions(-)
 
 diff --git a/git-p4.py b/git-p4.py
 index 647f110..949d66d 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -1454,6 +1454,24 @@ class P4Submit(Command, P4UserMap):
  tmpFile = open(fileName, rb)
  message = tmpFile.read()
  tmpFile.close()
 +
 +# HACK: If Perforce spontaneously generates Windows-style output,
 +#   compensate by assuming the entire p4 command went into
 +#   Windows mode.
 +if separatorLine not in message:
 +print WARNING: Perforce has spontaneously decided to 
 generate Windows-style output. Compensating.
 +
 +# Assume that Perforce is now inexplicably operating in 
 Windows mode
 +self.isWindows = True
 +
 +# Retroactively rewrite expected output
 +submitTemplate = submitTemplate.replace(\n, \r\n)
 +separatorLine = separatorLine.replace(\n, \r\n)
 +newdiff = newdiff.replace(\n, \r\n)
 +
 +if separatorLine not in message:
 +raise ValueError('Confused. Thought Perforce went into 
 Windows mode but apparently something else is wrong.')
 +
  submitTemplate = message[:message.index(separatorLine)]
  if self.isWindows:
  submitTemplate = submitTemplate.replace(\r\n, \n)
 -- 
 1.7.7.5 (Apple Git-26)
 

--
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 grep parallelism question

[Pete Wyckoff]

torva...@linux-foundation.org wrote on Fri, 26 Apr 2013 13:31 -0700:
 Anyway, I think your patch is good if for no other reason that it
 allows this kind of testing, but at least for my machine, clearly the
 current default of eight threads is actually good enough. Maybe
 somebody with a very different machine might want to run the above
 script and see if how sensitive other machines are to this parameter..

NFS numbers behave as expected:  IO concurrency is key.

WARM
1 real 0m23.147s
2 real 0m13.913s
4 real 0m6.958s
8 real 0m4.104s
16 real 0m3.588s
32 real 0m3.212s
64 real 0m3.173s

COLD
1 real 1m36.969s
2 real 0m51.627s
4 real 0m32.994s
8 real 0m25.657s
16 real 0m21.260s
32 real 0m18.138s
64 real 0m17.265s

I am tempted to change the default locally from 8 to 32.

-- 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

Re: is git-p4 compatible with p4/linux?

[Pete Wyckoff]

dav...@gmail.com wrote on Sat, 20 Apr 2013 03:50 -0700:
 On Thu, Apr 18, 2013 at 5:09 PM, Pete Wyckoff p...@padd.com wrote:
  First issue
  ---
 
  git-p4 assumes the output of 'p4 print' adds a newline to the
  target.  To work around this, git-p4.py strips the last char from
  symlinks as shown in the following snippet:
 
  if type_base == symlink:
  git_mode = 12
  # p4 print on a symlink contains target\n; remove the newline
  data = ''.join(contents)
  contents = [data[:-1]]
 
 This line could be made more robust by changing it to:
 
 contents = [data.rstrip('\n')]
 
 That way it only strips off newlines if they exist, which essentially
 papers over these rogue depot files.
 Alternatively, it could use rstrip() with no arguments to cast a wider
 net and catch all whitespace.

I was tempted to do that, but it is possible to put \n and other
space characters in the target of symlinks.  It's unfortunate
that p4 always appears to tack on a newline itself.

We'll see if Alex comes up with a pattern that shows how he ended
up with the odd symlinks.

-- 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

Re: is git-p4 compatible with p4/linux?

[Pete Wyckoff]

a...@aivor.com wrote on Thu, 18 Apr 2013 20:34 -0500:
 Perhaps it is a configuration item on the server and/or client.   It seems we
 are running the same version of p4.  But just to be sure, check yours against
 mine:
 
 $ cksum $(which p4)
 3254530484 2420552 /usr/bin/p4
 
 If yours if different, can you email a copy of your p4 executable to me
 so I can check if it works differently than mine?

It is the same binary.  Assuming you're running p4d 2013.1 too?

$ p4 info
[..]
Server version: P4D/LINUX26X86_64/2013.1/610569 (2013/03/19)

I'm using these p4 client settings:

Options:noallwrite noclobber nocompress unlocked nomodtime normdir
SubmitOptions:  submitunchanged
LineEnd:unix

Running strace -vf -s 2000 p4 print -q symlink shows that the
newline is embedded in the response from p4d:

read(7, ...\0\0\0symlink-target\n\0func\0\23..., 4096) = 277

Also the file depot/symlink,v in the p4d depot area includes the
\n in the RCS file too, somewhat surprisingly:

$ cat depot/symlink,v
head 1.6;
[..]
@@
text
@symlink-target
@

 I will also check with coworkers here to see how their client behaves.
  This code only happens on utf16 files.  But running it by hand,
  I cannot reproduce the different behavior:
  
 $ p4 print -q //depot/f-ascii
 three
 line
 text
  
 $ p4 print -o - -q //depot/f-ascii
 three
 line
  
 $ ls ./-
 ls: cannot access ./-: No such file or directory
  
  I'm again confused.  Any hints you can give would be helpful.
 
 This second issue is a non-issue.  It seems -o - does send to
 stdout for files.  For symlinks, it creates a symlink named -.
 Example:
 
 $ ls -l pcre
 lrwxrwxrwx 1 atomlinson atomlinson 12 Apr 18 17:17 pcre - ../libs/pcre/
 $ ls -l ./- ./xxx
 /bin/ls: cannot access ./-: No such file or directory
 /bin/ls: cannot access ./xxx: No such file or directory
 $ p4 print -q -o - pcre
 $ p4 print -q -o xxx pcre
 $ ls -l ./- ./xxx
 lrwxrwxrwx 1 atomlinson atomlinson 12 Apr 18 20:25 ./- - ../libs/pcre/
 lrwxrwxrwx 1 atomlinson atomlinson 12 Apr 18 20:25 ./xxx - ../libs/pcre/

Me too.  That's annoying behavior, but not used by git-p4 fortunately.
The -o - option is only used for odd utf16 files where p4 print
generates invalid output.

-- 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

Re: git p4 submit failing

[Pete Wyckoff]

christopher.yee...@gmail.com wrote on Thu, 18 Apr 2013 11:24 -0500:
 The issue is caused by the line endings. I retested the problem with a
 different file and in this case, the error is caused by the line
 endings of the file checked out in the perforce workspace being
 win-style crlf, and the line endings of the file in the git repo being
 unix style lf. (In this scenario, I took out the .gitattributes,
 core.autocrlf was set to false and LineEnd was set to share)
 
 In this case, I checked out the file in perforce, ran dos2unix against
 it, and submitted that, then ran git p4 submit and it worked.
 
 I noticed that the error is caused by the git apply failing in the def
 applyCommit(self, id) function at lines 1296-1305.
 
 diffcmd = git format-patch -k --stdout \%s^\..\%s\ % (id, id)
 patchcmd = diffcmd +  | git apply 
 tryPatchCmd = patchcmd + --check -
 applyPatchCmd = patchcmd + --check --apply -
 patch_succeeded = True
 
 if os.system(tryPatchCmd) != 0:
 fixed_rcs_keywords = False
 patch_succeeded = False
 print Unfortunately applying the change failed!
 
 So most likely in git apply command, it can't find the changes because
 of the line endings being different between them. I couldn't find a
 parameter that would magically make it work. When I added --verbose to
 git apply the output only says:
 error: while searching for:
 and then the first lines of the first diff

That seems like exactly the correct diagnosis of the problem.
What to do about it, I'm not so sure.

We could suggest that people use the same line-ending conventions
in both git and p4 land.  This is easy if they are both lf.  But,
if crlf is preferred, do you know how to configure git to use
crlf line endings?  Does that fix it?  There's also the config
setting apply.ignorewhitespace; not sure if that would allow
the apply step to apply an lf-ending patch to the crlf-ending p4
workspace.

-- Pete

 Hello Simon,
 
 I have CCed you to alert you to the possible bug. Any assistance would
 be appreciated.
 
 
 On Sat, Apr 13, 2013 at 5:09 PM, Christopher Yee Mon
 christopher.yee...@gmail.com wrote:
  Yes this is the case.
 
  Many of the files have crlf endings.
 
  core.autocrlf was recently set to input. I can't remember the timeline
  exactly though, but in addition to this, I have a .gitattributes file
  with the default set to text=auto (* text=auto) and the php files set
  to text eol=lf (*.php text eol=lf) Also my perforce workspace's
  LineEnd setting is set to Share.
 
  I've experienced the behavior in both .php and .xml files though
 
  Before all of this started I had core.autocrlf set to false, and no
  .gitattributes file and perforce workspace's LineEnd was set to the
  default, but I got a conflict where the only difference was the line
  endings, so I changed things to the way they are now.
 
  Any recommendations? Should I change everything back the way it was?
 
  On Sat, Apr 13, 2013 at 5:51 PM, Pete Wyckoff p...@padd.com wrote:
  l...@diamand.org wrote on Thu, 11 Apr 2013 21:19 +0100:
  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.
 
  I had recently debugged a similar-looking problem
  where core.autocrlf was set to input.
 
  Christopher, if you have this set and/or the .xml files
  have ^M (CRLF) line endings, please let us know.
 
  -- Pete
 
 
  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

Re: git p4 submit failing

[Pete Wyckoff]

l...@diamand.org wrote on Thu, 11 Apr 2013 21:19 +0100:
 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.

I had recently debugged a similar-looking problem
where core.autocrlf was set to input.

Christopher, if you have this set and/or the .xml files
have ^M (CRLF) line endings, please let us know.

-- Pete

 
 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', 

Re: [PATCH] git-p4: support exclusively locked files

[Pete Wyckoff]

danny.tho...@blackboard.com wrote on Wed, 20 Mar 2013 07:33 -0400:
 Sounds good to me. I've got a couple of busy days coming up, but should
 have time this week.

Here's what I'm playing with for test cases, by the way.  The fix
you're working on is definitely part of it, but there are more
issues as well.  I'll address them once you've taken care of the
opened/fstat issue.  Thanks,

-- Pete

--- 8 ---

From c6691126ae75c364763ab4d774c75045285b8ddd Mon Sep 17 00:00:00 2001
From: Pete Wyckoff p...@padd.com
Date: Sun, 17 Mar 2013 16:05:07 -0400
Subject: [PATCH] git p4 test: examine behavior with locked (+l) files

The p4 server can enforce file locking, so that only one user
can edit a file at a time.  Git p4 is unable to submit changes
to locked files.  Currently it exits poorly.  Ideally it would
notice the locked condition and clean up nicely.
---
 t/t9816-git-p4-locked.sh | 145 +++
 1 file changed, 145 insertions(+)
 create mode 100755 t/t9816-git-p4-locked.sh

diff --git a/t/t9816-git-p4-locked.sh b/t/t9816-git-p4-locked.sh
new file mode 100755
index 000..e71e543
--- /dev/null
+++ b/t/t9816-git-p4-locked.sh
@@ -0,0 +1,145 @@
+#!/bin/sh
+
+test_description='git p4 locked file behavior'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+   start_p4d
+'
+
+# See
+# 
http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.html#1088563
+# for suggestions on how to configure sitewide pessimistic locking
+# where only one person can have a file open for edit at a time.
+test_expect_success 'init depot' '
+   (
+   cd $cli 
+   echo TypeMap: +l //depot/... | p4 typemap -i 
+   echo file1 file1 
+   p4 add file1 
+   p4 submit -d add file1
+   )
+'
+
+test_expect_success 'edit with lock not taken' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line2 file1 
+   git add file1 
+   git commit -m line2 in file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit
+   )
+'
+
+test_expect_failure 'add with lock not taken' '
+   test_when_finished cleanup_git 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line1 add-lock-not-taken 
+   git add file2 
+   git commit -m add add-lock-not-taken 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+lock_in_another_client() {
+   # build a different client
+   cli2=$TRASH_DIRECTORY/cli2 
+   mkdir -p $cli2 
+   test_when_finished p4 client -f -d client2  rm -rf \$cli2\ 
+   (
+   cd $cli2 
+   P4CLIENT=client2 
+   cli=$cli2 
+   client_view //depot/... //client2/... 
+   p4 sync 
+   p4 open file1
+   )
+}
+
+test_expect_failure 'edit with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   echo line3 file1 
+   git add file1 
+   git commit -m line3 in file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'delete with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   git rm file1 
+   git commit -m delete file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'chmod with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 sync -f file1 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   chmod +x file1 
+   git add file1 
+   git commit -m chmod +x file1 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'copy with lock taken' '
+   lock_in_another_client 
+   test_when_finished cleanup_git 
+   test_when_finished cd \$cli\  p4 revert file2  rm -f file2 
+   git p4 clone --dest=$git //depot 
+   (
+   cd $git 
+   cp file1 file2 
+   git add file2 
+   git commit -m cp file1 to file2 
+   git config git-p4.skipSubmitEdit true 
+   git config git-p4.detectCopies true 
+   git p4 submit --verbose
+   )
+'
+
+test_expect_failure 'move with lock taken

Re: [PATCH] git-p4: support exclusively locked files

[Pete Wyckoff]

danny.tho...@blackboard.com wrote on Mon, 18 Mar 2013 09:26 -0400:
 Interesting. 'Implementing sitewide pessimistic locking with p4 typemap',
 http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.htm
 l seems to suggest this is all that's needed. I believe we're using the
 default configuration, the exclusive lock on all files behaviour was
 researching the exclusive locking problem,
 http://ericlathrop.com/2012/12/how-to-set-up-git-p4-in-windows/, so I
 thought I'd mention it.
 
 You might be onto something w/ fstat, it doesn't include the exclusive
 indicator:
 
 ... type binary+l
 
 Latest P4 client, and fairly recent server build:
 
 P4/DARWIN90X86_64/2012.2/536738 (2012/10/16)
 P4D/LINUX26X86_64/2012.2/538478 (2012/10/16)

Great, thanks for the pointer and explanation.  Do you want to
reroll your patch to use fstat?  I'll work on the tests, and
also look into potential failure modes of git p4 submit when somebody
else has the exclusive file open.

-- Pete

 On 17/03/2013 20:04, Pete Wyckoff p...@padd.com wrote:
 
 danny.tho...@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:
  By default, newly added binary files are exclusively locked by Perforce:
 
  'add default change (binary+l) *exclusive*'
 
  This results in a 'Could not determine file type' error as the regex
  expects
  the line to end after the file type matching group. Some repositories
 are
  also configured to always require exclusive locks, so may be a problem
 for
  all revisions in some cases.
 
 Can you explain how to configure p4d to default everything to
 binary+l?  Also, what version are you using (p4 info)?  I'm
 trying to write a test case for this.
 
 I did find a way to play with typemap to get +l, as:
 
 ( p4 typemap -o ; printf \tbinary+l\t//.../bash* ) | p4 typemap -i
 
 With this, the 2011.1 here just says:
 0
 tic-git-test$ p4 opened bash
 //depot/bash#1 - add default change (binary+l)
 
 I've not been able to make it say  *exclusive* too.
 
   result = p4_read_pipe([opened, wildcard_encode(file)])
  -match = re.match(.*\((.+)\)\r?$, result)
  +match = re.match(.*\((.+)\)(?:.+)?\r?$, result)
 
 I think this whole function would be less brittle
 using p4's -G output, like:
 
 d = p4Cmd([fstat, -T, type, wildcard_encode(file)])
 # some error checking
 return d['type']
 
 But I'm curious if your p4d includes  *exclusive* in the
 type there too, in which case we'll have to strip it.
 
 Thanks for starting the patch on this.  It's good if we can keep
 others from running into the same problem.
--
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: support exclusively locked files

[Pete Wyckoff]

danny.tho...@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:
 By default, newly added binary files are exclusively locked by Perforce:
 
 'add default change (binary+l) *exclusive*'
 
 This results in a 'Could not determine file type' error as the regex
 expects
 the line to end after the file type matching group. Some repositories are
 also configured to always require exclusive locks, so may be a problem for
 all revisions in some cases.

Can you explain how to configure p4d to default everything to
binary+l?  Also, what version are you using (p4 info)?  I'm
trying to write a test case for this.

I did find a way to play with typemap to get +l, as:

( p4 typemap -o ; printf \tbinary+l\t//.../bash* ) | p4 typemap -i

With this, the 2011.1 here just says:

tic-git-test$ p4 opened bash
//depot/bash#1 - add default change (binary+l)

I've not been able to make it say  *exclusive* too.

  result = p4_read_pipe([opened, wildcard_encode(file)])
 -match = re.match(.*\((.+)\)\r?$, result)
 +match = re.match(.*\((.+)\)(?:.+)?\r?$, result)

I think this whole function would be less brittle
using p4's -G output, like:

d = p4Cmd([fstat, -T, type, wildcard_encode(file)])
# some error checking
return d['type']

But I'm curious if your p4d includes  *exclusive* in the
type there too, in which case we'll have to strip it.

Thanks for starting the patch on this.  It's good if we can keep
others from running into the same problem.

-- 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

Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline

[Pete Wyckoff]

gits...@pobox.com wrote on Wed, 13 Mar 2013 12:38 -0700:
 Karsten Blees karsten.bl...@gmail.com writes:
 
  However, AFAIK inotify doesn't work recursively, so the daemon
  would at least have to track the directory structure to be able to
  register / unregister inotify handlers as directories come and go.
 
 Yes, and you would need one inotify per directory but you do not
 have an infinite supply of outstanding inotify watch (wasn't the
 limit like 8k per a single uid or something?), so the daemon must be
 prepared to say I'll watch this, that and that directories, but the
 consumers should check other directories themselves.

fanotify is an option here too; it can watch an entire file
system.

-- 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

[PATCH v2 0/3] fix git-p4 client root symlink problems

[Pete Wyckoff]

Update from v1:

* add SYMLINKS prerequisite to the new symlink test

Thanks Hannes.


Miklós pointed out in

http://thread.gmane.org/gmane.comp.version-control.git/214915

that when the p4 client root included a symlink, bad things
happen.  It is fixable, but inconvenient, to use an absolute path
in one's p4 client.  It's not too hard to be smarter about this
in git-p4.

Thanks to Miklós for the patch, and to John for the style
suggestions.  I wrote a couple of tests to make sure this part
doesn't break again.

This is maybe a bug introduced by bf1d68f (git-p4: use absolute
directory for PWD env var, 2011-12-09), but that's so long ago
that I don't think this is a candidate for maint.

-- Pete


Miklós Fazekas (1):
  git p4: avoid expanding client paths in chdir

Pete Wyckoff (2):
  git p4 test: make sure P4CONFIG relative path works
  git p4 test: should honor symlink in p4 client root

 git-p4.py   | 29 ++---
 t/t9808-git-p4-chdir.sh | 41 +
 2 files changed, 63 insertions(+), 7 deletions(-)

-- 
1.8.2.rc2.65.g92f3e2d

--
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 test: make sure P4CONFIG relative path works

[Pete Wyckoff]

This adds a test for the fix in bf1d68f (git-p4: use absolute
directory for PWD env var, 2011-12-09).  It is necessary to
set PWD to an absolute path so that p4 can find files referenced
by non-absolute paths, like the value of the P4CONFIG environment
variable.

P4 does not open files directly; it builds a path by prepending
the contents of the PWD environment variable.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9808-git-p4-chdir.sh | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index dc92e60..55c5e36 100755
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -42,6 +42,20 @@ test_expect_success 'P4CONFIG and relative dir clone' '
)
 '
 
+# Common setup using .p4config to set P4CLIENT and P4PORT breaks
+# if clone destination is relative.  Make sure that chdir() expands
+# the relative path in --dest to absolute.
+test_expect_success 'p4 client root would be relative due to clone --dest' '
+   test_when_finished cleanup_git 
+   (
+   echo P4PORT=$P4PORT git/.p4config 
+   P4CONFIG=.p4config 
+   export P4CONFIG 
+   unset P4PORT 
+   git p4 clone --dest=git //depot
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.2.rc2.65.g92f3e2d

--
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 test: should honor symlink in p4 client root

[Pete Wyckoff]

This test fails when the p4 client root includes
a symlink.  It complains:

Path /vol/bar/projects/foo/... is not under client root /p/foo

and dumps a traceback.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9808-git-p4-chdir.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index 55c5e36..4773296 100755
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -56,6 +56,33 @@ test_expect_success 'p4 client root would be relative due to 
clone --dest' '
)
 '
 
+# When the p4 client Root is a symlink, make sure chdir() does not use
+# getcwd() to convert it to a physical path.
+test_expect_failure SYMLINKS 'p4 client root symlink should stay symbolic' '
+   physical=$TRASH_DIRECTORY/physical 
+   symbolic=$TRASH_DIRECTORY/symbolic 
+   test_when_finished rm -rf \$physical\ 
+   test_when_finished rm \$symbolic\ 
+   mkdir -p $physical 
+   ln -s $physical $symbolic 
+   test_when_finished cleanup_git 
+   (
+   P4CLIENT=client-sym 
+   p4 client -i -EOF 
+   Client: $P4CLIENT
+   Description: $P4CLIENT
+   Root: $symbolic
+   LineEnd: unix
+   View: //depot/... //$P4CLIENT/...
+   EOF
+   git p4 clone --dest=$git //depot 
+   cd $git 
+   test_commit file2 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.2.rc2.65.g92f3e2d

--
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: avoid expanding client paths in chdir

[Pete Wyckoff]

From: Miklós Fazekas mfaze...@szemafor.com

The generic chdir() helper sets the PWD environment
variable, as that is what is used by p4 to know its
current working directory.  Normally the shell would
do this, but in git-p4, we must do it by hand.

However, when the path contains a symbolic link,
os.getcwd() will return the physical location.  If the
p4 client specification includes symlinks, setting PWD
to the physical location causes p4 to think it is not
inside the client workspace.  It complains, e.g.

Path /vol/bar/projects/foo/... is not under client root /p/foo

One workaround is to use AltRoots in the p4 client specification,
but it is cleaner to handle it directly in git-p4.

Other uses of chdir still require setting PWD to an
absolute path so p4 features like P4CONFIG work.  See
bf1d68f (git-p4: use absolute directory for PWD env
var, 2011-12-09).

[ pw: tweak patch and commit message ]

Thanks-to: John Keeping j...@keeping.me.uk
Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py   | 29 ++---
 t/t9808-git-p4-chdir.sh |  2 +-
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 647f110..7288c0b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -79,12 +79,27 @@ def p4_build_cmd(cmd):
 real_cmd += cmd
 return real_cmd
 
-def chdir(dir):
-# P4 uses the PWD environment variable rather than getcwd(). Since we're
-# not using the shell, we have to set it ourselves.  This path could
-# be relative, so go there first, then figure out where we ended up.
-os.chdir(dir)
-os.environ['PWD'] = os.getcwd()
+def chdir(path, is_client_path=False):
+Do chdir to the given path, and set the PWD environment
+   variable for use by P4.  It does not look at getcwd() output.
+   Since we're not using the shell, it is necessary to set the
+   PWD environment variable explicitly.
+   
+   Normally, expand the path to force it to be absolute.  This
+   addresses the use of relative path names inside P4 settings,
+   e.g. P4CONFIG=.p4config.  P4 does not simply open the filename
+   as given; it looks for .p4config using PWD.
+
+   If is_client_path, the path was handed to us directly by p4,
+   and may be a symbolic link.  Do not call os.getcwd() in this
+   case, because it will cause p4 to think that PWD is not inside
+   the client path.
+   
+
+os.chdir(path)
+if not is_client_path:
+path = os.getcwd()
+os.environ['PWD'] = path
 
 def die(msg):
 if verbose:
@@ -1624,7 +1639,7 @@ class P4Submit(Command, P4UserMap):
 new_client_dir = True
 os.makedirs(self.clientPath)
 
-chdir(self.clientPath)
+chdir(self.clientPath, is_client_path=True)
 if self.dry_run:
 print Would synchronize p4 checkout in %s % self.clientPath
 else:
diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index 4773296..11d2b51 100755
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -58,7 +58,7 @@ test_expect_success 'p4 client root would be relative due to 
clone --dest' '
 
 # When the p4 client Root is a symlink, make sure chdir() does not use
 # getcwd() to convert it to a physical path.
-test_expect_failure SYMLINKS 'p4 client root symlink should stay symbolic' '
+test_expect_success SYMLINKS 'p4 client root symlink should stay symbolic' '
physical=$TRASH_DIRECTORY/physical 
symbolic=$TRASH_DIRECTORY/symbolic 
test_when_finished rm -rf \$physical\ 
-- 
1.8.2.rc2.65.g92f3e2d

--
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 0/3] fix git-p4 client root symlink problems

[Pete Wyckoff]

Miklós pointed out in

http://thread.gmane.org/gmane.comp.version-control.git/214915

that when the p4 client root included a symlink, bad things
happen.  It is fixable, but inconvenient, to use an absolute path
in one's p4 client.  It's not too hard to be smarter about this
in git-p4.

Thanks to Miklós for the patch, and to John for the style
suggestions.  I wrote a couple of tests to make sure this part
doesn't break again.

This is maybe a bug introduced by bf1d68f (git-p4: use absolute
directory for PWD env var, 2011-12-09), but that's so long ago
that I don't think this is a candidate for maint.

-- Pete

Miklós Fazekas (1):
  git p4: avoid expanding client paths in chdir

Pete Wyckoff (2):
  git p4 test: make sure P4CONFIG relative path works
  git p4 test: should honor symlink in p4 client root

 git-p4.py   | 29 ++---
 t/t9808-git-p4-chdir.sh | 41 +
 2 files changed, 63 insertions(+), 7 deletions(-)

-- 
1.8.2.rc2.64.g8335025

--
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 1/3] git p4 test: make sure P4CONFIG relative path works

[Pete Wyckoff]

This adds a test for the fix in bf1d68f (git-p4: use absolute
directory for PWD env var, 2011-12-09).  It is necessary to
set PWD to an absolute path so that p4 can find files referenced
by non-absolute paths, like the value of the P4CONFIG environment
variable.

P4 does not open files directly; it builds a path by prepending
the contents of the PWD environment variable.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9808-git-p4-chdir.sh | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index dc92e60..55c5e36 100755
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -42,6 +42,20 @@ test_expect_success 'P4CONFIG and relative dir clone' '
)
 '
 
+# Common setup using .p4config to set P4CLIENT and P4PORT breaks
+# if clone destination is relative.  Make sure that chdir() expands
+# the relative path in --dest to absolute.
+test_expect_success 'p4 client root would be relative due to clone --dest' '
+   test_when_finished cleanup_git 
+   (
+   echo P4PORT=$P4PORT git/.p4config 
+   P4CONFIG=.p4config 
+   export P4CONFIG 
+   unset P4PORT 
+   git p4 clone --dest=git //depot
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.2.rc2.64.g8335025

--
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 2/3] git p4 test: should honor symlink in p4 client root

[Pete Wyckoff]

This test fails when the p4 client root includes
a symlink.  It complains:

Path /vol/bar/projects/foo/... is not under client root /p/foo

and dumps a traceback.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/t9808-git-p4-chdir.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index 55c5e36..af8bd8a 100755
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -56,6 +56,33 @@ test_expect_success 'p4 client root would be relative due to 
clone --dest' '
)
 '
 
+# When the p4 client Root is a symlink, make sure chdir() does not use
+# getcwd() to convert it to a physical path.
+test_expect_failure 'p4 client root symlink should stay symbolic' '
+   physical=$TRASH_DIRECTORY/physical 
+   symbolic=$TRASH_DIRECTORY/symbolic 
+   test_when_finished rm -rf \$physical\ 
+   test_when_finished rm \$symbolic\ 
+   mkdir -p $physical 
+   ln -s $physical $symbolic 
+   test_when_finished cleanup_git 
+   (
+   P4CLIENT=client-sym 
+   p4 client -i -EOF 
+   Client: $P4CLIENT
+   Description: $P4CLIENT
+   Root: $symbolic
+   LineEnd: unix
+   View: //depot/... //$P4CLIENT/...
+   EOF
+   git p4 clone --dest=$git //depot 
+   cd $git 
+   test_commit file2 
+   git config git-p4.skipSubmitEdit true 
+   git p4 submit
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
1.8.2.rc2.64.g8335025

--
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 3/3] git p4: avoid expanding client paths in chdir

[Pete Wyckoff]

From: Miklós Fazekas mfaze...@szemafor.com

The generic chdir() helper sets the PWD environment
variable, as that is what is used by p4 to know its
current working directory.  Normally the shell would
do this, but in git-p4, we must do it by hand.

However, when the path contains a symbolic link,
os.getcwd() will return the physical location.  If the
p4 client specification includes symlinks, setting PWD
to the physical location causes p4 to think it is not
inside the client workspace.  It complains, e.g.

Path /vol/bar/projects/foo/... is not under client root /p/foo

One workaround is to use AltRoots in the p4 client specification,
but it is cleaner to handle it directly in git-p4.

Other uses of chdir still require setting PWD to an
absolute path so p4 features like P4CONFIG work.  See
bf1d68f (git-p4: use absolute directory for PWD env
var, 2011-12-09).

[ pw: tweak patch and commit message ]

Thanks-to: John Keeping j...@keeping.me.uk
Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py   | 29 ++---
 t/t9808-git-p4-chdir.sh |  2 +-
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 647f110..7288c0b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -79,12 +79,27 @@ def p4_build_cmd(cmd):
 real_cmd += cmd
 return real_cmd
 
-def chdir(dir):
-# P4 uses the PWD environment variable rather than getcwd(). Since we're
-# not using the shell, we have to set it ourselves.  This path could
-# be relative, so go there first, then figure out where we ended up.
-os.chdir(dir)
-os.environ['PWD'] = os.getcwd()
+def chdir(path, is_client_path=False):
+Do chdir to the given path, and set the PWD environment
+   variable for use by P4.  It does not look at getcwd() output.
+   Since we're not using the shell, it is necessary to set the
+   PWD environment variable explicitly.
+   
+   Normally, expand the path to force it to be absolute.  This
+   addresses the use of relative path names inside P4 settings,
+   e.g. P4CONFIG=.p4config.  P4 does not simply open the filename
+   as given; it looks for .p4config using PWD.
+
+   If is_client_path, the path was handed to us directly by p4,
+   and may be a symbolic link.  Do not call os.getcwd() in this
+   case, because it will cause p4 to think that PWD is not inside
+   the client path.
+   
+
+os.chdir(path)
+if not is_client_path:
+path = os.getcwd()
+os.environ['PWD'] = path
 
 def die(msg):
 if verbose:
@@ -1624,7 +1639,7 @@ class P4Submit(Command, P4UserMap):
 new_client_dir = True
 os.makedirs(self.clientPath)
 
-chdir(self.clientPath)
+chdir(self.clientPath, is_client_path=True)
 if self.dry_run:
 print Would synchronize p4 checkout in %s % self.clientPath
 else:
diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh
index af8bd8a..09b2cc4 100755
--- a/t/t9808-git-p4-chdir.sh
+++ b/t/t9808-git-p4-chdir.sh
@@ -58,7 +58,7 @@ test_expect_success 'p4 client root would be relative due to 
clone --dest' '
 
 # When the p4 client Root is a symlink, make sure chdir() does not use
 # getcwd() to convert it to a physical path.
-test_expect_failure 'p4 client root symlink should stay symbolic' '
+test_expect_success 'p4 client root symlink should stay symbolic' '
physical=$TRASH_DIRECTORY/physical 
symbolic=$TRASH_DIRECTORY/symbolic 
test_when_finished rm -rf \$physical\ 
-- 
1.8.2.rc2.64.g8335025

--
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] Incorrect/misleading error when `git rev-list --objects --all` hits the max open files limit

[Pete Wyckoff]

rab...@rabbit.us wrote on Mon, 04 Mar 2013 10:29 +1100:
 I was tinkering with a massive git repository (actually a bup 
 repository, but it is a standard valid git repo underneath). While 
 validating that a repack ran succesfully I executed the command:
 
  git rev-list --objects --all  rev.list
 
 And got back this:
 
 error: packfile 
 ./objects/pack/pack-d9808b7515419737806d0c621a0a1910f71c5cba.pack cannot be 
 accessed
 fatal: missing blob object '27a8cf44da85b958aef2b5074931e7913e08ae95'
 
 Several hours later after successful fsck, and after chasing down trees 
 blobs etc, I realized that the problem is too many open files. The hint 
 came from ls-tree which lists the correct error (among a lot of spurious 
 junk):
 
  git ls-tree -r c636a5f51d4e  /dev/null
 
  error: packfile 
 ./objects/pack/pack-d9808b7515419737806d0c621a0a1910f71c5cba.pack cannot be 
 accessed
  error: packfile 
 ./objects/pack/pack-841e375f5e6c793a52fd1a3a2aea0b76219c4cdd.pack cannot be 
 accessed
  error: packfile 
 ./objects/pack/pack-e67d9bf75e0840fc6113170b314d2d5a32cbb45a.pack cannot be 
 accessed
  error: packfile 
 ./objects/pack/pack-b8fd8f083461c391fe6ec396840c328620d912e2.pack cannot be 
 accessed
  error: packfile 
 ./objects/pack/pack-d9808b7515419737806d0c621a0a1910f71c5cba.pack cannot be 
 accessed
  error: packfile 
 ./objects/pack/pack-804e0fadf56e2a165c157ef257620369adeea595.pack cannot be 
 accessed
  error: unable to open object pack directory: ./objects/pack: Too many open 
 files
  error: packfile 
 ./objects/pack/pack-804e0fadf56e2a165c157ef257620369adeea595.pack cannot be 
 accessed
  error: Could not read 32a050cb7e54a1e817d135d25ab251480e8d9e3c
 
 Failure to report the correct message verified with git 1.7.2.5 and 
 1.8.2 (debian testing and experimental).
 
 I hope this is sufficient description to address the underlying issue. I 
 will keep the un-repacked many files repo around just in case you need 
 more info/testing (though lowering the ulimit works equally well on 
 normal-size repos).

I've run into this twice:

1.  user with restrictive ulimit on #files.
2.  forgot to do git gc after regular fast-imports

Here's one thread:

http://thread.gmane.org/gmane.comp.version-control.git/179231/focus=179292

I've got a patch in cold storage.  It's not beautiful because
there are too many paths that can end up hitting EMFILE.  I'll
dust it off and see if it is worth considering.

-- 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

Re: Suggested improvements to the git-p4 documentation (branch-related)

[Pete Wyckoff]

gits...@pobox.com wrote on Fri, 22 Feb 2013 16:42 -0800:
 Olivier Delalleau sh...@keba.be writes:
 
  2013/1/5 Pete Wyckoff p...@padd.com:
  sh...@keba.be wrote on Thu, 03 Jan 2013 15:58 -0500:
  ...
  Please do feel welcome to to rearrange or expand the
  documentation so it makes more sense, if you are so inspired.
 
  I'm afraid I'm not familiar enough with git documentation to dig into
  it myself, but anyway that's about what I had for now. I'll send more
  comments to the mailing list if I have more suggestions in the future.
 
  Thanks for a great tool! :)
 
 Did anything come out of this thread?  If neither of you two are
 inclined to conclude the discussion with a final patch, I'd
 appreciate anybody else who does the honors ;-)
 
 We'll be in deep pre-release freeze for a few weeks, so there is no
 need to rush.

Two of Olivier's suggestions were best classified as code, not
documentation, bugs.  I finished off some ongoing work that fixed
those along the way.  The third led to a fix to the
documentation, 182edef (git p4 doc: fix branch detection example,
2013-01-14), that I added as part of that series.

All of it is in master now, via 801cbd7 (Merge branch
'pw/p4-branch-fixes', 2013-01-21).

I should have commented on this thread too.  Thanks for following
up!

-- 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

Re: [PATCH v2] read_directory: avoid invoking exclude machinery on tracked files

[Pete Wyckoff]

pclo...@gmail.com wrote on Sun, 17 Feb 2013 11:39 +0700:
 On Sun, Feb 17, 2013 at 1:11 AM, Pete Wyckoff p...@padd.com wrote:
  pclo...@gmail.com wrote on Sat, 16 Feb 2013 14:17 +0700:
  Finally some numbers (best of 20 runs) that shows why it's worth all
  the hassle:
 
  git status   | webkit linux-2.6 libreoffice-core gentoo-x86
  -+--
  before   | 1.097s0.208s   0.399s 0.539s
  after| 0.736s0.159s   0.248s 0.501s
  nr. patterns |89   376   19  0
  nr. tracked  |   182k   40k  63k   101k
 
  Thanks for this work.  I repeated some of the tests across NFS,
  where I'd expect to see bigger differences.
 
 This is about reducing CPU processing time, not I/O time. So no bigger
 differences is expected. I/O time can be reduced with inotify, or fam
 in nfs case because inotify does not support nfs.

Numbers from the last mail were core.preloadindex=true.  Here's
time output from average runs:

stock = 0m2.28s user 0m4.18s sys 0m11.28s elapsed 57.39 %CPU
duy   = 0m1.25s user 0m4.43s sys 0m7.45s elapsed 76.41 %CPU

With this huge repo, preloadindex may be stressing directory
cache behavior on the NFS server or client.  Your patch helps
both CPU and wait time by avoiding the 6000-odd open() of
non-existent .gitignore.

With core.preloadindex=false, it's a 1 sec speedup, all from CPU:

stock = 0m2.18s user 0m1.59s sys 0m7.78s elapsed 48.45 %CPU
duy   = 0m1.17s user 0m1.63s sys 0m6.91s elapsed 40.59 %CPU


-- 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

Re: [PATCH v2] read_directory: avoid invoking exclude machinery on tracked files

[Pete Wyckoff]

pclo...@gmail.com wrote on Sat, 16 Feb 2013 14:17 +0700:
 Finally some numbers (best of 20 runs) that shows why it's worth all
 the hassle:
 
 git status   | webkit linux-2.6 libreoffice-core gentoo-x86
 -+--
 before   | 1.097s0.208s   0.399s 0.539s
 after| 0.736s0.159s   0.248s 0.501s
 nr. patterns |89   376   19  0
 nr. tracked  |   182k   40k  63k   101k

Thanks for this work.  I repeated some of the tests across NFS,
where I'd expect to see bigger differences.  Best of 20 values
reported in min 

webkit
Stock min  9.61 avg 11.61 +/- 1.35 max 14.26
Duy   min  6.91 avg  7.67 +/- 0.46 max  8.71

linux
Stock min  2.27 avg  3.16 +/- 0.56 max  4.49
Duy   min  2.04 avg  3.12 +/- 0.69 max  4.87

libreoffice-core
Stock min  4.56 avg  5.79 +/- 0.79 max  7.08
Duy   min  3.96 avg  5.25 +/- 0.95 max  6.95

Similar 30%-ish speedup on webkit.  And an absolute gain
of 2.7 seconds is quite nice.

-- 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

Re: [RFC/PATCH v2] CodingGuidelines: add Python coding guidelines

[Pete Wyckoff]

j...@keeping.me.uk wrote on Fri, 01 Feb 2013 11:16 +:
 On Fri, Feb 01, 2013 at 09:39:39AM +0100, Michael Haggerty wrote:
  On 01/30/2013 09:31 PM, John Keeping wrote:
   On Wed, Jan 30, 2013 at 11:05:10AM +0100, Michael Haggerty wrote:
   [...] maybe we should establish a small Python library of
   compatibility utilities (like a small six). [...]
   But I haven't had time to think of where to put such a library, how to
   install it, etc.
   
   If we want to go that route, I think restructuring the
   git_remote_helpers directory and re-using its infrastructure for
   installing the Git Python modules would be the way to go.  The
   directory structure would become something like this:
   
   git/
   `-- python/
   |-- Makefile# existing file pulled out of git_remote_helpers
   |--  some new utility library 
   |-- git_remote_helpers
   |   |-- __init__.py
   |   |-- git
   |   |   |-- __init__.py
   |   |   |-- exporter.py
   |   |   |-- git.py
   |   |   |-- importer.py
   |   |   |-- non_local.py
   |   |   `-- repo.py
   |   `-- util.py
   |-- setup.cfg   # existing file pulled out of git_remote_helpers
   `-- setup.py# existing file pulled out of git_remote_helpers
   
   
   It looks like the GitPython project[1] as already taken the git module
   name, so perhaps we should use git_core if we do introduce a new
   module.
   
   [1] http://pypi.python.org/pypi/GitPython
  
  This sounds reasonable.  But not all Python code will go under the
  python subdirectory, right?  For example, I am working on a Python
  script that fits thematically under contrib/hooks.
 
 I was thinking of it as analagous with the perl directory that
 currently exists.  So the python directory will contain library code
 but scripts can live wherever is most appropriate.
 
 One way of looking at it is: could the user want to have this installed
 for all available versions of Python?  For a script, the answer is no
 because they will call it and it will just run.  For libraries, you want
 them to be available with whatever Python interpreter you happen to be
 running (assuming that it is a version supported by the library).
 
  OTOH (I'm thinking aloud here) it is probably a bad idea for a hook
  script to depend on a Python module that is part of git itself.  Doing
  so would make the hook script depend on a particular version of git (or
  at least a version with a compatible Python module).  But users might be
  reluctant to upgrade git just to install a hook script.
 
 I don't think such a dependency is a bad idea in the longer term.  If a
 Git Python library is developed, then at some point most people who
 have Git installed will have some version of that library - it becomes a
 case of perhaps wanting to limit yourself to some subset of the library
 rather than just not using it.
 
 In fact, git_remote_helpers has been available since Git 1.7.0 and
 contains a lot of functionality that is more generic than its name
 suggests.

This library idea would be a great help; there are 100-odd calls
to git in git-p4, and we've had to deal with getting the arguments
and parsing correct.  I'd happily switch to using git_core.

Probably some elements of GitPython can be used too.  I'm not so
interested in the raw database manipulation, but the command
wrappers look reasonable.

-- 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

Re: [PATCH] git p4: chdir resolves symlinks only for relative paths

[Pete Wyckoff]

mfaze...@szemafor.com wrote on Tue, 29 Jan 2013 09:37 +0100:
 If a p4 client is configured to /p/foo which is a symlink
 to /vol/bar/projects/foo, then resolving symlink, which
 is done by git-p4's chdir will confuse p4: Path
 /vol/bar/projects/foo/... is not under client root /p/foo
 While AltRoots in p4 client specification can be used as a
 workaround on p4 side, git-p4 should not resolve symlinks
 in client paths.
 chdir(dir) uses os.getcwd() after os.chdir(dir) to resolve
 relative paths, but as a side effect it resolves symlinks
 too. Now it checks if the dir is relative before resolving.
 
 Signed-off-by: Miklós Fazekas mfaze...@szemafor.com
 ---
  git-p4.py |5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/git-p4.py b/git-p4.py
 index 2da5649..5d74649 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -64,7 +64,10 @@ def chdir(dir):
  # not using the shell, we have to set it ourselves.  This path could
  # be relative, so go there first, then figure out where we ended up.
  os.chdir(dir)
 -os.environ['PWD'] = os.getcwd()
 +if os.path.isabs(dir):
 +os.environ['PWD'] = dir
 +else:
 +os.environ['PWD'] = os.getcwd()
 
  def die(msg):
  if verbose:

Thanks, this is indeed a bug and I have reproduced it with a test
case.  Your patch works, but I think it would be better to
separate the callers of chdir():  those that know they are
cd-ing to a path from a p4 client, and everybody else.  The former
should not use os.getcwd(), as you show.

I'll whip something up soon, unless you beat me to it.

-- 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

Re: What's cooking in git.git (Jan 2013, #10; Sun, 27)

[Pete Wyckoff]

gits...@pobox.com wrote on Sun, 27 Jan 2013 22:45 -0800:
 * pw/git-p4-on-cygwin (2013-01-26) 21 commits
  - git p4: introduce gitConfigBool
  - git p4: avoid shell when calling git config
  - git p4: avoid shell when invoking git config --get-all
  - git p4: avoid shell when invoking git rev-list
  - git p4: avoid shell when mapping users
  - git p4: disable read-only attribute before deleting
  - git p4 test: use test_chmod for cygwin
  - git p4: cygwin p4 client does not mark read-only
  - git p4 test: avoid wildcard * in windows
  - git p4 test: use LineEnd unix in windows tests too
  - git p4 test: newline handling
  - git p4: scrub crlf for utf16 files on windows
  - git p4: remove unreachable windows \r\n conversion code
  - git p4 test: translate windows paths for cygwin
  - git p4 test: start p4d inside its db dir
  - git p4 test: use client_view in t9806
  - git p4 test: avoid loop in client_view
  - git p4 test: use client_view to build the initial client
  - git p4: generate better error message for bad depot path
  - git p4: remove unused imports
  - git p4: temp branch name should use / even on windows
 
  Improve git p4 on Cygwin.  The cover letter said it is not yet
  ready for full Windows support so I won't move this to 'next' until
  told by the author (the area maintainer) otherwise.

The series is ready as is to support Cygwin platforms, and
thus useful to people who would use git on windows via cygwin.

Future work will be to add support for Msysgit.  That work
will need much of the changes in this Cygwin series as well.
It is more complicated since there's no native python for
Msysgit (yet).

I think the Cygwin changes should go in now.

-- 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

Re: [PATCH 1/2] git-p4.py: support Python 2.5

[Pete Wyckoff]

draf...@gmail.com wrote on Fri, 25 Jan 2013 12:44 -0800:
 Python 2.5 and older do not accept None as the first argument to
 translate() and complain with:
 
TypeError: expected a character buffer object
 
 Satisfy this older python by calling maketrans() to generate an empty
 translation table and supplying that to translate().
 
 This allows git-p4 to be used with Python 2.5.

This was a lot easier than I imagined!

  def wildcard_present(path):
 -return path.translate(None, *#@%) != path
 +from string import maketrans
 +return path.translate(maketrans(,), *#@%) != path

translate() was a bit too subtle already.  Could you try
something like this instead?

m = re.search([*#@%], path)
return m is not None

I think that'll work everywhere and not force people to look
up how translate and maketrans work.

-- 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

Re: [PATCH 2/2] git-p4.py: support Python 2.4

[Pete Wyckoff]

draf...@gmail.com wrote on Fri, 25 Jan 2013 12:44 -0800:
 Python 2.4 lacks the following features:
 
subprocess.check_call
struct.pack_into
 
 Take a cue from 460d1026 and provide an implementation of the
 CalledProcessError exception.  Then replace the calls to
 subproccess.check_call with calls to subprocess.call that check the return
 status and raise a CalledProcessError exception if necessary.
 
 The struct.pack_into in t/9802 can be converted into a single struct.pack
 call which is available in Python 2.4.

Excellent.  Should have used struct.pack() from the get-go.

Acked-by: Pete Wyckoff p...@padd.com

 diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
 index 21924df..be299dc 100755
 --- a/t/t9802-git-p4-filetype.sh
 +++ b/t/t9802-git-p4-filetype.sh
 @@ -105,12 +105,13 @@ build_gendouble() {
   cat gendouble.py -\EOF
   import sys
   import struct
 - import array
  
 - s = array.array(c, '\0' * 26)
 - struct.pack_into(L, s,  0, 0x00051607)  # AppleDouble
 - struct.pack_into(L, s,  4, 0x0002)  # version 2
 - s.tofile(sys.stdout)
 + s = struct.pack(LL18s,
 + 0x00051607,  # AppleDouble
 + 0x0002,  # version 2
 +# pad to 26 bytes
 + )
 + sys.stdout.write(s);
   EOF

One stray semicolon.

In terms of maintenance, I'll not run tests with 2.4 or 2.5
myself, but maybe you would be willing to check an RC candidate
each release?

-- 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

Re: [PATCH 06/21] git p4 test: use client_view in t9806

[Pete Wyckoff]

Yes, this really is four months later.  Somehow I forgot all
about this series.

gits...@pobox.com wrote on Fri, 28 Sep 2012 12:11 -0700:
 Pete Wyckoff p...@padd.com writes:
 
  Use the standard client_view function from lib-git-p4.sh
  instead of building one by hand.  This requires a bit of
  rework, using the current value of $P4CLIENT for the client
  name.  It also reorganizes the test to isolate changes to
  $P4CLIENT and $cli in a subshell.
 
  Signed-off-by: Pete Wyckoff p...@padd.com
  ---
   t/lib-git-p4.sh   |  4 ++--
   t/t9806-git-p4-options.sh | 50 
  ++-
   2 files changed, 25 insertions(+), 29 deletions(-)
 
  diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
  index 890ee60..d558dd0 100644
  --- a/t/lib-git-p4.sh
  +++ b/t/lib-git-p4.sh
  @@ -116,8 +116,8 @@ marshal_dump() {
   client_view() {
  (
  cat -EOF 
  -   Client: client
  -   Description: client
  +   Client: $P4CLIENT
  +   Description: $P4CLIENT
  Root: $cli
  View:
  EOF
  diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
  index fa40cc8..37ca30a 100755
  --- a/t/t9806-git-p4-options.sh
  +++ b/t/t9806-git-p4-options.sh
  @@ -126,37 +126,33 @@ test_expect_success 'clone --use-client-spec' '
  exec /dev/null 
  test_must_fail git p4 clone --dest=$git --use-client-spec
  ) 
  -   cli2=$(test-path-utils real_path $TRASH_DIRECTORY/cli2) 
  +   # build a different client
  +   cli2=$TRASH_DIRECTORY/cli2 
  mkdir -p $cli2 
  test_when_finished rmdir \$cli2\ 
  test_when_finished cleanup_git 
  ...
  -   # same thing again, this time with variable instead of option
  (
  ...
  +   # group P4CLIENT and cli changes in a sub-shell
  +   P4CLIENT=client2 
  +   cli=$cli2 
  +   client_view //depot/sub/... //client2/bus/... 
  +   git p4 clone --dest=$git --use-client-spec //depot/... 
  +   (
  +   cd $git 
  +   test_path_is_file bus/dir/f4 
  +   test_path_is_missing file1
  +   ) 
  +   cleanup_git 
 
 Hmm, the use of test-path-utils real_path to form cli2 in the
 original was not necessary at all?

Thanks, I will make this removal more explicit, putting it in
with 8/21 where it belongs, with explanation.

  +   # same thing again, this time with variable instead of option
  +   (
  +   cd $git 
  +   git init 
  +   git config git-p4.useClientSpec true 
  +   git p4 sync //depot/... 
  +   git checkout -b master p4/master 
  +   test_path_is_file bus/dir/f4 
  +   test_path_is_missing file1
  +   )
 
 Do you need a separate sub-shell inside a sub-shell we are already
 in that you called client_view in?
 
  )
   '

The first subshell is to hide P4CLIENT and cli variable changes
from the rest of the tests.

The second is to keep the cd $git from changing behavior of the
following cleanup_git call.  That does rm -rf $git which
would fail on some file systems if cwd is still in there.  With
just one subshell it would look like:

(
P4CLIENT=client2 
git p4 clone .. 
cd $git 
... do test
cd $TRASH_DIRECTORY 
cleanup_git 

cd $git 
... more test
)

It's a bit easier to understand with an extra level of shell,
and sticks to the pattern used in the rest of the t98*.

-- 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

[PATCHv2 00/21] git p4: work on cygwin

[Pete Wyckoff]

Junio and Hannes:  thanks for the comments four months ago; I've
been slow getting back to this.  I incorporated all your
suggestions.

Junio: this merges okay with Brandon's v2.4 support series.

This series fixes problems in git-p4, and its tests, so that
git-p4 works on the cygwin platform.

See the wiki for info on how to get started on cygwin:

https://git.wiki.kernel.org/index.php/GitP4

Testing by people who use cygwin would be appreciated.  It would
be good to support cygwin more regularly.  Anyone who had time
to contribute to testing on cygwin, and reporting problems, would
be welcome.

There's more work requried to support msysgit.  Those patches
are not in good enough shape to ship out yet, but a lot of what
is in this series is required for msysgit too.

These patches:

- fix bugs in git-p4 related to issues found on cygwin

- cleanup some ugly code in git-p4 observed in error paths while
  getting tests to work on cygwin

- simplify and refactor code and tests to make cygwin changes easier

- handle newline and path issues for cygwin platform

- speed up some aspects of git-p4 by removing extra shell invocations

Changes from v1:

http://thread.gmane.org/gmane.comp.version-control.git/206557

- Addressed comments from Junio and Hannes:

- Removed git p4: fix error message when describe -s fails;
  it was fixed as part of 18fa13d (git p4: catch p4 describe
  errors, 2012-11-23), with messages like p4 describe -s ...
  failed.

- Removed extranneous grep -q in git p4: generate better
  error message for bad depot path.

- Added git p4 test: avoid loop in client_view after a
  suggestion from Junio.

- Made the test-path-utils removal explicit.

- Modify the chmod test to use test_chmod, and verify at
  least the p4 bits on cygwin, although not the filesystem.

- Retested on latest cygwin

Pete Wyckoff (21):
  git p4: temp branch name should use / even on windows
  git p4: remove unused imports
  git p4: generate better error message for bad depot path
  git p4 test: use client_view to build the initial client
  git p4 test: avoid loop in client_view
  git p4 test: use client_view in t9806
  git p4 test: start p4d inside its db dir
  git p4 test: translate windows paths for cygwin
  git p4: remove unreachable windows \r\n conversion code
  git p4: scrub crlf for utf16 files on windows
  git p4 test: newline handling
  git p4 test: use LineEnd unix in windows tests too
  git p4 test: avoid wildcard * in windows
  git p4: cygwin p4 client does not mark read-only
  git p4 test: use test_chmod for cygwin
  git p4: disable read-only attribute before deleting
  git p4: avoid shell when mapping users
  git p4: avoid shell when invoking git rev-list
  git p4: avoid shell when invoking git config --get-all
  git p4: avoid shell when calling git config
  git p4: introduce gitConfigBool

 git-p4.py | 119 --
 t/lib-git-p4.sh   |  64 ---
 t/t9800-git-p4-basic.sh   |   5 ++
 t/t9802-git-p4-filetype.sh| 117 +
 t/t9806-git-p4-options.sh |  51 --
 t/t9807-git-p4-submit.sh  |  14 -
 t/t9809-git-p4-client-view.sh |  16 --
 t/t9812-git-p4-wildcards.sh   |  37 ++---
 t/t9815-git-p4-submit-fail.sh |  11 ++--
 t/test-lib.sh |   3 ++
 10 files changed, 332 insertions(+), 105 deletions(-)

-- 
1.8.1.1.460.g6fa8886

--
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 01/21] git p4: temp branch name should use / even on windows

[Pete Wyckoff]

Commit fed2369 (git-p4: Search for parent commit on branch creation,
2012-01-25) uses temporary branches to help find the parent of a
new p4 branch.  The temp branches are of the form git-p4-tmp/%d
for some p4 change number.  Mistakenly, this string was made
using os.path.join() instead of just string concatenation.  On
windows, this turns into a backslash (\), which is not allowed in
git branch names.

Reported-by: Casey McGinty casey.mcgi...@gmail.com
Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-p4.py b/git-p4.py
index 2da5649..fb77c56 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2687,7 +2687,7 @@ class P4Sync(Command, P4UserMap):
 
 blob = None
 if len(parent)  0:
-tempBranch = os.path.join(self.tempBranchLocation, 
%d % (change))
+tempBranch = %s/%d % (self.tempBranchLocation, 
change)
 if self.verbose:
 print Creating temporary branch:  + 
tempBranch
 self.commit(description, filesForCommit, 
tempBranch)
-- 
1.8.1.1.460.g6fa8886

--
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 02/21] git p4: remove unused imports

[Pete Wyckoff]

Found by pyflakes checker tool.
Modules shelve, getopt were unused.
Module os.path is exported by os.
Reformat one-per-line as is PEP008 suggested style.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index fb77c56..47d092d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -7,16 +7,20 @@
 #2007 Trolltech ASA
 # License: MIT http://www.opensource.org/licenses/mit-license.php
 #
-
 import sys
 if sys.hexversion  0x0204:
 # The limiter is the subprocess module
 sys.stderr.write(git-p4: requires Python 2.4 or later.\n)
 sys.exit(1)
-
-import optparse, os, marshal, subprocess, shelve
-import tempfile, getopt, os.path, time, platform
-import re, shutil
+import os
+import optparse
+import marshal
+import subprocess
+import tempfile
+import time
+import platform
+import re
+import shutil
 
 verbose = False
 
-- 
1.8.1.1.460.g6fa8886

--
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 03/21] git p4: generate better error message for bad depot path

[Pete Wyckoff]

Depot paths must start with //.  Exit with a better explanation
when a bad depot path is supplied.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 git-p4.py   | 1 +
 t/t9800-git-p4-basic.sh | 5 +
 2 files changed, 6 insertions(+)

diff --git a/git-p4.py b/git-p4.py
index 47d092d..cbf8525 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3163,6 +3163,7 @@ class P4Clone(P4Sync):
 self.cloneExclude = [/+p for p in self.cloneExclude]
 for p in depotPaths:
 if not p.startswith(//):
+sys.stderr.write('Depot paths must start with //: %s\n' % p)
 return False
 
 if not self.cloneDestination:
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 166e752..665607c 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -30,6 +30,11 @@ test_expect_success 'basic git p4 clone' '
)
 '
 
+test_expect_success 'depot typo error' '
+   test_must_fail git p4 clone --dest=$git /depot 2errs 
+   grep Depot paths must start with errs
+'
+
 test_expect_success 'git p4 clone @all' '
git p4 clone --dest=$git //depot@all 
test_when_finished cleanup_git 
-- 
1.8.1.1.460.g6fa8886

--
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 04/21] git p4 test: use client_view to build the initial client

[Pete Wyckoff]

Simplify the code a bit by using an existing function.

Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/lib-git-p4.sh | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7061dce..890ee60 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -74,15 +74,8 @@ start_p4d() {
fi
 
# build a client
-   (
-   cd $cli 
-   p4 client -i -EOF
-   Client: client
-   Description: client
-   Root: $cli
-   View: //depot/... //client/...
-   EOF
-   )
+   client_view //depot/... //client/... 
+
return 0
 }
 
-- 
1.8.1.1.460.g6fa8886

--
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 05/21] git p4 test: avoid loop in client_view

[Pete Wyckoff]

The printf command re-interprets the format string as
long as there are arguments to consume.  Use this to
simplify a for loop in the client_view() library function.

This requires a fix to one of the client_view callers.
An errant \n in the string was converted into a harmless
newline in the input to p4 client -i, but now shows up
as a literal \n as passed through by %s.  Remove the \n.

Based-on-patch-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Pete Wyckoff p...@padd.com
---
 t/lib-git-p4.sh   | 4 +---
 t/t9809-git-p4-client-view.sh | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 890ee60..b1dbded 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -121,8 +121,6 @@ client_view() {
Root: $cli
View:
EOF
-   for arg ; do
-   printf \t$arg\n
-   done
+   printf \t%s\n $@
) | p4 client -i
 }
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 281be29..0b58fb9 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -196,7 +196,7 @@ test_expect_success 'exclusion single file' '
 
 test_expect_success 'overlay wildcard' '
client_view //depot/dir1/... //client/cli/... \
-   +//depot/dir2/... //client/cli/...\n 
+   +//depot/dir2/... //client/cli/... 
files=cli/file11 cli/file12 cli/file21 cli/file22 
client_verify $files 
test_when_finished cleanup_git 
-- 
1.8.1.1.460.g6fa8886

--
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