diff --git a/src/land/engine/ArcanistMercurialLandEngine.php b/src/land/engine/ArcanistMercurialLandEngine.php --- a/src/land/engine/ArcanistMercurialLandEngine.php +++ b/src/land/engine/ArcanistMercurialLandEngine.php @@ -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 @@ } 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 @@ $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 @@ } protected function selectIntoCommit() { - // Make sure that our "into" target is valid. $log = $this->getLogEngine(); if ($this->getIntoEmpty()) { @@ -385,7 +446,8 @@ $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 @@ $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); + $target_name = $target->getRef(); - $markers = phutil_json_decode($raw_data); + $remote_ref = id(new ArcanistRemoteRef()) + ->setRemoteName($target->getRemote()); - $target_name = $target->getRef(); + $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 @@ } $bookmark = head($bookmarks); - $target_hash = $bookmark['node']; - $is_bookmark = true; + $target_marker = $bookmark; } if ($branches) { @@ -559,13 +589,12 @@ $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 @@ // 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 @@ // 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 @@ $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 @@ 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 @@ '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. + + + } + } diff --git a/src/repository/marker/ArcanistGitRepositoryMarkerQuery.php b/src/repository/marker/ArcanistGitRepositoryMarkerQuery.php --- a/src/repository/marker/ArcanistGitRepositoryMarkerQuery.php +++ b/src/repository/marker/ArcanistGitRepositoryMarkerQuery.php @@ -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 @@ return $matches[1]; } + protected function newRemoteRefMarkers(ArcanistRemoteRef $remote) { + throw new PhutilMethodNotImplementedException(); + } + } diff --git a/src/repository/marker/ArcanistMarkerRef.php b/src/repository/marker/ArcanistMarkerRef.php --- a/src/repository/marker/ArcanistMarkerRef.php +++ b/src/repository/marker/ArcanistMarkerRef.php @@ -7,6 +7,7 @@ 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 @@ return array( $this->newHardpoint(self::HARDPOINT_COMMITREF), $this->newHardpoint(self::HARDPOINT_WORKINGCOPYSTATEREF), + $this->newHardpoint(self::HARDPOINT_REMOTEREF), ); } @@ -167,4 +169,12 @@ 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); + } + } diff --git a/src/repository/marker/ArcanistMercurialRepositoryMarkerQuery.php b/src/repository/marker/ArcanistMercurialRepositoryMarkerQuery.php --- a/src/repository/marker/ArcanistMercurialRepositoryMarkerQuery.php +++ b/src/repository/marker/ArcanistMercurialRepositoryMarkerQuery.php @@ -3,144 +3,123 @@ final class ArcanistMercurialRepositoryMarkerQuery extends ArcanistRepositoryMarkerQuery { - protected function newRefMarkers() { - $markers = array(); + protected function newLocalRefMarkers() { + return $this->newMarkers(); + } - if ($this->shouldQueryMarkerType(ArcanistMarkerRef::TYPE_BRANCH)) { - $markers[] = $this->newBranchOrBookmarkMarkers(false); - } + protected function newRemoteRefMarkers(ArcanistRemoteRef $remote = null) { + return $this->newMarkers($remote); + } + + private function newMarkers(ArcanistRemoteRef $remote = null) { + $api = $this->getRepositoryAPI(); + + // 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. - if ($this->shouldQueryMarkerType(ArcanistMarkerRef::TYPE_BOOKMARK)) { - $markers[] = $this->newBranchOrBookmarkMarkers(true); + $argv = array(); + foreach ($api->getMercurialExtensionArguments() as $arg) { + $argv[] = $arg; } + $argv[] = 'arc-ls-markers'; - return array_mergev($markers); - } + // 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. - private function newBranchOrBookmarkMarkers($is_bookmarks) { - $api = $this->getRepositoryAPI(); + if ($remote !== null) { + $tmpfile = new TempFile(); + Filesystem::remove($tmpfile); - $is_branches = !$is_bookmarks; + $argv[] = '--output'; + $argv[] = phutil_string_cast($tmpfile); + } - // 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[] = '--'; - // 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. + if ($remote !== null) { + $argv[] = $remote->getRemoteName(); + } - $template_fields = array( - '{node}', - '{branch}', - '{join(bookmarks, "\3")}', - '{activebookmark}', - '{desc}', - ); - $expect_fields = count($template_fields); + if ($remote !== null) { + $passthru = $api->newPassthru('%Ls', $argv); - $template = implode('\2', $template_fields).'\1'; + $err = $passthru->execute(); + if ($err) { + throw new Exception( + pht( + 'Call to "hg arc-ls-markers" failed with error "%s".', + $err)); + } - if ($is_bookmarks) { - $query = hgsprintf('bookmark()'); + $raw_data = Filesystem::readFile($tmpfile); + unset($tmpfile); } else { - $query = hgsprintf('head()'); + $future = $api->newFuture('%Ls', $argv); + list($raw_data) = $future->resolve(); } - $future = $api->newFuture( - 'log --rev %s --template %s --', - $query, - $template); - - list($lines) = $future->resolve(); + $items = phutil_json_decode($raw_data); $markers = array(); - - $lines = explode("\1", $lines); - foreach ($lines as $line) { - if (!strlen(trim($line))) { + foreach ($items as $item) { + if (!empty($item['isClosed'])) { + // NOTE: For now, we ignore closed branch heads. continue; } - $fields = explode("\2", $line, $expect_fields); - $actual_fields = count($fields); - if ($actual_fields !== $expect_fields) { - 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))); + $node = $item['node']; + if (!$node) { + // NOTE: For now, we ignore the virtual "current branch" marker. + continue; } - $node = $fields[0]; - - $branch = $fields[1]; - if (!strlen($branch)) { - $branch = 'default'; + 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 ($is_bookmarks) { - $bookmarks = $fields[2]; - if (strlen($bookmarks)) { - $bookmarks = explode("\3", $fields[2]); - } else { - $bookmarks = array(); - } - - if (strlen($fields[3])) { - $active_bookmark = $fields[3]; - } else { - $active_bookmark = null; - } - } else { - $bookmarks = array(); - $active_bookmark = null; + if ($marker_type === null) { + // NOTE: For now, we ignore the virtual "head" marker. + continue; } - $message = $fields[4]; - $message_lines = phutil_split_lines($message, false); - $commit_ref = $api->newCommitRef() - ->setCommitHash($node) - ->attachMessage($message); + ->setCommitHash($node); - $template = id(new ArcanistMarkerRef()) + $marker_ref = id(new ArcanistMarkerRef()) + ->setName($item['name']) ->setCommitHash($node) - ->setSummary(head($message_lines)) ->attachCommitRef($commit_ref); - if ($is_bookmarks) { - foreach ($bookmarks as $bookmark) { - $is_active = ($bookmark === $active_bookmark); + if (isset($item['description'])) { + $description = $item['description']; + $commit_ref->attachMessage($description); - $markers[] = id(clone $template) - ->setMarkerType(ArcanistMarkerRef::TYPE_BOOKMARK) - ->setName($bookmark) - ->setIsActive($is_active); - } + $description_lines = phutil_split_lines($description, false); + $marker_ref->setSummary(head($description_lines)); } - if ($is_branches) { - $markers[] = id(clone $template) - ->setMarkerType(ArcanistMarkerRef::TYPE_BRANCH) - ->setName($branch); - } - } + $marker_ref + ->setMarkerType($marker_type) + ->setIsActive(!empty($item['isActive'])); - if ($is_branches) { - $current_hash = $api->getCanonicalRevisionName('.'); - - foreach ($markers as $marker) { - if ($marker->getMarkerType() !== ArcanistMarkerRef::TYPE_BRANCH) { - continue; - } - - if ($marker->getCommitHash() === $current_hash) { - $marker->setIsActive(true); - } - } + $markers[] = $marker_ref; } return $markers; diff --git a/src/repository/marker/ArcanistRepositoryMarkerQuery.php b/src/repository/marker/ArcanistRepositoryMarkerQuery.php --- a/src/repository/marker/ArcanistRepositoryMarkerQuery.php +++ b/src/repository/marker/ArcanistRepositoryMarkerQuery.php @@ -8,6 +8,7 @@ private $names; private $commitHashes; private $ancestorCommitHashes; + private $remotes; final public function withMarkerTypes(array $types) { $this->markerTypes = array_fuse($types); @@ -19,13 +20,35 @@ 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 @@ } } - return $this->sortMarkers($markers); } @@ -92,4 +114,7 @@ return isset($this->markerTypes[$marker_type]); } + abstract protected function newLocalRefMarkers(); + abstract protected function newRemoteRefMarkers(ArcanistRemoteRef $remote); + } diff --git a/src/repository/state/ArcanistMercurialLocalState.php b/src/repository/state/ArcanistMercurialLocalState.php --- a/src/repository/state/ArcanistMercurialLocalState.php +++ b/src/repository/state/ArcanistMercurialLocalState.php @@ -17,12 +17,14 @@ 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( diff --git a/support/hg/arc-hg.py b/support/hg/arc-hg.py --- a/support/hg/arc-hg.py +++ b/support/hg/arc-hg.py @@ -19,16 +19,16 @@ 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 @@ 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 @@ '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