From c13147cf534930b448ec93fef84c32ff1e59995e Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Feb 2011 14:08:49 -0800 Subject: [PATCH 1/2] Stop arc from destroying files under HPHPi. Summary: Test Plan: Reviewers: CC: --- src/lint/patcher/ArcanistLintPatcher.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lint/patcher/ArcanistLintPatcher.php b/src/lint/patcher/ArcanistLintPatcher.php index eefb8ad6..de687147 100644 --- a/src/lint/patcher/ArcanistLintPatcher.php +++ b/src/lint/patcher/ArcanistLintPatcher.php @@ -106,7 +106,12 @@ final class ArcanistLintPatcher { $new_str = $lint->getReplacementText(); $new_len = strlen($new_str); - $data = substr_replace($data, $new_str, $working_offset, $old_len); + if ($working_offset == strlen($data)) { + // Temporary hack to work around a destructive hphpi issue, see #451031. + $data .= $new_str; + } else { + $data = substr_replace($data, $new_str, $working_offset, $old_len); + } $this->changeCharacterDelta($new_len - $old_len); $this->setDirtyCharacterOffset($orig_offset + $old_len); From 651f567f96624ce2d6ae607bd6a92f05f7905d9a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 24 Feb 2011 16:34:27 -0800 Subject: [PATCH 2/2] Improve error messages when trying to parse bad .arcconfig files. Summary: Haiping is getting a pretty confusing error message when trying to commit. Test Plan: Created a mock repository, installed the hook, made commits against directories with bad .arcconfigs. Reviewers: CC: --- scripts/arcanist.php | 7 ++++-- .../api/subversion/ArcanistSubversionAPI.php | 6 ++--- src/workflow/diff/ArcanistDiffWorkflow.php | 2 +- .../ArcanistSvnHookPreCommitWorkflow.php | 3 ++- .../ArcanistWorkingCopyIdentity.php | 22 ++++++++++++------- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/scripts/arcanist.php b/scripts/arcanist.php index dfd7309e..c3ec4308 100755 --- a/scripts/arcanist.php +++ b/scripts/arcanist.php @@ -82,10 +82,13 @@ try { if ($config_trace_mode) { echo "Loading phutil library '{$name}' from '{$location}'...\n"; } - $library_root = Filesystem::resolvePath( + $resolved_location = Filesystem::resolvePath( $location, $working_copy->getProjectRoot()); - phutil_load_library($library_root); + if (Filesystem::pathExists($resolved_location)) { + $location = $resolved_location; + } + phutil_load_library($location); } } diff --git a/src/repository/api/subversion/ArcanistSubversionAPI.php b/src/repository/api/subversion/ArcanistSubversionAPI.php index 7d4cef26..4dd7fa7e 100644 --- a/src/repository/api/subversion/ArcanistSubversionAPI.php +++ b/src/repository/api/subversion/ArcanistSubversionAPI.php @@ -29,7 +29,7 @@ class ArcanistSubversionAPI extends ArcanistRepositoryAPI { protected $svnInfoRaw = array(); protected $svnDiffRaw = array(); - + private $svnBaseRevisionNumber; public function getSourceControlSystemName() { @@ -165,7 +165,7 @@ class ArcanistSubversionAPI extends ArcanistRepositoryAPI { $info = $this->getSVNInfo('/'); return $info['URL'].'@'.$this->getSVNBaseRevisionNumber(); } - + public function getSVNBaseRevisionNumber() { if ($this->svnBaseRevisionNumber) { return $this->svnBaseRevisionNumber; @@ -173,7 +173,7 @@ class ArcanistSubversionAPI extends ArcanistRepositoryAPI { $info = $this->getSVNInfo('/'); return $info['Revision']; } - + public function overrideSVNBaseRevisionNumber($effective_base_revision) { $this->svnBaseRevisionNumber = $effective_base_revision; return $this; diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index a1d75ef5..3eb23bb1 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -522,7 +522,7 @@ EOTEXT $revlist); } } - + // If you have a change which affects several files, all of which are // at a consistent base revision, treat that revision as the effective // base revision. The use case here is that you made a change to some diff --git a/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php b/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php index 68620928..f0caaefb 100644 --- a/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php +++ b/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php @@ -190,7 +190,8 @@ EOTEXT $working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile( $project_root, - $config); + $config, + $config_file." (svnlook: {$transaction} {$repository})"); $lint_engine = $working_copy->getConfig('lint_engine'); if (!$lint_engine) { diff --git a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php b/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php index 76992e9c..7763ac9a 100644 --- a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php +++ b/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php @@ -36,24 +36,30 @@ class ArcanistWorkingCopyIdentity { continue; } $proj_raw = Filesystem::readFile($config_file); - $config = self::parseRawConfigFile($proj_raw); + $config = self::parseRawConfigFile($proj_raw, $config_file); $project_root = $dir; break; } return new ArcanistWorkingCopyIdentity($project_root, $config); } - public static function newFromRootAndConfigFile($root, $config_raw) { - $config = self::parseRawConfigFile($config_raw); + public static function newFromRootAndConfigFile( + $root, + $config_raw, + $from_where) { + + $config = self::parseRawConfigFile($config_raw, $from_where); return new ArcanistWorkingCopyIdentity($root, $config); } - private static function parseRawConfigFile($raw_config) { + private static function parseRawConfigFile($raw_config, $from_where) { $proj = json_decode($raw_config, true); if (!is_array($proj)) { throw new Exception( - "Unable to parse '.arcconfig' file in '{$dir}'. The file contents ". - "should be valid JSON."); + "Unable to parse '.arcconfig' file '{$from_where}'. The file contents ". + "should be valid JSON.\n\n". + "FILE CONTENTS\n". + substr($raw_config, 0, 2048)); } $required_keys = array( 'project_id', @@ -61,8 +67,8 @@ class ArcanistWorkingCopyIdentity { foreach ($required_keys as $key) { if (!array_key_exists($key, $proj)) { throw new Exception( - "Required key '{$key}' is missing from '.arcconfig' file in ". - "'{$dir}'."); + "Required key '{$key}' is missing from '.arcconfig' file ". + "'{$from_where}'."); } } return $proj;