Don't match Perforce repositories when outside the checkout.

Review Request #5271 — Created Jan. 17, 2014 and submitted

Information

RBTools
release-0.5.x

Reviewers

Don't match Perforce repositories when outside the checkout.

We had a long-standing bug where if $P4PORT was set and p4 info
returned valid repository information, we'd consider that a match, even
if not within that Perforce checkout.

We now make use of the Client root field from p4 info and only match
a repository if we're within that Checkout somewhere.

If working with a nested repository where one checkout is within
another, and one type of repository is listed before another, then this
could still fail. That's, however, a larger problem that needs to be
solved across SCMClients.

Before running any tests, I ran with the following scenarios, all with P4PORT set to
valid repository.

  1. Ran in /
  2. Ran in a Perforce checkout matching the default client for my P4PORT.
  3. Ran in a Perforce checkout not matching the default client for my P4PORT.

In each of these, running with --debug would show a repository match for my
default client for P4PORT. Note that in my tests, Perforce was checked after Git,
but this is no guarantee. If it were checked before, I'd get the same repository
match in a Git checkout.

After this change, I repeated the same tests.

Test #1 no longer matched a repository, giving me an error about my current directory
not containing a checkout.

Test #2 matched the repository correctly.

Test #3 gave me the same results as #1, since I was not in the correct checkout.

Note that test #3 didn't match the repository because I didn't have any information
available for p4 info to know what the client was for that directory. Adding a proper
.p4config with P4CLIENT set resulted in a proper repostory match.

I also tested with P4PORT set to $HOME/temp/perforcecheckout, and then ran inside
$HOME/temp/perforcecheckout2, to make sure the path prefix check didn't see the second
starting with the first. I got the expected results.

Unit tests pass.

david
  1. I haven't looked at the code yet, but this should also cover bug 2801.

  2. 
      
chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Loading...