This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10407

urlutil: add a new `get_unique_push_path`
ClosedPublic

Authored by marmoute on Apr 14 2021, 7:37 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG368294967c95: urlutil: add a new `get_unique_push_path`
Summary

This function is dedicated to call that needs a single destination. Currently
most caller actually need that since few actually support multiple destinations
(the most importants hg push and hg outgoing do). So having a clear API
point for that will be important when the time comes to have a single [paths]
alias resolving to multiple urls.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

marmoute created this revision.Apr 14 2021, 7:37 PM
pulkit requested changes to this revision.Apr 15 2021, 7:49 AM
pulkit added a subscriber: pulkit.

This needs a better commit message and description.

This revision now requires changes to proceed.Apr 15 2021, 7:49 AM
marmoute added a comment.Apr 15 2021, 10:30 AM
In D10407#158466, @pulkit wrote:

This needs a better commit message and description.

woops, looks like I forgot that one while writing description.

marmoute retitled this revision from get_unique_push_path to urlutil: add a new `get_unique_push_path`.Apr 15 2021, 10:37 AM
marmoute edited the summary of this revision. (Show Details)
marmoute updated this revision to Diff 26942.
marmoute added a comment.Apr 15 2021, 10:40 AM

This should be fixed

pulkit accepted this revision.Apr 15 2021, 11:07 AM
pulkit added inline comments.
mercurial/utils/urlutil.py
489

AssertionError does not look friendly here. Can you follow up with error.Abort() or something else here?

This revision is now accepted and ready to land.Apr 15 2021, 11:07 AM
marmoute added inline comments.Apr 16 2021, 5:15 AM
mercurial/utils/urlutil.py
489

This is made into a proper error late down the stack. When this get in an actual risk of being multiple value.

marmoute added a commit: rHG368294967c95: urlutil: add a new `get_unique_push_path`.Apr 17 2021, 6:11 PM
Closed by commit rHG368294967c95: urlutil: add a new `get_unique_push_path` (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/utils/urlutil.py (19 lines)

Diff 26863

mercurial/utils/urlutil.py

"""yields all the `(path, branch)` selected as pull source by `sources`""" """yields all the `(path, branch)` selected as pull source by `sources`"""
if not sources: if not sources:
sources = [b'default'] sources = [b'default']
for source in sources: for source in sources:
url = ui.expandpath(source) url = ui.expandpath(source)
yield parseurl(url, default_branches) yield parseurl(url, default_branches)
def get_unique_push_path(action, repo, ui, dest=None):
"""return a unique `path` or abort if multiple are found
This is useful for command and action that does not support multiple
destination (yet).
Note that for now, we cannot get multiple destination so this function is "trivial".
The `action` parameter will be used for the error message.
"""
if dest is None:
dests = []
else:
dests = [dest]
dests = list(get_push_paths(repo, ui, dests))
assert len(dests) == 1
pulkitUnsubmitted
Not Done

AssertionError does not look friendly here. Can you follow up with error.Abort() or something else here?

pulkit: `AssertionError` does not look friendly here. Can you follow up with `error.Abort()` or…
marmouteAuthorUnsubmitted
Done

This is made into a proper error late down the stack. When this get in an actual risk of being multiple value.

marmoute: This is made into a proper error late down the stack. When this get in an actual risk of being…
return dests[0]
def get_unique_pull_path(action, repo, ui, source=None, default_branches=()): def get_unique_pull_path(action, repo, ui, source=None, default_branches=()):
"""return a unique `(path, branch)` or abort if multiple are found """return a unique `(path, branch)` or abort if multiple are found
This is useful for command and action that does not support multiple This is useful for command and action that does not support multiple
destination (yet). destination (yet).
Note that for now, we cannot get multiple destination so this function is "trivial". Note that for now, we cannot get multiple destination so this function is "trivial".