diff --git a/src/repository/api/base/ArcanistRepositoryAPI.php b/src/repository/api/base/ArcanistRepositoryAPI.php index af86d025..1b5c7d23 100644 --- a/src/repository/api/base/ArcanistRepositoryAPI.php +++ b/src/repository/api/base/ArcanistRepositoryAPI.php @@ -209,4 +209,121 @@ abstract class ArcanistRepositoryAPI { abstract protected function buildLocalFuture(array $argv); + +/* -( Scratch Files )------------------------------------------------------ */ + + + /** + * Try to read a scratch file, if it exists and is readable. + * + * @param string Scratch file name. + * @return mixed String for file contents, or false for failure. + * @task scratch + */ + public function readScratchFile($path) { + $full_path = $this->getScratchFilePath($path); + if (!$full_path) { + return false; + } + + if (!Filesystem::pathExists($full_path)) { + return false; + } + + try { + $result = Filesystem::readFile($full_path); + } catch (FilesystemException $ex) { + return false; + } + + return $result; + } + + + /** + * Try to write a scratch file, if there's somewhere to put it and we can + * write there. + * + * @param string Scratch file name to write. + * @param string Data to write. + * @return bool True on success, false on failure. + * @task scratch + */ + public function writeScratchFile($path, $data) { + $dir = $this->getScratchFilePath(''); + if (!$dir) { + return false; + } + + if (!Filesystem::pathExists($dir)) { + try { + Filesystem::createDirectory($dir); + } catch (Exception $ex) { + return false; + } + } + + try { + Filesystem::writeFile($this->getScratchFilePath($path), $data); + } catch (FilesystemException $ex) { + return false; + } + + return true; + } + + + /** + * Try to remove a scratch file. + * + * @param string Scratch file name to remove. + * @return bool True if the file was removed successfully. + * @task scratch + */ + public function removeScratchFile($path) { + $full_path = $this->getScratchFilePath($path); + if (!$full_path) { + return false; + } + + try { + Filesystem::remove($full_path); + } catch (FilesystemException $ex) { + return false; + } + + return true; + } + + + /** + * Get a human-readable description of the scratch file location. + * + * @param string Scratch file name. + * @return mixed String, or false on failure. + * @task scratch + */ + public function getReadableScratchFilePath($path) { + $full_path = $this->getScratchFilePath($path); + if ($full_path) { + return Filesystem::readablePath( + $full_path, + $this->getPath()); + } else { + return false; + } + } + + + /** + * Get the path to a scratch file, if possible. + * + * @param string Scratch file name. + * @return mixed File path, or false on failure. + * @task scratch + */ + public function getScratchFilePath($path) { + return $this->getPath('.arc/'.$path); + } + } diff --git a/src/repository/api/git/ArcanistGitAPI.php b/src/repository/api/git/ArcanistGitAPI.php index ecbc5b45..9f1bbe50 100644 --- a/src/repository/api/git/ArcanistGitAPI.php +++ b/src/repository/api/git/ArcanistGitAPI.php @@ -112,18 +112,80 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { public function getRelativeCommit() { if ($this->relativeCommit === null) { + + // Detect zero-commit or one-commit repositories. There is only one + // relative-commit value that makes any sense in these repositories: the + // empty tree. list($err) = $this->execManualLocal('rev-parse --verify HEAD^'); if ($err) { list($err) = $this->execManualLocal('rev-parse --verify HEAD'); if ($err) { $this->repositoryHasNoCommits = true; } + $this->relativeCommit = self::GIT_MAGIC_ROOT_COMMIT; - } else { - $this->relativeCommit = 'HEAD^'; + return $this->relativeCommit; } + + $default_relative = $this->readScratchFile('default-relative-commit'); + $do_write = false; + if (!$default_relative) { + + // TODO: Remove the history lesson soon. + + echo phutil_console_format( + "** Select a Default Commit Range **\n\n"); + echo phutil_console_wrap( + "You're running a command which operates on a range of revisions ". + "(usually, from some revision to HEAD) but have not specified the ". + "revision that should determine the start of the range.\n\n". + "Previously, arc assumed you meant 'HEAD^' when you did not specify ". + "a start revision, but this behavior does not make much sense in ". + "most workflows outside of Facebook's historic git-svn workflow.\n\n". + "arc no longer assumes 'HEAD^'. You must specify a relative commit ". + "explicitly when you invoke a command (e.g., `arc diff HEAD^`, not ". + "just `arc diff`) or select a default for this working copy.\n\n". + "In most cases, the best default is 'origin/master'. You can also ". + "select 'HEAD^' to preserve the old behavior, or some other remote ". + "or branch. But you almost certainly want to select ". + "'origin/master'.\n\n". + "(Technically: the merge-base of the selected revision and HEAD is ". + "used to determine the start of the commit range.)"); + + $prompt = "What default do you want to use? [origin/master]"; + $default = phutil_console_prompt($prompt); + + if (!strlen(trim($default))) { + $default = 'origin/master'; + } + + $default_relative = $default; + $do_write = true; + } + + list($object_type) = $this->execxLocal( + 'cat-file -t %s', + $default_relative); + + if (trim($object_type) !== 'commit') { + throw new Exception( + "Relative commit '{$relative}' is not the name of a commit!"); + } + + if ($do_write) { + // Don't perform this write until we've verified that the object is a + // valid commit name. + $this->writeScratchFile('default-relative-commit', $default_relative); + } + + list($merge_base) = $this->execxLocal( + 'merge-base %s HEAD', + $default_relative); + + $this->relativeCommit = trim($merge_base); } + return $this->relativeCommit; } diff --git a/src/repository/api/git/__init__.php b/src/repository/api/git/__init__.php index 3bfab98b..e57f8047 100644 --- a/src/repository/api/git/__init__.php +++ b/src/repository/api/git/__init__.php @@ -11,6 +11,7 @@ phutil_require_module('arcanist', 'exception/usage'); phutil_require_module('arcanist', 'parser/diff'); phutil_require_module('arcanist', 'repository/api/base'); +phutil_require_module('phutil', 'console'); phutil_require_module('phutil', 'future'); phutil_require_module('phutil', 'future/exec'); phutil_require_module('phutil', 'utils'); diff --git a/src/workflow/base/ArcanistBaseWorkflow.php b/src/workflow/base/ArcanistBaseWorkflow.php index 74ce899e..03c9d7c8 100644 --- a/src/workflow/base/ArcanistBaseWorkflow.php +++ b/src/workflow/base/ArcanistBaseWorkflow.php @@ -1007,6 +1007,7 @@ abstract class ArcanistBaseWorkflow { return implode('', $list); } + /* -( Scratch Files )------------------------------------------------------ */ @@ -1018,22 +1019,10 @@ abstract class ArcanistBaseWorkflow { * @task scratch */ protected function readScratchFile($path) { - $full_path = $this->getScratchFilePath($path); - if (!$full_path) { + if (!$this->repositoryAPI) { return false; } - - if (!Filesystem::pathExists($full_path)) { - return false; - } - - try { - $result = Filesystem::readFile($full_path); - } catch (FilesystemException $ex) { - return false; - } - - return $result; + return $this->getRepositoryAPI()->readScratchFile($path); } @@ -1047,26 +1036,10 @@ abstract class ArcanistBaseWorkflow { * @task scratch */ protected function writeScratchFile($path, $data) { - $dir = $this->getScratchFilePath(''); - if (!$dir) { + if (!$this->repositoryAPI) { return false; } - - if (!Filesystem::pathExists($dir)) { - try { - execx('mkdir %s', $dir); - } catch (Exception $ex) { - return false; - } - } - - try { - Filesystem::writeFile($this->getScratchFilePath($path), $data); - } catch (FilesystemException $ex) { - return false; - } - - return true; + return $this->getRepositoryAPI()->writeScratchFile($path, $data); } @@ -1078,18 +1051,10 @@ abstract class ArcanistBaseWorkflow { * @task scratch */ protected function removeScratchFile($path) { - $full_path = $this->getScratchFilePath($path); - if (!$full_path) { + if (!$this->repositoryAPI) { return false; } - - try { - Filesystem::remove($full_path); - } catch (FilesystemException $ex) { - return false; - } - - return true; + return $this->getRepositoryAPI()->removeScratchFile($path); } @@ -1101,14 +1066,10 @@ abstract class ArcanistBaseWorkflow { * @task scratch */ protected function getReadableScratchFilePath($path) { - $full_path = $this->getScratchFilePath($path); - if ($full_path) { - return Filesystem::readablePath( - $full_path, - $this->getRepositoryAPI()->getPath()); - } else { + if (!$this->repositoryAPI) { return false; } + return $this->getRepositoryAPI()->getReadableScratchFilePath($path); } @@ -1123,9 +1084,7 @@ abstract class ArcanistBaseWorkflow { if (!$this->repositoryAPI) { return false; } - - $repository_api = $this->getRepositoryAPI(); - return $repository_api->getPath('.arc/'.$path); + return $this->getRepositoryAPI()->getScratchFilePath($path); } protected function getRepositoryEncoding() { diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index 1959d447..a68e130b 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -52,7 +52,7 @@ EOTEXT Under git, you can specify a commit (like __HEAD^^^__ or __master__) and Differential will generate a diff against the merge base of that - commit and HEAD. If you omit the commit, the default is __HEAD^__. + commit and HEAD. Under svn, you can choose to include only some of the modified files in the working copy in the diff by specifying their paths. If you