1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-08 16:02:39 +01:00

In "arc land" in Mercurial, inch closer to making complex branch/bookmark workflows function

Summary:
Ref T9948. Ref T13546. This change moves toward a functional "arc land" in Mercurial.

Because of how "bundlerepo.getremotechanges()" works, "hg arc-ls-markers" does not actually list markers in the remote that aren't different from local markers so it's hard to get anywhere with this.

Test Plan: Got somewhat-encouraging output from "arc land" and "hg arc-ls-markers", but too many things are still broken for this to really work yet.

Maniphest Tasks: T13546, T9948

Differential Revision: https://secure.phabricator.com/D21348
This commit is contained in:
epriestley 2020-06-10 13:43:08 -07:00
parent 727d73fec9
commit 488a24c40a
7 changed files with 429 additions and 190 deletions

View file

@ -3,6 +3,9 @@
final class ArcanistMercurialLandEngine
extends ArcanistLandEngine {
private $ontoBranchMarker;
private $ontoMarkers;
protected function getDefaultSymbols() {
$api = $this->getRepositoryAPI();
$log = $this->getLogEngine();
@ -242,6 +245,8 @@ final class ArcanistMercurialLandEngine
}
protected function confirmOntoRefs(array $onto_refs) {
$api = $this->getRepositoryAPI();
foreach ($onto_refs as $onto_ref) {
if (!strlen($onto_ref)) {
throw new PhutilArgumentUsageException(
@ -251,6 +256,63 @@ final class ArcanistMercurialLandEngine
$onto_ref));
}
}
$remote_ref = id(new ArcanistRemoteRef())
->setRemoteName($this->getOntoRemote());
$markers = $api->newMarkerRefQuery()
->withRemotes(array($remote_ref))
->execute();
$onto_markers = array();
$new_markers = array();
foreach ($onto_refs as $onto_ref) {
$matches = array();
foreach ($markers as $marker) {
if ($marker->getName() === $onto_ref) {
$matches[] = $marker;
}
}
$match_count = count($matches);
if ($match_count > 1) {
throw new PhutilArgumentUsageException(
pht(
'TODO: Ambiguous ref.'));
} else if (!$match_count) {
$new_bookmark = id(new ArcanistMarkerRef())
->setMarkerType(ArcanistMarkerRef::TYPE_BOOKMARK)
->setName($onto_ref)
->attachRemoteRef($remote_ref);
$onto_markers[] = $new_bookmark;
$new_markers[] = $new_bookmark;
} else {
$onto_markers[] = $marker;
}
}
$branches = array();
foreach ($onto_markers as $onto_marker) {
if ($onto_marker->isBranch()) {
$branches[] = $onto_marker;
}
$branch_count = count($branches);
if ($branch_count > 1) {
throw new PhutilArgumentUsageException(
pht(
'TODO: You can not push onto multiple branches in Mercurial.'));
} else if ($branch_count) {
$this->ontoBranchMarker = head($branches);
}
}
if ($new_markers) {
// TODO: If we're creating bookmarks, ask the user to confirm.
}
$this->ontoMarkers = $onto_markers;
}
protected function selectIntoRemote() {
@ -366,7 +428,6 @@ final class ArcanistMercurialLandEngine
}
protected function selectIntoCommit() {
// Make sure that our "into" target is valid.
$log = $this->getLogEngine();
if ($this->getIntoEmpty()) {
@ -385,7 +446,8 @@ final class ArcanistMercurialLandEngine
$api = $this->getRepositoryAPI();
$local_ref = $this->getIntoRef();
// TODO: This error handling could probably be cleaner.
// TODO: This error handling could probably be cleaner, it will just
// raise an exception without any context.
$into_commit = $api->getCanonicalRevisionName($local_ref);
@ -446,51 +508,20 @@ final class ArcanistMercurialLandEngine
$api = $this->getRepositoryAPI();
$log = $this->getLogEngine();
// See T9948. If the user specified "--into X", we don't know if it's a
// branch, a bookmark, or a symbol which doesn't exist yet.
// In native Mercurial it is difficult to figure this out, so we use
// an extension to provide a command which works like "git ls-remote".
// NOTE: We're using passthru on this because it's a remote command and
// may prompt the user for credentials.
$tmpfile = new TempFile();
Filesystem::remove($tmpfile);
$command = $this->newPassthruCommand(
'%Ls arc-ls-remote --output %s -- %s',
$api->getMercurialExtensionArguments(),
phutil_string_cast($tmpfile),
$target->getRemote());
$command->setDisplayCommand(
'hg ls-remote -- %s',
$target->getRemote());
$err = $command->execute();
if ($err) {
throw new Exception(
pht(
'Call to "hg arc-ls-remote" failed with error "%s".',
$err));
}
$raw_data = Filesystem::readFile($tmpfile);
unset($tmpfile);
$markers = phutil_json_decode($raw_data);
$target_name = $target->getRef();
$remote_ref = id(new ArcanistRemoteRef())
->setRemoteName($target->getRemote());
$markers = $api->newMarkerRefQuery()
->withRemotes(array($remote_ref))
->withNames(array($target_name))
->execute();
$bookmarks = array();
$branches = array();
foreach ($markers as $marker) {
if ($marker['name'] !== $target_name) {
continue;
}
if ($marker['type'] === 'bookmark') {
if ($marker->isBookmark()) {
$bookmarks[] = $marker;
} else {
$branches[] = $marker;
@ -536,8 +567,7 @@ final class ArcanistMercurialLandEngine
}
$bookmark = head($bookmarks);
$target_hash = $bookmark['node'];
$is_bookmark = true;
$target_marker = $bookmark;
}
if ($branches) {
@ -559,13 +589,12 @@ final class ArcanistMercurialLandEngine
$branch = head($branches);
$target_hash = $branch['node'];
$is_branch = true;
$target_marker = $branch;
}
if ($is_branch) {
$err = $this->newPassthru(
'pull -b %s -- %s',
'pull --branch %s -- %s',
$target->getRef(),
$target->getRemote());
} else {
@ -581,12 +610,12 @@ final class ArcanistMercurialLandEngine
// them as the cost of doing business.
$err = $this->newPassthru(
'pull -B %s -- %s',
'pull --bookmark %s -- %s',
$target->getRef(),
$target->getRemote());
}
// NOTE: It's possible that between the time we ran "ls-remote" and the
// NOTE: It's possible that between the time we ran "ls-markers" and the
// time we ran "pull" that the remote changed.
// It may even have been rewound or rewritten, in which case we did not
@ -597,7 +626,7 @@ final class ArcanistMercurialLandEngine
// TODO: If the Mercurial command server is revived, this check becomes
// more reasonable if it's cheap.
return $target_hash;
return $target_marker->getCommitHash();
}
protected function selectCommits($into_commit, array $symbols) {
@ -691,6 +720,17 @@ final class ArcanistMercurialLandEngine
$revision_ref = $set->getRevisionRef();
$commit_message = $revision_ref->getCommitMessage();
// If we're landing "--onto" a branch, set that as the branch marker
// before creating the new commit.
// TODO: We could skip this if we know that the "$into_commit" already
// has the right branch, which it will if we created it.
$branch_marker = $this->ontoBranchMarker;
if ($branch_marker) {
$api->execxLocal('branch -- %s', $branch_marker);
}
try {
$argv = array();
$argv[] = '--dest';
@ -724,12 +764,83 @@ final class ArcanistMercurialLandEngine
protected function pushChange($into_commit) {
$api = $this->getRepositoryAPI();
// TODO: This does not respect "--into" or "--onto" properly.
$bookmarks = array();
foreach ($this->ontoMarkers as $onto_marker) {
if (!$onto_marker->isBookmark()) {
continue;
}
$bookmarks[] = $onto_marker;
}
// If we're pushing to bookmarks, move all the bookmarks we want to push
// to the merge commit. (There doesn't seem to be any way to specify
// "push commit X as bookmark Y" in Mercurial.)
$restore = array();
if ($bookmarks) {
$markers = $api->newMarkerRefQuery()
->withNames(array(mpull($bookmarks, 'getName')))
->withTypes(array(ArcanistMarkerRef::TYPE_BOOKMARK))
->execute();
$markers = mpull($markers, 'getCommitHash', 'getName');
foreach ($bookmarks as $bookmark) {
$bookmark_name = $bookmark->getName();
$old_position = idx($markers, $bookmark_name);
$new_position = $into_commit;
if ($old_position === $new_position) {
continue;
}
$api->execxLocal(
'bookmark --force --rev %s -- %s',
hgsprintf('%s', $new_position),
$bookmark_name);
$restore[$bookmark_name] = $old_position;
}
}
// Now, do the actual push.
$argv = array();
$argv[] = 'push';
if ($bookmarks) {
// If we're pushing at least one bookmark, we can just specify the list
// of bookmarks as things we want to push.
foreach ($bookmarks as $bookmark) {
$argv[] = '--bookmark';
$argv[] = $bookmark->getName();
}
} else {
// Otherwise, specify the commit itself.
$argv[] = '--rev';
$argv[] = hgsprintf('%s', $into_commit);
}
$argv[] = '--';
$argv[] = $this->getOntoRemote();
try {
$this->newPassthru('%Ls', $argv);
} finally {
foreach ($restore as $bookmark_name => $old_position) {
if ($old_position === null) {
$api->execxLocal(
'bookmark --delete -- %s',
$bookmark_name);
} else {
$api->execxLocal(
'bookmark --force --rev %s -- %s',
hgsprintf('%s', $old_position),
$bookmark_name);
}
}
}
$this->newPassthru(
'push --rev %s -- %s',
hgsprintf('%s', $into_commit),
$this->getOntoRemote());
}
protected function cascadeState(ArcanistLandCommitSet $set, $into_commit) {
@ -871,4 +982,13 @@ final class ArcanistMercurialLandEngine
'Local branches and bookmarks have not been changed, and are still '.
'in the same state as before.'));
}
private function newRemoteMarkers($remote) {
// See T9948. If the user specified "--into X" or "--onto X", we don't know
// if it's a branch, a bookmark, or a symbol which doesn't exist yet.
}
}

View file

@ -3,8 +3,7 @@
final class ArcanistGitRepositoryMarkerQuery
extends ArcanistRepositoryMarkerQuery {
protected function newRefMarkers() {
protected function newLocalRefMarkers() {
$api = $this->getRepositoryAPI();
$future = $this->newCurrentBranchNameFuture()->start();
@ -122,4 +121,8 @@ final class ArcanistGitRepositoryMarkerQuery
return $matches[1];
}
protected function newRemoteRefMarkers(ArcanistRemoteRef $remote) {
throw new PhutilMethodNotImplementedException();
}
}

View file

@ -7,6 +7,7 @@ final class ArcanistMarkerRef
const HARDPOINT_COMMITREF = 'arc.marker.commitRef';
const HARDPOINT_WORKINGCOPYSTATEREF = 'arc.marker.workingCopyStateRef';
const HARDPOINT_REMOTEREF = 'arc.marker.remoteRef';
const TYPE_BRANCH = 'branch';
const TYPE_BOOKMARK = 'bookmark';
@ -48,6 +49,7 @@ final class ArcanistMarkerRef
return array(
$this->newHardpoint(self::HARDPOINT_COMMITREF),
$this->newHardpoint(self::HARDPOINT_WORKINGCOPYSTATEREF),
$this->newHardpoint(self::HARDPOINT_REMOTEREF),
);
}
@ -167,4 +169,12 @@ final class ArcanistMarkerRef
return $this->getHardpoint(self::HARDPOINT_WORKINGCOPYSTATEREF);
}
public function attachRemoteRef(ArcanistRemoteRef $ref = null) {
return $this->attachHardpoint(self::HARDPOINT_REMOTEREF, $ref);
}
public function getRemoteRef() {
return $this->getHardpoint(self::HARDPOINT_REMOTEREF);
}
}

