This patch series is intended to allow bundle2 push reply part handlers to
make changes to the local repository; it has been developed in parallel with
an extension that allows the server to rebase incoming changesets while applying
them.
This diff adds an experimental config option "bundle2.pushback" which provides
a transaction to the reply unbundler during a push operation. This behavior is
opt-in because of potential security issues: the response can contain any part
type that has a handler defined, allowing the server to make arbitrary changes
to the local repository.
This patch series is intended to allow bundle2 push reply part handlers to
make changes to the local repository; it has been developed in parallel with
an extension that allows the server to rebase incoming changesets while applying
them.
Most pushes already open a transaction in order to sync phase information.
This diff replaces that transaction with one that spans the entire push
operation.
This transaction will be used in a later patch to guard repository changes
made during the reply handler.
This patch series is intended to allow bundle2 push reply part handlers to
make changes to the local repository; it has been developed in parallel with
an extension that allows the server to rebase incoming changesets while applying
them.
Aside from the transaction logic, the pulloperation class is used primarily as
a logic-free data structure for storing state information. This diff extracts
the transaction logic into its own class that can be shared with push
operations.
The phase-syncing code was using bundle2 if the remote supported it. It was
doing so without regard to bundle2 activation on the client. Moreover, the
phase push is now properly included in the unified bundle2 push, so having extra
code in syncphase should be useless. If the remote is bundle2-enabled, the
phases should already be synced.
The buggy verification code was leading to a crash when a 3.2 client was pushing
to a 3.1 server. The real bundle2 path detected that their versions were
incompatible, but the syncphase code failed to, sending an incompatible bundle2
to the server.
We drop the useless and buggy code as a result. The "else" clause is
de-indented in the process.
This mirrors the API for 'pending' and 'finalize' callbacks. I do not have
immediate usage planned for it, but I'm sure some callback will be happy to
access transaction related data.
Changeset d79feb65f3ee added advertising of supported changegroup version
through the new 'b2x:changegroup' capability. However, this capability is not
new and has been around since 3.1 with an empty value. This makes new clients
unable to push to 3.2 servers through bundle2 as they cannot find a common
changegroup version to use from and empty list.
Treating empty 'b2x:changegroup' value as old client fixes it.
The 'delayupdate' method now takes a transaction object and registers its
'_writepending' method for execution in 'transaction.writepending()'. The hook can then
use 'transaction.writepending()' directly.
At some point this will allow the addition of other file creation
during writepending.
When using bundle2, we find the common subset of supported changegroup-packers
and we pick the max of them. This allow to use generaldelta aware changegroups through
bundle2.
When using bundle2, we find the common subset of supported changegroup-packers
and we pick the max of them. This allow to use generaldelta aware changegroup
through bundle2.
The commands getchangegroup, getlocalchangegroup and getsubset now each
have a version ending in -raw. The raw versions return the chunk generator
from the changegroup packer directly, without wrapping it in a chunkbuffer
and unpacker. This avoids extra chunkbuffers in the bundle2 code path.
Also, the raw versions can be extended to support alternative packers
in the future, to be used from bundle2.
48062b2d0f30 regressed the behavior of pushing an unchanged bookmark to
a remote. Before that commit, pushing a unchanged bookmark would result
in "exporting bookmark @" being printed. After that commit, we now see
an incorrect message "bookmark %s does not exist on the local or remote
repository!"
This patch fixes the regression introduced by 48062b2d0f30 by having
the bookmark error reporting code filter identical bookmarks and adds
a test for the behavior.
bookmarks.compare() previously lumped identical bookmarks in the
"invalid" bucket. This patch adds a "same" bucket.
An 8-tuple for holding this state is pretty gnarly. The return value
should probably be converted into a class to increase readability. But
that is beyond the scope of a patch intended to be a late arrival to
stable.
Hooks that run after the transaction need to be able to touch the
repository. So we need to run them after the lock release. This is
similar to what the "changegroup" hook is doing in the
`addchangegroup` function.
We are changing all integers that denote the size of a chunk to read to int32.
There are two main motivations for that.
First, we change everything to the same width (32 bits) to make it possible for
a reasonably agnostic actor to forward a bundle2 without any extra processing.
With this change, this could be achieved by just reading int32s and forwarding
chunks of the size read. A bit a smartness would be logic to detect the end of
stream but nothing too complicated.
Second, we need some capacity to transmit special information during the bundle
processing. For example we would like to be able to raise an exception while a
part is being read if this exception happend while this part was generated.
Having signed integer let us use negative numbers to trigger special events
during the parsing of the bundle.
The format is renamed for B2X to B2Y because this breaks binary
compatibility. The B2X format support is dropped. It was experimental to
allow this kind of things. All elements not directly related to the binary
format remain flagged "b2x" because they are still compatible.
We need a wider set of hooks to process all the changes that happened during the
pull transaction. We reuse the experimental `b2x-transactionclose` hook set
from server's unbundle for consistency. This hook is experimental and will not
remains as-is forever, but this will open the door for experimentation in 3.2.
The source information can, should be applied once when opening the transaction
for the pull. This will lets element processed within a bundle2 be aware of them
and open the door to running a set of hooks when closing this pull transaction.
This is similar to what is done in server's unbundle call.
We store the source and url of the current data into `transaction.hookargs` this
let us inherit it from upper layers that may have created a much wider
transaction. We have to modify bundle2 at the same time to register the source
and url in the transaction. We have to do it in the same patch otherwise, the
`addchangegroup` call would fill these values and the hook calling will crash
because of the duplicated 'source' and 'url' arguments passed to the hook call.
A bundle2 may contain multiple parts adding changegroups, in which case there
are multiple operation records for changegroups, each with its own return
value. Those multiple return values are aggregated in a single cgresult value
for the whole operation.
As can be seen in the associated test case, the situation with hooks is not
really the best, but without deeper thoughts and changes, we can't do much
better. Hopefully, things will be improved before bundle2 is enabled by default.
In the meanwhile, multiple changegroups is not expected to be in widespread
use, and even less expected to be used for pushes. Also, not many clients
cloning or pulling bundle2 with multiple changesets are not expected to have
changegroup hooks anyways.
The push process uses a `stepsdone` attribute instead of a `todosteps` one (with
the logic swapped). We unify the two process by picking the `stepsdone` version.
I feel like `stepsdone` better fits extensions that would want to extend the push
exchange process.
We apply the same approach as for push and make the discovery extensible. There
is only one user in core right now, but we already know we'll need something
smarter for obsmarkers. In fact the evolve extension could use this to cleanly
extend discovery.
The main motivation for this change is consistency between push and pull.
We mimic what was done for `push` for similar reason. We are about to drop
`localrepo.pull` (for consistency with dropping `localrepo.push` and we better
have an API as extensible as `push` is.
Find explanations about localrepo.push removal in 88d9d4ec499e.
Retrieving bookmarks before obsmarkers will avoid turning some changesets hidden
right before making them visible again if a bookmark keeps them visible.
The discovery phases for bookmarks now use the list of explicitly pushed bookmarks
to do addition, removal and overwriting.
Tests are impacted because this reduces the amount of listkeys calls issued, removes
some duplicated messages and improves the accuracy of some messages.
To gather all the bookmark pushing actions together, we need code performing
those actions to be ready for them. We need to be able to produce different
messages for different actions.
There is no reason for bookmarks to get a special treatment. As a first step we
move the code as is in the `exchange.pull` function. Integration with the rest
of the flow will come later.
Adding bookmarks to pull means that most clone paths are now pulling bookmarks
through pull. We ensure that bookmark-update messages are properly suppressed in
that case.
In test-pull-http.t the 'requesting all changes' message disappear because we
now get the authentication error on the `listkeys`command before such message
is printed.
This part is responsible for adding new bookmarks on the remote. Before that,
it was done on its own in `commands.push`. The export is still not integrated
with the rest of the push process, but at least it now dwells in the right
function.
Returning the pushop object gives access to more information (upcoming bookmark
push result for example). `localrepo.push` currently extracts the `cgresult` for
callers.
We are about to introduce more results-related attributes on pushop (for
bookmarks) so we need a more distinctive name. We now use `cgresult` as
`pulloperation` does.
The primary goal is to make it easier for extensions to alter how bundle2
parts are laid out. They now can use the getbundle2partsgenerator decorator
to add new parts, or directly act on getbundle2partsmapping to wrap existing
part functions.
Note the 'request for bundle10 must include changegroup' error was kept
under the same conditions as before, although the logic changes don't make
it obvious.
