- User Since
- Oct 1 2021, 12:37 PM (17 w, 16 h)
Thu, Jan 20
This demonstrates a bug that results in a repo corruption any time a transaction is aborted that adds multiple revs of the same revlog while growing it beyond 128KiB.
I would have thought this affects most hg users, since none of the pre-conditions for this bug is very unique.
Probably this went unnoticed for such a long time simply because transactions are rarely aborted.
Dec 17 2021
Dec 13 2021
Thanks for your review!
The point is to make the output smaller.
Seems ok to output the whole file, too (pushed). (it was 50 lines, not 20, when I introduced it)
Dec 10 2021
The shell strikes again. :-\
I removed the problematic test, so it should be good now.
I don't know about revlog-v1 (it seems there are tests in the tree that are revlog-v1-specific and don't gate against it), but (the lack of) zstd compression is also changing the result, so clearly needs to be guarded against, since it's breaking the --pure build.
I pushed that change. If you have ideas how to make the test less fragile, those would be appreciated. One possibility is to remove the test of corrupted repo altogether, if you think that's better.
While working on comments I discovered that the rust implementation was different from the C/Python implementation in how it deals with a wrong value of delta chain base.
I made sure that rust code behaves the same as hg there (which is: ignore the corruption) and added a test that checks that.
Dec 8 2021
The last update was to fix the issue with --pure: the output of debugdeltachain was different there.
Dec 7 2021
Dec 6 2021
I tried this, and ran into another problem: dirstate is linked to wlock, so if you share a wlock then you must also share the dirstate.
I wrote a patch that does this, but I can't tell how safe that is. All tests pass, at least.
Dec 3 2021
Nov 30 2021
Wow, looks I just didn't run all tests here. Sorry!
Investigating this, I went pretty deep into a rabbit hole and may need rescuing.
The bug seems to go like this:
I'm sorry. I forgot to backport a patch for shell compatibility. :-(
Nov 29 2021
Fixed the wording along with some other issues discovered in tests and review (we're using this patch in our internal fork).
Done. The refactor part was extracted into https://phab.mercurial-scm.org/D11818.
@Alphare, Ok. I understood you before, but those commits were not suitable for independent submission.
@Alphare, folded commit seems better than the separate ones in this case, though, since the second commit is simply incorporating review comments, not really an independent change.
I ran hg fold --from .^ followed by hg phabsend . to make this less confusing.
Trying to send this, I get:
$ hg update 356d27775359 $ hg phabsend .^ . D11766 - updated - 48486:e53ef79cd9d5 "sparse: demonstrate a bug when used with safe-share" abort: Conduit Error (ERR-CONDUIT-CORE): Graph cycle detected (type=5, cycle=PHID-DREV-z7jbc5yf2jun6v2pbnqj, PHID-DREV-z7jbc5yf2jun6v2pbnqj). EXIT STATUS 255
Nov 26 2021
I rebased to the stable branch. I did hg phabsend --fold again to avoid changing things. Let me know if you'd like me to re-do the phabsend.
Sorry, that test was weird indeed. I pushed a better one.
Nov 24 2021
I'm not sure I understand what your use case is. The config is already fully read by the point we read it, so how does that help you?
Indeed. You can see the diff history on the "History" tab, but I'm guessing it only coincides with the commits because I pushed them one by one.
The patch does consist of two commits. I was under assumption that this is the way to review chains of commits. Should I in the future create a separate phabricator revision for each commit?
Nov 18 2021
Nov 16 2021
I'm abandoning this in favor of: https://phab.mercurial-scm.org/D11764
That's less conservative (it does the checks at the level of individual commands, not at the level of hg-core), but it's simpler and more useful.
Nov 11 2021
The motivation here is that I kept running into this error when testing things with rhg, and I thought it didn't need to be an error.
I think there is no (significant) downside, since checking that fallback-executable is set by no means guarantees that the fallback will work, so not much is lost by delaying that check.