( )⚙ D2108 infinitepush: drop the `--to` flag to push and use `-B` instead

This is an archive of the discontinued Mercurial Phabricator instance.

infinitepush: drop the `--to` flag to push and use `-B` instead
ClosedPublic

Authored by pulkit on Feb 9 2018, 6:47 AM.

Details

Summary

The extension added a --to flag to specify the bookmark to which revs should
be pushed. This patch deletes that flag and instead uses the -B flag. After
this patch, bookmark passed as -B is parsed and if it matches the infinitepush
bookmark pattern, we consider that push as infinitepush.

This is still not the best of what we can do. Later patches in the series will
drop the use of -B flag and will instead handle things at bookmark bundle2
part. Plugging these logic to bookmark bundle2 part will also get rid of the
scratchbranchparttype bundle2 part.

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

pulkit created this revision.Feb 9 2018, 6:47 AM
pulkit added a subscriber: durham.Feb 9 2018, 7:03 AM

There is still some work required to make this extension better. I didn't want to grow my stack too long and hence send upto here. Few things which will be coming for sure:

  • removal of wrapping of push command on client side
  • using the bookmarks bundle2 part instead of scratchbranchparttype on both client and server side
  • drop the scratchbranchparttype bundle2 part
  • dropping the debugfillinfinitepushmetadata command defined in infinitepushcommands.py

There are things which I am not sure whether to keep or not:

  • the --bundle-store flag to push command
  • functionality to pull from bundlestore using hg pull
  • functionality to pull changesets from bundlestore if a changeset is not found locally on hg update
  • logic around sql store
  • interaction with the hoisting functionality of remotenames extension which is also being moved to core

Once these things are figured out, then I will try to add support for old clients and the clients on which we have no control. Also I will like to get these reviews less painful as much possible, so if there is any another way which can make the review easy, I will be happy to follow.

cc: @durham

indygreg accepted this revision.Feb 9 2018, 10:15 PM
indygreg added a subscriber: indygreg.

I'm find with you sending more parts to this series. Especially if they delete code: if they delete code then my review of the import will be a review of the final state of the code post deletions.

This revision is now accepted and ready to land.Feb 9 2018, 10:15 PM

There are things which I am not sure whether to keep or not:

  • the --bundle-store flag to push command

This is useful for scripts or tools that want to upload a commit to the cloud without having to give it a name. For instance, you can use it to push a commit then send that commit hash to some build service which can checkout the commit without having to worry about a bookmark name. But this could always be added back later, so it's probably fine to drop it if there's not an immediate need in Mozilla's use case.

  • functionality to pull from bundlestore using hg pull

Similar to the points above and below, this is useful for automation that already passes hashes around. Not having to pass around bookmark names as well means it's easier for that automation to migrate to infinitepush.

  • functionality to pull changesets from bundlestore if a changeset is not found locally on hg update

This is a bit of magic that user's really like. When combined with automatic backup pushes, it makes it feel like everyone is using the same repository. I'd highly recommend keeping this just for the eventual PR of saying "I can just hg commit, and my friend can do hg checkout HASH"

  • logic around sql store

Without this, would the server always store data in the filesystem? The sql store seems like an important bit of making this robust in enterprise usage.

  • interaction with the hoisting functionality of remotenames extension which is also being moved to core

I'm not familiar with how infinitepush plays into hoisting, but I just wanted to make sure users never have to type 'remote/' or 'default/'.

In D2108#36335, @durham wrote:

There are things which I am not sure whether to keep or not:

  • the --bundle-store flag to push command

This is useful for scripts or tools that want to upload a commit to the cloud without having to give it a name. For instance, you can use it to push a commit then send that commit hash to some build service which can checkout the commit without having to worry about a bookmark name. But this could always be added back later, so it's probably fine to drop it if there's not an immediate need in Mozilla's use case.

To be clear, Mozilla has 2 use cases where infinitepush could be useful:

  1. For our Try repository. Upload a nameless bundle somewhere and CI consumes it.
  2. For user repositories (basically forks of the main repos).

I'm find with you sending more parts to this series. Especially if they delete code: if they delete code then my review of the import will be a review of the final state of the code post deletions.

With discussions on D2096 and this patch, understanding your requirements, I think the wrapping of push command, the scratchbranchparttype bundle2 part etc. will be useful. So, can you review the current series with this patch as the last one?

This revision was automatically updated to reflect the committed changes.