diff --git a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php --- a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php @@ -35,22 +35,26 @@ protected function getGitResult(ConduitAPIRequest $request) { $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); + $path = $request->getValue('path'); + if (!strlen($path)) { + $path = null; + } + $commit = $request->getValue('commit'); $offset = (int)$request->getValue('offset'); $limit = (int)$request->getValue('limit'); $result = $this->getEmptyResultSet(); - if ($path == '') { + if ($path === null) { // Fast path to improve the performance of the repository view; we know // the root is always a tree at any commit and always exists. $stdout = 'tree'; } else { try { list($stdout) = $repository->execxLocalCommand( - 'cat-file -t -- %s:%s', - $commit, - $path); + 'cat-file -t -- %s', + sprintf('%s:%s', $commit, $path)); } catch (CommandException $e) { // The "cat-file" command may fail if the path legitimately does not // exist, but it may also fail if the path is a submodule. This can @@ -121,14 +125,20 @@ return $result; } - list($stdout) = $repository->execxLocalCommand( - 'ls-tree -z -l %s -- %s', - gitsprintf('%s', $commit), - $path); + if ($path === null) { + list($stdout) = $repository->execxLocalCommand( + 'ls-tree -z -l %s --', + gitsprintf('%s', $commit)); + } else { + list($stdout) = $repository->execxLocalCommand( + 'ls-tree -z -l %s -- %s', + gitsprintf('%s', $commit), + $path); + } $submodules = array(); - if (strlen($path)) { + if ($path !== null) { $prefix = rtrim($path, '/').'/'; } else { $prefix = ''; @@ -226,8 +236,8 @@ $dict[$key] = $value; } - foreach ($submodules as $path) { - $full_path = $path->getFullPath(); + foreach ($submodules as $submodule_path) { + $full_path = $submodule_path->getFullPath(); $key = 'submodule.'.$full_path.'.url'; if (isset($dict[$key])) { $path->setExternalURI($dict[$key]); diff --git a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php --- a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php @@ -45,7 +45,12 @@ $repository = $drequest->getRepository(); $commit_hash = $request->getValue('commit'); $against_hash = $request->getValue('against'); + $path = $request->getValue('path'); + if (!strlen($path)) { + $path = null; + } + $offset = $request->getValue('offset'); $limit = $request->getValue('limit'); @@ -55,18 +60,27 @@ $commit_range = $commit_hash; } + $argv = array(); + + $argv[] = '--skip'; + $argv[] = $offset; + + $argv[] = '--max-count'; + $argv[] = $limit; + + $argv[] = '--format=%H:%P'; + + $argv[] = gitsprintf('%s', $commit_range); + + $argv[] = '--'; + + if ($path !== null) { + $argv[] = $path; + } + list($stdout) = $repository->execxLocalCommand( - 'log '. - '--skip=%d '. - '-n %d '. - '--pretty=format:%s '. - '%s -- %C', - $offset, - $limit, - '%H:%P', - gitsprintf('%s', $commit_range), - // Git omits merge commits if the path is provided, even if it is empty. - (strlen($path) ? csprintf('%s', $path) : '')); + 'log %Ls', + $argv); $lines = explode("\n", trim($stdout)); $lines = array_filter($lines); diff --git a/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php --- a/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php @@ -43,9 +43,9 @@ // it requires the commit to have a parent that we can diff against. The // first commit doesn't, so "commit^" is not a valid ref. list($parents) = $repository->execxLocalCommand( - 'log -n1 --format=%s %s', - '%P', - $commit); + 'log -n1 %s %s --', + '--format=%P', + gitsprintf('%s', $commit)); $use_log = !strlen(trim($parents)); // First, get a fast raw diff without "--find-copies-harder". This flag @@ -96,18 +96,18 @@ // NOTE: "--pretty=format: " is to disable diff output, we only want the // part we get from "--raw". $future = $repository->getLocalCommandFuture( - 'log %Ls --pretty=format: %s', + 'log %Ls --pretty=format: %s --', $flags, - $commit); + gitsprintf('%s', $commit)); } else { // Otherwise, we can use "diff", which will give us output for merges. // We diff against the first parent, as this is generally the expectation // and results in sensible behavior. $future = $repository->getLocalCommandFuture( - 'diff %Ls %s^1 %s', + 'diff %Ls %s %s --', $flags, - $commit, - $commit); + gitsprintf('%s^1', $commit), + gitsprintf('%s', $commit)); } // Don't spend more than 30 seconds generating the slower output. diff --git a/src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php --- a/src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php @@ -33,7 +33,8 @@ continue; } list($hash) = $repository->execxLocalCommand( - 'log -n1 --format=%%H %s -- %s', + 'log -n1 %s %s -- %s', + '--format=%H', gitsprintf('%s', $commit), $path); $results[$path] = trim($hash); diff --git a/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php --- a/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php @@ -35,8 +35,8 @@ $limit = $this->getLimit($request); list($parents) = $repository->execxLocalCommand( - 'log -n 1 --format=%s %s --', - '%P', + 'log -n 1 %s %s --', + '--format=%P', gitsprintf('%s', $commit)); $parents = preg_split('/\s+/', trim($parents)); @@ -50,10 +50,10 @@ $first_parent = head($parents); list($logs) = $repository->execxLocalCommand( - 'log -n %d --format=%s %s %s --', + 'log -n %d %s %s %s --', // NOTE: "+ 1" accounts for the merge commit itself. $limit + 1, - '%H', + '--format=%H', gitsprintf('%s', $commit), gitsprintf('%s', '^'.$first_parent)); diff --git a/src/applications/diffusion/conduit/DiffusionRefsQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionRefsQueryConduitAPIMethod.php --- a/src/applications/diffusion/conduit/DiffusionRefsQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionRefsQueryConduitAPIMethod.php @@ -28,9 +28,9 @@ $commit = $request->getValue('commit'); list($stdout) = $repository->execxLocalCommand( - 'log --format=%s -n 1 %s --', - '%d', - $commit); + 'log -n 1 %s %s --', + '--format=%d', + gitsprintf('%s', $commit)); // %d, gives a weird output format // similar to (remote/one, remote/two, remote/three) diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -608,9 +608,9 @@ // repository. Particularly, this will cover the cases of a new branch, a // completely moved tag, etc. $futures[$key] = $this->getRepository()->getLocalCommandFuture( - 'log --format=%s %s --not --all', - '%H', - $ref_update->getRefNew()); + 'log %s %s --not --all --', + '--format=%H', + gitsprintf('%s', $ref_update->getRefNew())); } $content_updates = array(); diff --git a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php --- a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php @@ -10,9 +10,8 @@ $commit = $drequest->getCommit(); return $repository->getLocalCommandFuture( - 'cat-file blob -- %s:%s', - $commit, - $path); + 'cat-file blob -- %s', + sprintf('%s:%s', $commit, $path)); } } diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php @@ -55,12 +55,15 @@ $split_body = true; } - // Even though we pass --encoding here, git doesn't always succeed, so - // we try a little harder, since git *does* tell us what the actual encoding - // is correctly (unless it doesn't; encoding is sometimes empty). - list($info) = $repository->execxLocalCommand( - 'log -n 1 --encoding=%s --format=%s %s --', - 'UTF-8', + $argv = array(); + + $argv[] = '-n'; + $argv[] = '1'; + + $argv[] = '--encoding=UTF-8'; + + $argv[] = sprintf( + '--format=%s', implode( '%x00', array( @@ -78,8 +81,15 @@ // so include an explicit terminator: this makes sure the exact // body text is surrounded by "\0" characters. '~', - )), - $this->identifier); + ))); + + // Even though we pass --encoding here, git doesn't always succeed, so + // we try a little harder, since git *does* tell us what the actual encoding + // is correctly (unless it doesn't; encoding is sometimes empty). + list($info) = $repository->execxLocalCommand( + 'log -n 1 %Ls %s --', + $argv, + gitsprintf('%s', $this->identifier)); $parts = explode("\0", $info); $encoding = array_shift($parts); diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelFilesizeQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelFilesizeQuery.php --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelFilesizeQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelFilesizeQuery.php @@ -33,7 +33,7 @@ $paths_future = $repository->getLocalCommandFuture( 'diff-tree -z -r --no-commit-id %s --', - $identifier); + gitsprintf('%s', $identifier)); // With "-z" we get "\0\0" for each line. Process the // delimited text as ", " pairs. diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php @@ -37,8 +37,8 @@ $repository = $this->getRepository(); list($stdout) = $repository->execxLocalCommand( - 'log -n 1 --format=%s %s --', - '%P', + 'log -n 1 %s %s --', + '--format=%P', gitsprintf('%s', $this->identifier)); return preg_split('/\s+/', trim($stdout)); diff --git a/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php b/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php --- a/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php +++ b/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php @@ -23,8 +23,8 @@ // Check if this is the root commit by seeing if it has parents, since // `git diff X^ X` does not work if "X" is the initial commit. list($parents) = $repository->execxLocalCommand( - 'log -n 1 --format=%s %s --', - '%P', + 'log -n 1 %s --', + '--format=%P', gitsprintf('%s', $commit)); if (strlen(trim($parents))) { diff --git a/src/applications/repository/daemon/PhabricatorGitGraphStream.php b/src/applications/repository/daemon/PhabricatorGitGraphStream.php --- a/src/applications/repository/daemon/PhabricatorGitGraphStream.php +++ b/src/applications/repository/daemon/PhabricatorGitGraphStream.php @@ -19,13 +19,13 @@ if ($start_commit !== null) { $future = $repository->getLocalCommandFuture( - 'log --format=%s %s --', - '%H%x01%P%x01%ct', - $start_commit); + 'log %s %s --', + '--format=%H%x01%P%x01%ct', + gitsprintf('%s', $start_commit)); } else { $future = $repository->getLocalCommandFuture( - 'log --format=%s --all --', - '%H%x01%P%x01%ct'); + 'log %s --all --', + '--format=%H%x01%P%x01%ct'); } $this->iterator = new LinesOfALargeExecFuture($future); diff --git a/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php b/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php --- a/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php @@ -483,17 +483,17 @@ $ref_list = implode("\n", $ref_list)."\n"; $future = $this->getRepository()->getLocalCommandFuture( - 'log --format=%s --stdin', - '%H'); + 'log %s --stdin --', + '--format=%H'); list($stdout) = $future ->write($ref_list) ->resolvex(); } else { list($stdout) = $this->getRepository()->execxLocalCommand( - 'log --format=%s %s', - '%H', - $new_head); + 'log %s %s --', + '--format=%H', + gitsprintf('%s', $new_head)); } $stdout = trim($stdout); diff --git a/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php b/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php --- a/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php +++ b/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyDiscoveryTestCase.php @@ -4,6 +4,8 @@ extends PhabricatorWorkingCopyTestCase { public function testSubversionCommitDiscovery() { + $this->requireBinaryForTest('svn'); + $refs = $this->discoverRefs('ST'); $this->assertEqual( array(