The checkheads function is long and complex, extract that logic in a subfunction
is win in itself. As the comment in the code says, this postprocessing is
currently very basic and either misbehave or fails to detect valid push in many
cases. My deeper motive for this extraction is to be make it easier to provide
extensive testing of this case and strategy to cover them. Final test and logic
will makes it to core once done.
c8fefe878a71 introduced default params for discovery.outgoing(), but it used a
falsy check instead of an explicit check for None. The result is that callers
that passed in an empty list would have that list overridden by the defaults,
which is not the expected behavior.
This was discovered by changes to the test-pushrebase.t test in Facebook's
repository of mercurial extensions.
This argument can be used instead of 'commonheads' to determine the 'outgoing'
set. We remove the outgoingbetween function as its role can now be handled by
'outgoing' itself.
I've thought of using an external function instead of making the constructor
more complicated. However, there is low hanging fruit to improve the current
code flow by storing some side products of the processing of 'missingroots'. So
in my opinion it make senses to add all this to the class.
We are about to introduce a third option to create an outgoing object:
'missingroots'. This argument will be mutually exclusive with 'commonheads' so
we implement some default value handling in preparation.
This will also help use to make more use of outgoing creation around the code
base.
We are to introduce more code constructing such object in the code base. It will
be more convenient to pass a repository object, all current users already
operate at the repository level anyway. More changes to the contructor argument
are coming in later changeset.
changegroup.changegroupsubset() contained somewhat low-level code for
constructing an "outgoing" instance from a list of roots and heads
nodes. It feels like discovery.py is a more appropriate location for
this code.
This code can definitely be optimized, as outgoing.missing will
recompute the set of changesets we've already discovered from
cl.between(). But code shouldn't be refactored during a move, so
I've simply inserted a TODO calling attention to that.
Before 'hg push -B .' on new remote head complained with:
abort: push creates new remote head ...
It was because _nowarnheads was not expanding active bookmark
name, so it didn't add active bookmark "proper" name to no
warn heads list.
Previously, we passed only enough information to _nowarnheads to perform the old
calculation. However, extensions may want to use more information in this
calculation. Now that pushop is extensible, extensions will often want access
to the additional fields they have added to the pushop, so pass that through.
Specifically, the remotenames extension will use this information to allow
pushing new heads that are bookmarked without requiring --force.
Previously, we passed a bunch of parameters to discovery.checkheads, but all
of the arguments can be fetched out of pushop, which may contain a lot more
useful information for extensions now that pushop is extensible.
In addition to taking a step towards getting an unreasonably large function
factored into smaller, more manageable functions, this will allow extensions
such as remotenames have more control over what pushes are allowed or not.
Having the simple and tiny branch of the conditional first help readability. The
"else" that appears after a screen of code is harder to relate to a conditional.
The home of 'Abort' is 'error' not 'util' however, a lot of code seems to be
confused about that and gives all the credit to 'util' instead of the
hardworking 'error'. In a spirit of equity, we break the cycle of injustice and
give back to 'error' the respect it deserves. And screw that 'util' poser.
For great justice.
In repos with many changesets, the computation of allfuturecommon
can take a significant amount of time. Since it's only used if
there's an obsstore, don't compute it otherwise.
Although Python supports `X = Y if COND else Z`, this was only
introduced in Python 2.5. Since we have to support Python 2.4, it was
a very common thing to write instead `X = COND and Y or Z`, which is a
bit obscure at a glance. It requires some intricate knowledge of
Python to understand how to parse these one-liners.
We change instead all of these one-liners to 4-liners. This was
executed with the following perlism:
find -name "*.py" -exec perl -pi -e 's,(\s*)([\.\w]+) = \(?(\S+)\s+and\s+(\S*)\)?\s+or\s+(\S*)$,$1if $3:\n$1 $2 = $4\n$1else:\n$1 $2 = $5,' {} \;
I tweaked the following cases from the automatic Perl output:
prev = (parents and parents[0]) or nullid
port = (use_ssl and 443 or 80)
cwd = (pats and repo.getcwd()) or ''
rename = fctx and webutil.renamelink(fctx) or []
ctx = fctx and fctx or ctx
self.base = (mapfile and os.path.dirname(mapfile)) or ''
I also added some newlines wherever they seemd appropriate for readability
There are probably a few ersatz ternary operators still in the code
somewhere, lurking away from the power of a simple regex.
When a remote is not capable of the `branchmap` wireproto command, we denote
incoming heads with None. This leads to a crash in the code in charge of
displaying the list of unknown remote heads.
We now properly detect this case and display a shorter message in this case.
The reason for this `set([None])` value is now documented.
This situation is not necessarily a problem and do not deserve a warning. It is
just some information that can guide the user in understanding what is going
on.
Making it 'debug' would usually not give the hint when it is relevant so we
make it a 'status' message.
This message frequently caused confusion. "unsynced" is not a well established
user-facing concept in Mercurial and the message was not very specific or
helpful.
Instead, show a message like:
remote has heads on branch 'default' that are not known locally: 6c0482d977a3
This message will also (when relevant) be shown before aborting on "push
creates new remote head".
A similar (but actually very different) message was addressed in cbd5a12a601a.
This note (which actually is a warning) frequently caused confusion.
"unsynced" is not a well established user-facing concept in Mercurial and the
message was not very specific or helpful.
Instead, show a messages like:
remote has heads on branch 'default' that are not known locally: 6c0482d977a3
and show it before aborting on "push creates new remote head". This will also
give more of a hint in the case where the branch has been closed remotely and
'hg heads' thus not would show any new heads after pulling.
A similar (but actually very different) message was addressed in cbd5a12a601a.
Nodemap is not aware of filtering so we need to ask the changelog itself if a
node is known. This is probably a bit slower but such check does not dominated
discovery time. This is necessary if we want to run discovery on filtered repo.
Push is currently allowed to create a new head if there is a remote
bookmark that will be updated to point to the new head. If the
bookmark is not known remotely then push aborts, even if a -B argument
is about to push the bookmark. This change allows push to continue in
this case. This does not require a wireproto force.
This patch revises hint message from "for detail about" introduced by
changeset 49ed20ea8da0 to "for details about", to unify it with the
hint message introduced by proceeding patch.
"use push -f to force" in the hint at abortion of "hg push" may cause
novice users to execute "push -f" easily without understanding about
problems of multiple branch heads in the repository.
This patch hides description about "-f" in the hint, and leads into
seeing "hg help push" for details about pushing new heads.
Before this patch, pushing with --new-branch permits to create
multiple headed branch on the destination repository.
But permitting to create new branch should be different from
permitting to create multiple heads on branch.
This patch prevents from careless pushing multiple headed new branch,
and requires --force to push such branch forcibly.
Now that changelog filtering is in place, it's become evident that
naming the filters according to the set of revs _not_ included in the
filtered changelog is confusing. This is especially evident in the
collaborative branch cache scheme.
This changes the names of the filters to reflect the revs that _are_
included:
hidden -> visible
unserved -> served
mutable -> immutable
impactable -> base
repoview.filteredrevs is renamed to filterrevs, so that callers read a
bit more sensibly, e.g.:
filterrevs('visible') # filter revs according to what's visible
Creation of changectx objects is very slow, and they are not very
useful. We are going to drop them. The first step is to change the
function argument type.
Value and key of branchcache would benefit from being hold by the same object.
Moreover some logic (update, write, validation) could be move on such object.
The creation of this object is the first step toward this move. The result will
clarify branchcache related code and hide most of the detail in the class
itself. This encapsulation will greatly helps implementation of branchcache for
filtered view of the repo.
This is the second real use of changelog filtering. The change is very small to
allow testing the new filter with a setup close to the original one.
We replace custom post processing on `heads`function by call to the standard
code pass on a filtering repo.
In later coming will have wider usage of filtering that will make the dedicated
function useless.
Here is the first real use of changelog filtering. The change is very small to
allow testing the new filter with a setup close to the original one.
We replace custom post processing on branchmap function by call to the
standard code pass on a filtering repo.
In later coming will have wider usage of filtering that will make the dedicated
function useless.
We usually update bookmarks only if the new location is descendant of the old
bookmarks location. We extract this logic into a function. This is the first
step to allow more complex logic using obsolescence in this validation of the
bookmark movement.
