Differential D7486 Diff 16872 src/applications/differential/DifferentialGetWorkingCopy.php

Changeset View

Standalone View

src/applications/differential/DifferentialGetWorkingCopy.php

This file was added.

				<?php

				/**
				* Can't find a good place for this, so I'm putting it in the most notably
				* wrong place.
				*/
				epriestleyUnsubmitted Not Done Inline Actions This is fine, we'll move it into Drydock eventually in some form. epriestley: This is fine, we'll move it into Drydock eventually in some form.
				final class DifferentialGetWorkingCopy {

				/**
				* Creates and/or cleans a workspace for the requested repo.
				* tries to handle locking.
				*
				* return ArcanistGitAPI TODO is this the right abstraction for server side?
				epriestleyUnsubmitted Not Done Inline Actions Discussed a bit below, but since you don't need any of the stuff it offers, using `PhabricatorRepository` is more consistent. Broadly, these classes (PhabricatorRepository vs RepositoryAPI vs all the Diffusion queries) probably were not perfectly architected and should theoretically split their responsibilities out a little differently than they do, but the current split doesn't seem to cause any major issues. epriestley: Discussed a bit below, but since you don't need any of the stuff it offers, using…
				aviveyAuthorUnsubmitted Not Done Inline Actions Like we discussed on irc - it's kinda awkward right now to use `PhabricatorRepository`. `ArcanistGitApi` is mostly missing the "HOME=" env var (which was solved in git 1.8.3.1). avivey: Like we discussed on irc - it's kinda awkward right now to use `PhabricatorRepository`.
				*/
				public static function getCleanGitWorkspace(
				PhabricatorRepository $repo) {

				$origin_path = $repo->getLocalPath();

				$path = rtrim($origin_path, '/');
				$path = $path . '__workspace';

				if (! Filesystem::pathExists($path)) {
				$future = new ExecFuture(
				epriestleyUnsubmitted Not Done Inline Actions Slightly simpler as `execxLocalCommand()`, I think. epriestley: Slightly simpler as `execxLocalCommand()`, I think.
				'git clone file://%s %s',
				$origin_path,
				$path);
				$future->resolvex();
				epriestleyUnsubmitted Not Done Inline Actions Use `$repo->newRemoteCommandFuture()` so we pick up all the environmental hacks we need on some systems. This is slightly more correct as `clone -- %s` than `clone %s` (that is, add `--` to terminate flags), since `$path` could some day start with a `"-"`. epriestley: Use `$repo->newRemoteCommandFuture()` so we pick up all the environmental hacks we need on some…
				aviveyAuthorUnsubmitted Not Done Inline Actions `getLocalCommandFuture()` seems to be a better match (Because we ignore the repo's own remote, and our remote here is `file://`. avivey: `getLocalCommandFuture()` seems to be a better match (Because we ignore the repo's own remote…
				}

				self::obtainLock($path .'/.git/phabricator_lockfile');
				epriestleyUnsubmitted Not Done Inline Actions Use `PhabricatorGlobalLock` (probably much easier) or `PhutilFileLock` (probably not a good fit). These locks have better release semantics, unit tests, etc. See PhabricatorRepositoryPullLocalDaemon for an example of creating a GlobalLock on a repo. I'd just put a lock on the whole repo for now -- by the time we support mor than one simultaneous workspace, hopefully we'll be on Drydock. epriestley: Use `PhabricatorGlobalLock` (probably much easier) or `PhutilFileLock` (probably not a good…

				$workspace = new ArcanistGitAPI($path);
				$workspace->execxLocal('clean -fd');
				$workspace->execxLocal('checkout master');
				$workspace->execxLocal('fetch');
				$workspace->execxLocal('reset --hard origin/master');
				$workspace->reloadWorkingCopy();
				epriestleyUnsubmitted Not Done Inline Actions Using `$repo->execxLocalCommand()` is probably slightly preferable here if you don't need the GitAPI elsewhere. In theory we should maybe share more code between Repository and the API classes, but in practice they mostly tend to deal with dissimilar situations. epriestley:* Using `$repo->execxLocalCommand()` is probably slightly preferable here if you don't need the…

				return $workspace;
				}

				private static function obtainLock($filename) {
				$file = fopen($filename, 'c');
				if ($file === false) {
				throw new Exception("Could not create lockfile");
				}
				$locked = flock($file, LOCK_EX \| LOCK_NB);
				if ($locked !== true) {
				throw new Exception("Could not lock lockfile");
				}
				// ...and keep the lock for the remainder of run.
				}
				epriestleyUnsubmitted Not Done Inline Actions Although it doesn't impact anything for now, this locking should probably happen in the Strategy so we can release it at the right time. epriestley: Although it doesn't impact anything for now, this locking should probably happen in the…
				}