This was leading to some difficult to trace problems because the
values were set in one place, but then blew up much later in the
program. Exploding violently with an assertion seems reasonable here.
The _first field is used for tracking when to emit a separator between
items. It seems like it's clearly formatter state, not ui state, so
let's move it there.
Default-push has been deprecated in favour of default:pushurl. But "hg clone" still
inserts this in every hgrc file it creates. This patch updates the message by replacing
default-push with default:pushurl and also makes the necessary changes to test files.
Before this patch, worker implementation assumes that os.waitpid()
with os.WNOHANG returns '(0, 0)' for still running child process. This
is explicitly specified as below in Python API document.
os.WNOHANG
The option for waitpid() to return immediately if no child
process status is available immediately. The function returns
(0, 0) in this case.
On the other hand, POSIX specification doesn't define the "stat_loc"
value returned by waitpid() with WNOHANG for such child process.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/waitpid.html
CPython implementation for os.waitpid() on POSIX doesn't take any care
of this gap, and this may cause unexpected "exit status indication"
even on POSIX conformance platform.
For example, os.waitpid() with os.WNOHANG returns non-zero "exit
status indication" on FreeBSD. This implies os.kill() with own pid or
sys.exit() with non-zero exit code, even if no child process fails.
To ignore meaningless exit status indication returned by os.waitpid(),
this patch skips subsequent steps forcibly, if os.waitpid() returns 0
as pid.
This patch also arranges examination of 'p' value for readability.
FYI, there are some issues below about this behavior reported for
CPython.
https://bugs.python.org/issue21791https://bugs.python.org/issue27808
Previously Abort raised during 'getbundle' call poorly reported (HTTP-500 for
http, some scary messages for ssh). Abort error have been properly reported for
"push" for a long time, there is not reason to be different for 'getbundle'. We
properly catch such error and report them back the best way available. For
bundle, we issue a valid bundle2 reply (as expected by the client) with an
'error:abort' part. With bundle1 we do as best as we can depending of http or
ssh.
bundle2 allow the server to report error explicitly. This was initially
implemented for push but there is not reason to not use it for pull too. This
changeset add logic similar to the one in 'unbundle' to the
client side of 'getbundle'. That logic make sure the error is properly reported
as "remote". This will allow the server side of getbundle to send clean "Abort"
message in the next changeset.
Changeset a0966f529e1b introduced a config option to have the server deny pull
using bundle1. The original protocol has not really been design to allow that
kind of error reporting so some hack was used. It turned the hack only works on
HTTP and that ssh server hangs forever when this is used. After further
digging, there is no way to report the error in a unified way. Using `ooberror`
freeze ssh and raising 'Abort' makes HTTP return a HTTP-500 without further
details. So with sadness we implement a version that dispatch according to the
protocol used.
Now the error is properly reported, but we still have ungraceful abort after
that. The protocol do not allow anything better to happen using bundle1.
Changeset a0966f529e1b introduced a config option to have the server deny push
using bundle1. The original protocol has not really be design to allow such kind
of error reporting so some hack was used. It turned the hack only works on HTTP
and that ssh wire peer hangs forever when the same hack is used. After further
digging, there is no way to report the error in a unified way. Using 'ooberror'
freeze ssh and raising 'Abort' makes HTTP return a HTTP500 without further
details. So with sadness we implement a version that dispatch according to the
protocol used.
We also add a test for pushing over ssh to make sure we won't regress in the
future. That test show that the hint is missing, this is another bug fixed in
the next changeset.
The remote hint message was ignored when reporting the remote error and
passed to the local generic abort error. I think I might initially have
tried to avoid reimplementing logic controlling the hint display depending of
the verbosity level. However, first, there does not seems to have such verbosity
related logic and second the resulting was wrong as the primary error and the
hint were split apart. We now properly print the hint as remote output.
This patch also makes some expected output lines in tests glob-ed for
persistence of them.
BTW, files below aren't yet changed in 2017, but this patch also
updates copyright of them, because:
- mercurial/help/hg.1.txt
almost all of "man hg" output comes from online help of hg
command, and is already changed in 2017
- mercurial/help/hgignore.5.txt
- mercurial/help/hgrc.5
"copyright 2005-201X Matt Mackall" in them mentions about
copyright of Mercurial itself
In a flat manifest, a node with the same content but different parents is still
considered a new node. In the current tree manifests however, if the content is
the same, we ignore the parents entirely and just reuse the existing node.
In our external treemanifest extension, we want to allow having one treemanifest
for every flat manifests, as a way of easeing the migration to treemanifests. To
make this possible, let's change the root node treemanifest behavior to match
the behavior for flat manifests, so we can have a 1:1 relationship.
While this sounds like a BC breakage, it's not actually a state users can
normally get in because: A) you can't make empty commits, and B) even if you try
to make an empty commit (by making a commit then amending it's changes away),
the higher level commit logic in localrepo.commitctx() forces the commit to use
the original p1 manifest node if no files were changed. So this would only
affect extensions and automation that reached passed the normal
localrepo.commit() logic straight into the manifest logic.
Mercurial 3.9 added the [hostsecurity] section, which is better
than [hostfingerprints] in every way.
One of the ways that [hostsecurity] is better is that it supports
SHA-256 and SHA-512 fingerprints, not just SHA-1 fingerprints.
The world is moving away from SHA-1 because it is borderline
secure. Mercurial should be part of that movement.
This patch adds a warning when a valid SHA-1 fingerprint from
the [hostfingerprints] section is being used. The warning informs
users to switch to [hostsecurity]. It even prints the config
option they should set. It uses the SHA-256 fingerprint because
recommending a SHA-1 fingerprint in 2017 would be ill-advised.
The warning will print itself on every connection to a server until
it is fixed. There is no way to suppress the warning. I admit this
is annoying. But given the security implications of sticking with
SHA-1, I think this is justified. If this patch is accepted,
I'll likely send a follow-up to start warning on SHA-1
certificates in [hostsecurity] as well. Then sometime down
the road, we can drop support for SHA-1 fingerprints.
Credit for this idea comes from timeless in issue 5466.
0b5f1f2efc77 introduced handling of a crash in this case. A review comment
suggested that it was not entirely obvious that a 'dm' always would have a 'r'
for the source file.
To mitigate that risk, make the code more conservative and make less
assumptions.
Work around that 'dm' in the data model only can have one operation for the
target file, but still can have multiple and conflicting operations on the
source file where the other operation is a 'rm'. The move would thus fail with
'abort: No such file or directory'.
In this case it is "obvious" that the file should be removed, either before or
after moving it. We thus keep the 'rm' of the source file but drop the 'dm'.
This is not a pretty fix but quite "obviously" safe (famous last words...) as
it only touches a rare code path that used to crash. It is possible that it
would be better to swap the files for 'dm' as suggested on
https://bz.mercurial-scm.org/show_bug.cgi?id=5020#c13 but it is not entirely
obvious that it not just would create conflicts on the other file. That can be
revisited later.
dict.keys() is documented to return a copy, so it's surprising that
sortdict.keys() did not. I noticed this because we have an extension
that calls readlocaltags(). That method tries to remove any tags that
point to non-existent revisions (most likely stripped). However, since
it's unintentionally working on the instance it's modifying, it
sometimes fails to remove tags when there are multiple bad tags in a
row. This was not caught because localrepo.tags() does an additional
layer of filtering.
sortdict is also used in other places, but I have not checked whether
its keys() and/or __delitem__() methods are used there.
outgoing() and remote() may stall for long due to network I/O, which seems
unsafe per definition, "whether a predicate is safe for DoS attack." But I'm
not 100% sure about this. If our concern isn't elapsed time but CPU resource,
these predicates are considered safe. Perhaps that would be up to the
web/application server configuration?
Anyway, outgoing() and remote() wouldn't be useful in hgweb, so I think
it's okay to ban them.
statprof has a __main__ handler that allows viewing of previously
written data files. As Yuya pointed out during review, 82ee01726a77
broke this. This patch fixes that.
Until callsites are updated, this will have no effect. Once callsites
are updated, specifying experimental.editortmpinhg will create editor
temporary files in a subdirectory of .hg, which will make it easier
for tool integrations to determine what repository is in play when
they're asked to edit an hg-related file.
