( )⚙ D3047 push: avoid using repo.lookup() for converting to nodeid

This is an archive of the discontinued Mercurial Phabricator instance.

push: avoid using repo.lookup() for converting to nodeid
ClosedPublic

Authored by martinvonz on Apr 3 2018, 6:08 PM.

Details

Summary

repo.lookup(x) currently simply does repo[x].node(), which supports
various types of inputs. As I explained in 0194dac77c93 (scmutil: add
method for looking up a context given a revision symbol, 2018-04-02),
I'd like to split that up so we use the new scmutil.revsymbol() for
string inputs repo[x] for integer revnums and binary nodeids. Since
repo.lookup() seems to exist in order to serve peer.lookup(), I think
it should be calling revsymbol. However, we have several callers that
use repo.lookup() with something that's not a string, so we need to
remove those first. This patch starts doing that. Many more will
follow.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Apr 3 2018, 6:08 PM
indygreg accepted this revision.Apr 3 2018, 7:09 PM
This revision is now accepted and ready to land.Apr 3 2018, 7:09 PM

I'll likely take this series. But part of me wonders if we should be performing conversion on the changelog instead. I guess it depends on whether the string inputs should resolve names and other things not known to the changelog itself. For converting integer revision numbers to nodes, I would argue we should go through the changelog.

Anyway, we'll likely have a bikeshed about all of this once we establish a formal interface for the changelog. That's a bit of a ways off, however. These changes seem fine for today.

I'll likely take this series. But part of me wonders if we should be performing conversion on the changelog instead. I guess it depends on whether the string inputs should resolve names and other things not known to the changelog itself. For converting integer revision numbers to nodes, I would argue we should go through the changelog.
Anyway, we'll likely have a bikeshed about all of this once we establish a formal interface for the changelog. That's a bit of a ways off, however. These changes seem fine for today.

Sure, we can talk about that later :) I think I'm at least moving us closer to that by moving out the handling of user-supplied strings.

This revision was automatically updated to reflect the committed changes.