View file

@ -3,144 +3,123 @@
final class ArcanistMercurialRepositoryMarkerQuery
extends ArcanistRepositoryMarkerQuery {
protected function newRefMarkers() {
$markers = array();
if ($this->shouldQueryMarkerType(ArcanistMarkerRef::TYPE_BRANCH)) {
$markers[] = $this->newBranchOrBookmarkMarkers(false);
protected function newLocalRefMarkers() {
return $this->newMarkers();
}
if ($this->shouldQueryMarkerType(ArcanistMarkerRef::TYPE_BOOKMARK)) {
$markers[] = $this->newBranchOrBookmarkMarkers(true);
protected function newRemoteRefMarkers(ArcanistRemoteRef $remote = null) {
return $this->newMarkers($remote);
}
return array_mergev($markers);
}
private function newBranchOrBookmarkMarkers($is_bookmarks) {
private function newMarkers(ArcanistRemoteRef $remote = null) {
$api = $this->getRepositoryAPI();
$is_branches = !$is_bookmarks;
// In native Mercurial it is difficult to identify remote markers, and
// complicated to identify local markers efficiently. We use an extension
// to provide a command which works like "git for-each-ref" locally and
// "git ls-remote" when given a remote.
// NOTE: This is a bit clumsy, but it allows us to get most bookmark and
// branch information in a single command, including full hashes, without
// using "--debug" or matching any human readable strings in the output.
$argv = array();
foreach ($api->getMercurialExtensionArguments() as $arg) {
$argv[] = $arg;
}
$argv[] = 'arc-ls-markers';
// NOTE: We can't get branches and bookmarks together in a single command
// because if we query for "heads() + bookmark()", we can't tell if a
// bookmarked result is a branch head or not.
// NOTE: In remote mode, we're using passthru and a tempfile on this
// because it's a remote command and may prompt the user to provide
// credentials interactively. In local mode, we can just read stdout.
$template_fields = array(
'{node}',
'{branch}',
'{join(bookmarks, "\3")}',
'{activebookmark}',
'{desc}',
);
$expect_fields = count($template_fields);
if ($remote !== null) {
$tmpfile = new TempFile();
Filesystem::remove($tmpfile);
$template = implode('\2', $template_fields).'\1';
if ($is_bookmarks) {
$query = hgsprintf('bookmark()');
} else {
$query = hgsprintf('head()');
$argv[] = '--output';
$argv[] = phutil_string_cast($tmpfile);
}
$future = $api->newFuture(
'log --rev %s --template %s --',
$query,
$template);
$argv[] = '--';
list($lines) = $future->resolve();
$markers = array();
$lines = explode("\1", $lines);
foreach ($lines as $line) {
if (!strlen(trim($line))) {
continue;
if ($remote !== null) {
$argv[] = $remote->getRemoteName();
}
$fields = explode("\2", $line, $expect_fields);
$actual_fields = count($fields);
if ($actual_fields !== $expect_fields) {
if ($remote !== null) {
$passthru = $api->newPassthru('%Ls', $argv);
$err = $passthru->execute();
if ($err) {
throw new Exception(
pht(
'Unexpected number of fields in line "%s", expected %s but '.
'found %s.',
$line,
new PhutilNumber($expect_fields),
new PhutilNumber($actual_fields)));
'Call to "hg arc-ls-markers" failed with error "%s".',
$err));
}
$node = $fields[0];
$branch = $fields[1];
if (!strlen($branch)) {
$branch = 'default';
}
if ($is_bookmarks) {
$bookmarks = $fields[2];
if (strlen($bookmarks)) {
$bookmarks = explode("\3", $fields[2]);
$raw_data = Filesystem::readFile($tmpfile);
unset($tmpfile);
} else {
$bookmarks = array();
$future = $api->newFuture('%Ls', $argv);
list($raw_data) = $future->resolve();
}
if (strlen($fields[3])) {
$active_bookmark = $fields[3];
} else {
$active_bookmark = null;
}
} else {
$bookmarks = array();
$active_bookmark = null;
}
$items = phutil_json_decode($raw_data);
$message = $fields[4];
$message_lines = phutil_split_lines($message, false);
$commit_ref = $api->newCommitRef()
->setCommitHash($node)
->attachMessage($message);
$template = id(new ArcanistMarkerRef())
->setCommitHash($node)
->setSummary(head($message_lines))
->attachCommitRef($commit_ref);
if ($is_bookmarks) {
foreach ($bookmarks as $bookmark) {
$is_active = ($bookmark === $active_bookmark);
$markers[] = id(clone $template)
->setMarkerType(ArcanistMarkerRef::TYPE_BOOKMARK)
->setName($bookmark)
->setIsActive($is_active);
}
}
if ($is_branches) {
$markers[] = id(clone $template)
->setMarkerType(ArcanistMarkerRef::TYPE_BRANCH)
->setName($branch);
}
}
if ($is_branches) {
$current_hash = $api->getCanonicalRevisionName('.');
foreach ($markers as $marker) {
if ($marker->getMarkerType() !== ArcanistMarkerRef::TYPE_BRANCH) {
$markers = array();
foreach ($items as $item) {
if (!empty($item['isClosed'])) {
// NOTE: For now, we ignore closed branch heads.
continue;
}
if ($marker->getCommitHash() === $current_hash) {
$marker->setIsActive(true);
$node = $item['node'];
if (!$node) {
// NOTE: For now, we ignore the virtual "current branch" marker.
continue;
}
switch ($item['type']) {
case 'branch':
$marker_type = ArcanistMarkerRef::TYPE_BRANCH;
break;
case 'bookmark':
$marker_type = ArcanistMarkerRef::TYPE_BOOKMARK;
break;
case 'commit':
$marker_type = null;
break;
default:
throw new Exception(
pht(
'Call to "hg arc-ls-markers" returned marker of unknown '.
'type "%s".',
$item['type']));
}
if ($marker_type === null) {
// NOTE: For now, we ignore the virtual "head" marker.
continue;
}
$commit_ref = $api->newCommitRef()
->setCommitHash($node);
$marker_ref = id(new ArcanistMarkerRef())
->setName($item['name'])
->setCommitHash($node)
->attachCommitRef($commit_ref);
if (isset($item['description'])) {
$description = $item['description'];
$commit_ref->attachMessage($description);
$description_lines = phutil_split_lines($description, false);
$marker_ref->setSummary(head($description_lines));
}
$marker_ref
->setMarkerType($marker_type)
->setIsActive(!empty($item['isActive']));
$markers[] = $marker_ref;
}
return $markers;

View file

@ -8,6 +8,7 @@ abstract class ArcanistRepositoryMarkerQuery
private $names;
private $commitHashes;
private $ancestorCommitHashes;
private $remotes;
final public function withMarkerTypes(array $types) {
$this->markerTypes = array_fuse($types);
@ -19,13 +20,35 @@ abstract class ArcanistRepositoryMarkerQuery
return $this;
}
final public function withRemotes(array $remotes) {
assert_instances_of($remotes, 'ArcanistRemoteRef');
$this->remotes = $remotes;
return $this;
}
final public function withIsActive($active) {
$this->isActive = $active;
return $this;
}
final public function execute() {
$markers = $this->newRefMarkers();
$remotes = $this->remotes;
if ($remotes !== null) {
$marker_lists = array();
foreach ($remotes as $remote) {
$marker_list = $this->newRemoteRefMarkers($remote);
foreach ($marker_list as $marker) {
$marker->attachRemoteRef($remote);
}
$marker_lists[] = $marker_list;
}
$markers = array_mergev($marker_lists);
} else {
$markers = $this->newLocalRefMarkers();
foreach ($markers as $marker) {
$marker->attachRemoteRef(null);
}
}
$api = $this->getRepositoryAPI();
foreach ($markers as $marker) {
@ -65,7 +88,6 @@ abstract class ArcanistRepositoryMarkerQuery
}
}
return $this->sortMarkers($markers);
}
@ -92,4 +114,7 @@ abstract class ArcanistRepositoryMarkerQuery
return isset($this->markerTypes[$marker_type]);
}
abstract protected function newLocalRefMarkers();
abstract protected function newRemoteRefMarkers(ArcanistRemoteRef $remote);
}

View file

@ -17,12 +17,14 @@ final class ArcanistMercurialLocalState
protected function executeSaveLocalState() {
$api = $this->getRepositoryAPI();
// TODO: Fix this.
// TODO: We need to save the position of "." and the current active
// branch, which may be any symbol at all. Both of these can be pulled
// from "hg arc-ls-markers".
}
protected function executeRestoreLocalState() {
$api = $this->getRepositoryAPI();
// TODO: Fix this.
// TODO: In Mercurial, we may want to discard commits we've created.
// $repository_api->execxLocal(

View file

@ -19,16 +19,16 @@ cmdtable = {}
command = registrar.command(cmdtable)
@command(
"arc-ls-remote",
"arc-ls-markers",
[('', 'output', '',
_('file to output refs to'), _('FILE')),
] + cmdutil.remoteopts,
_('[--output FILENAME] [SOURCE]'))
def lsremote(ui, repo, source="default", **opts):
"""list markers in a remote
def lsmarkers(ui, repo, source=None, **opts):
"""list markers
Show the current branch heads and bookmarks in a specified path/URL or the
default pull location.
Show the current branch heads and bookmarks in the local working copy, or
a specified path/URL.
Markers are printed to stdout in JSON.
@ -37,14 +37,128 @@ def lsremote(ui, repo, source="default", **opts):
Returns 0 if listing the markers succeeds, 1 otherwise.
"""
if source is None:
markers = localmarkers(ui, repo)
else:
markers = remotemarkers(ui, repo, source, opts)
json_opts = {
'indent': 2,
'sort_keys': True,
}
output_file = opts.get('output')
if output_file:
if os.path.exists(output_file):
raise error.Abort(_('File "%s" already exists.' % output_file))
with open(output_file, 'w+') as f:
json.dump(markers, f, **json_opts)
else:
print json.dumps(markers, output_file, **json_opts)
return 0
def localmarkers(ui, repo):
markers = []
active_node = repo['.'].node()
all_heads = set(repo.heads())
current_name = repo.dirstate.branch()
saw_current = False
saw_active = False
branch_list = repo.branchmap().iterbranches()
for branch_name, branch_heads, tip_node, is_closed in branch_list:
for head_node in branch_heads:
is_active = (head_node == active_node)
is_tip = (head_node == tip_node)
is_current = (branch_name == current_name)
if is_current:
saw_current = True
if is_active:
saw_active = True
if is_closed:
head_closed = True
else:
head_closed = bool(head_node not in all_heads)
description = repo[head_node].description()
markers.append({
'type': 'branch',
'name': branch_name,
'node': node.hex(head_node),
'isActive': is_active,
'isClosed': head_closed,
'isTip': is_tip,
'isCurrent': is_current,
'description': description,
})
# If the current branch (selected with "hg branch X") is not reflected in
# the list of heads we selected, add a virtual head for it so callers get
# a complete picture of repository marker state.
if not saw_current:
markers.append({
'type': 'branch',
'name': current_name,
'node': None,
'isActive': False,
'isClosed': False,
'isTip': False,
'isCurrent': True,
'description': None,
})
bookmarks = repo._bookmarks
active_bookmark = repo._activebookmark
for bookmark_name, bookmark_node in bookmarks.iteritems():
is_active = (active_bookmark == bookmark_name)
description = repo[bookmark_node].description()
if is_active:
saw_active = True
markers.append({
'type': 'bookmark',
'name': bookmark_name,
'node': node.hex(bookmark_node),
'isActive': is_active,
'description': description,
})
# If the current working copy state is not the head of a branch and there is
# also no active bookmark, add a virtual marker for it so callers can figure
# out exactly where we are.
if not saw_active:
markers.append({
'type': 'commit',
'name': None,
'node': node.hex(active_node),
'isActive': False,
'isClosed': False,
'isTip': False,
'isCurrent': True,
'description': repo['.'].description(),
})
return markers
def remotemarkers(ui, repo, source, opts):
# Disable status output from fetching a remote.
ui.quiet = True
markers = []
source, branches = hg.parseurl(ui.expandpath(source))
remote = hg.peer(repo, opts, source)
markers = []
bundle, remotebranches, cleanup = bundlerepo.getremotechanges(
ui,
repo,
@ -73,18 +187,4 @@ def lsremote(ui, repo, source="default", **opts):
'node': node.hex(remotemarks[mark]),
})
json_opts = {
'indent': 2,
'sort_keys': True,
}
output_file = opts.get('output')
if output_file:
if os.path.exists(output_file):
raise error.Abort(_('File "%s" already exists.' % output_file))
with open(output_file, 'w+') as f:
json.dump(markers, f, **json_opts)
else:
print json.dumps(markers, output_file, **json_opts)
return 0
return markers