From e99c53da2ea2b1dc7a097d40adc749b625ea7151 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 21 Nov 2013 14:41:38 -0800
Subject: [PATCH] Fix an issue with SVN path construction in the presence of
 subpath configuration

Summary: D7590 made path construction more consistent, but affected this callsite if a subpath is configured. Currently, we end up with double `@@` in the URI.

Test Plan:
  - Ran unit tests.
  - Ran `bin/repostitory discover`.

Reviewers: staticshock, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7619
---
 .../engine/PhabricatorRepositoryDiscoveryEngine.php   |  5 ++---
 .../repository/storage/PhabricatorRepository.php      |  4 ++--
 .../__tests__/PhabricatorRepositoryTestCase.php       | 11 +++++++++++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
index 17374a02d6..3134e1eb5f 100644
--- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
@@ -88,10 +88,9 @@ final class PhabricatorRepositoryDiscoveryEngine
 
       try {
         list($xml, $stderr) = $repository->execxRemoteCommand(
-          'log --xml --quiet --limit %d %s@%s',
+          'log --xml --quiet --limit %d %s',
           $limit,
-          $repository->getSubversionBaseURI(),
-          $at_rev);
+          $repository->getSubversionBaseURI($at_rev));
       } catch (CommandException $ex) {
         $stderr = $ex->getStdErr();
         if (preg_match('/(path|File) not found/', $stderr)) {
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
index 4597a36fe3..c8601e6729 100644
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -155,12 +155,12 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     return $this->getDetail('local-path');
   }
 
-  public function getSubversionBaseURI() {
+  public function getSubversionBaseURI($commit = null) {
     $subpath = $this->getDetail('svn-subpath');
     if (!strlen($subpath)) {
       $subpath = null;
     }
-    return $this->getSubversionPathURI($subpath);
+    return $this->getSubversionPathURI($subpath, $commit);
   }
 
   public function getSubversionPathURI($path = null, $commit = null) {
diff --git a/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php b/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php
index c58e7c2dfe..69f18f454a 100644
--- a/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php
+++ b/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php
@@ -94,6 +94,17 @@ final class PhabricatorRepositoryTestCase
     $this->assertEqual(
       'file:///var/repo/SVN/%3F@22',
       $repo->getSubversionPathURI('?', 22));
+
+    $repo->setDetail('svn-subpath', 'quack/trunk/');
+
+    $this->assertEqual(
+      'file:///var/repo/SVN/quack/trunk/@',
+      $repo->getSubversionBaseURI());
+
+    $this->assertEqual(
+      'file:///var/repo/SVN/quack/trunk/@HEAD',
+      $repo->getSubversionBaseURI('HEAD'));
+
   }