localrepo: stop doing special dirstate backup at transaction open Since the dirstate writes are already managed by the transaction, we already do a backup of the dirstate when necessary (and even trigger one to keep `hg rollback` happy). We needs some special code to deal with the initial empty checkout, but it is not too complicated. Managing variable filename (as dirstate-v2 uses) at the "journalfile" level, is complex and fragile (which is consistent with the fact these files are not journal…). If we no longer do it, our life is significantly simpler. In some sense, we apply the xkcd-1134¹ solution to our savebackup/restorebackup problem. [1] https://xkcd.com/1134/ (the change to test-hardlink are expect as decreasing the number of duplicated backup drive the hardlink count down)

localrepo: "blindly" do a dirstate backup at the end of the transaction Having the file backup mechanism dealing with file backup as benefit. So lets move closer to that. The fact `hg rollback` even needs this is sad. I hope to have the time to implement one of the alternative soon.

test-hardlink: stop explicitly listing `undo.dirstate` The files is on its way out.

localrepo: enforce a clean dirstate when the transaction open This is important to simplify the backup process. This also seems like a quite reasonable expectation. See inline documentation for details.

dirstate: explicitly backup the datafile Since the transaction does not know about this file, we need to explicitly mark it for backup before touching it.

mq: write the dirstate before stripping I am not certain why it is dirty (probably the `repo.setparent` call a right before. However I know we need it flushed before the repository starts to be stripped.

dirstate: simplify the shelve hack to not go through the disk We already have the data in memory, so why not simply keep the data in memory? This avoid abusing the `savebackup/restorebackup` logic and will make our life easier.

test: fix the flakyness in test-remotefilelog-local.t I now get about 80% of my `test-chg` CI run that fails on flakyness in this tests. It turns out this is only ambiguous status that end up doing file download. So… calling status early will do that potential download separately and the calls we scrutinize during that test will be just fine.

dirstate: detect potential fishy transaction patterns while changing If we rely on the transaction more, we should be more careful about the transaction. Adding extra security seems reasonable.

dirstate: generalize the dirstate's invalidation on transaction abort The previous code was too specific, we should add the invalidation at next to the `addfilegenerator` in the `write` call.

dirstate: simplify some methods' decorator Since `changing_parents` and `changing_files` are mutually exclusive, having both `@requires_changing_files` and `@requires_not_changing_parents` on a method is redundant.

dirstate: document the functions that need consolidation They are more functions that make the dirstate dirty but does not currently enforce a clean change scoping, we add comments in front of each of them so clarify this. There is no urgency to it, but the world will be a better place when all of them have proper scoping in place.

dirstate: make `restorebackup` more robust when it is a noop If there are no data_file, we might not be able to determine its filename. Which can be safely ignored. (all this code is on the way out anyway)

dirstate-guard: remove the feature The dirstate guard duplicated some of the logic already implemented in the transaction (and now the changing_* context). However the feature was incomplete, for example, living only in memory meant we could not recover from the hardest crash. In addition this duplicated with the transaction logic meant things could go out of sync or step on each other. Removing the feature now that we no longer needs it seems the safest.

rollback: remove the dirstateguard usage Thanks to the previous changeset, we no longer needs it. begone !

rollback: explicitly skip dirstate rollback when applicable instead of letting the transaction logic overwrite the dirstate and then overwrite that overwrite with a backup. We simply do not restore the dirstate related file during the _playback call. This open the way to removing the last dirstate guard usage.

rollback: detect "parentgone" case earlier Detecting this earlier will help us to affect the rollback process sooner, which will help the next changesets. To keep things simple, we degrade the behavior a bit when the `undo.desc` is missing. However since `hg rollback` is not supposed to be used and the `undo.desc` file have been introduced in mercurial 1.6 (2010). I think this is an acceptable evil.

rollback: avoid a `hg commit --addremove` at a critical point The rollback behavior around `hg commit --addremove` has changed slightly. It does not really matters here but keeping that variant out of the way cannot hurt.

rollback: display some graphlog before/after a test piece This make the situation clearer.

rollback: show that the safety works in a associated test That test section is checking that --force is overcoming the safety check, however we did not check that the safety properly detect the situation. We now do for clarity.

dirstate-guard: remove its usage in `backout` We can simply replace it with a transaction.

dirstate-guard: remove the usage in `import` It is now redundant with the transaction spawning the same scope.

dirstate-guard: replace a usage in `rebase` with a transaction Opening the transaction sooner will provide us with the same benefit.

dirstate-guard: remove usage in `rebase` Now that the dirstate change and write are clearer, it does not seems we need to use a dirstate-guard here anymore. The transaction already wrap the full operation.

dirstate-guard: remove it usage in `mq` The code it covered is also covered by a `changing_parents` context.

dirstate: enforce the use of `changing_files` context to change tracking Now that everything is using the new context, we can start enforcing its usage.

dirstate: warn if we write to the dirstate without holding the wlock Writing the dirstate without holding the wlock can race with other update and overwrite important change. The comment questionning the current state of things was right, we should the `wlock` should always cover the dirstate. (Yes, this is me, agreeing with myself, half a decade later).

dirstate: avoid transaction backup/restore if we do not hold the lock Doing overwriting the dirstate content without holding the lock is a recipe for disaster.

dirstate: issue a developer warning on implicit write on wlock release Our goal is to get rid of all these to clarify the writing pattern, so it is time to warn about this (and later, forbid it).

status: fix post status invalidation If the dirstate changed under us, we should throw away what we have a reload it, should we not ?

status: fix post status writing With dirstate-v2, the status process itself might update internal states. So we make sure this get written on disk

dirstate: use `dirstate.change_files` to scope the change in `shelve` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `unshelve` This is the way.

shelve: adjust what happens in some `changing_parents` context It seems like the rest is more about changing tracked_files, so we let start with moving them out of the `changing_parents` contexts.

dirstate: use `dirstate.change_files` to scope the change in `lfconvert` This is the way.

largefiles: rely on main scoping for writing dirstate in `markcommitted` Yeah, cleaner code.

largefiles: rely on main scoping for writing dirstate in `mergeupdate` Yeah, cleaner code.

largefiles: rely on the higher level `changing_giles` in `mergerecordupdates` Now that context open on the main dirstate also affect the underlying one, we can skip opening our own in `mergerecordupdates`

dirstate: use wlock and `dirstate.change_files` to scope the change in `mq` This is the way.

subrepo: use `changing_files` context in subrepository code This is better, not ideal, but better.

subrepo: let black expand some call on multiple lines early newer version of black are righfully doing this, so I apply it before more semantic change to reduce noise in the next changeset.

dirstate: use `dirstate.change_files` to scope the change in `import` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `automv` This is... mostly... the way.

dirstate: use `dirstate.change_files` to scope the change in `amend` This is the way.

dirstate: use the `changing_files` context in the `keyword` demo This is the way.

dirstate: wrap repository change in appropriate context in `test-context` We need the `wlock` (to add files), the `lock` (to commit), a `transaction` (to commit) and a `changing_files` context (to add files). Strictly speaking, we could let the commit take the `lock` and create a `transaction`, but it seems more consistent that way.

dirstate: use wlock and changing_files context in `test-revlog-ancestry` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `revert` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `gpg` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `tag` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `rename` This is the way, unless we are not actually touching the working copy. In such cases we don't need to do something.

dirstate: use `dirstate.change_files` to scope the change in `copy` This is the way, unless we are not actually touching the working copy. In such cases we don't need to do something.

dirstate: use `dirstate.change_files` to scope the change in `remove` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `forget` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `addremove` This is the way.

dirstate: use `dirstate.change_files` to scope the change in `add` This is the way.

commit: use `dirstate.change_files` to scope the associated `addremove` This was significantly more complicated than I expected, because multiple extensions get in the way. I introduced a context that lazily open the transaction and associated context to work around these complication. See the inline documentation for details. Introducing the wrapping transaction remove the need for dirstate-guard (one of the ultimate goal of all this), and slightly affect the result of a `hg rollback` after a `hg commit --addremove`. That last part is deemed fine. It aligns the behavior with what happens after a failed `hg commit --addremove` and nobody should be using `hg rollback` anyway. The small output change in the test come from the different transaction timing and fact the transaction now backup the dirstate before the addremove, which might mean "no file to backup" when the repository starts from an empty state.

commit: move the addremove logic around to make the next changeset clearer Lets do the noise now, without changing any thing. So the new changeset can focus on the actual semantic changes.

largefiles: link the core dirstate._changing context to the lfdirstate one This will be much cleaner and safer to make sure the two dirstates are in sync. This way, the large-files dirstate will simply inherit the state of the main dirstate, so if the core code does the right thing, the large-files code should be right too.

dirstate: add a context for tracking files change Let us start to use it. We will enforce it later.

dirstate: invalidate the dirstate change on transaction failure If the change context lives inside a transaction, the change are not flushed to disk on exit as this is delegated to the transaction. As a result we should also delegate the part that do cleanup on failure. The issue was caught by tests with other change, but it seems useful to fix this as soon as possible.

dirstate: factor the "changing" context logic out This makes it reusable for other types of changes.

dirstate: introduce a `is_changing_any` property This will embrace other cases than changing parents.

dirstate: rename `pendingparentchange` to `is_changing_parents` This is clearer and more inline witht he other change we did.

dirstate: rename _parentwriters to _changing_level We will have context for changing more than parents, so lets be prepared.

largefiles: remove the `changing_parents` context in `openlfdirstate` This code might run in any kind of change context, we should rely on the higher level context instead of opening our own.

largefiles: remove the second `changing_parents` in `updatelfiles` Now that the `update_file` call have been migrated, we can drop the semantically-wrong `changing_parents` context.

largefiles: remove the first `changing_parents` in `updatelfiles` Now that the `update_file` call have been migrated, we can drop the semantically-wrong `changing_parents` context.

largefiles: use `hacky_extension_update_file` in `updatelfiles` This is what the function is meant for.

largefiles: use `hacky_extension_update_file` in `synclfdirstate` This is what the function is meant for.

largefiles: use `hacky_extension_update_file` in `openlfdirstate` This is what the function is meant for.

win32text: make the hacky call cover more cases Now that I understand what this is doing, It seems like the case for `p1_tracked` were covered by neither the code nor the test. Now the code cover it at least.

win32text: drop the `changing_parents` context in revert upgrade We are not changing parents here, so let us not pretend we do.

win32text: clean up and clarify the post-revert hack of dirstate The change is unrelated to changing parents and should not be a in a "changing_parents" context. This call is quite hacky, but now it is at least explicitly hacky. We will drop the `changing_parents` context in the next changesets, but we kept this change simple to help readability.

dirstate: introduce a `hacky_extension_update_file` method See inline documentation for details.

mq: properly take the wlock during the full qfold operation Otherwise the operation could be raced… for unknown result.

locking: hold the wlock for the full duration of the "keyword demo" The risk of racing the demo is low, since it seems to create its own repository on the fly. However it is clearer and more consistent.

locking: grab the wlock before touching the dirstate in `perfdirstatewrite` If we touch the dirstate, we should hold the `wlock`.

locking: take the `wlock` for the full `hg addremove` duration Otherwise, there is a race condition window between the time we resolve the file to addremove with the matcher and the time we lock the repo and modify the dirstate. For example, the working copy might have been updated away, or purged, and the matched files would no longer be correct.

locking: take the `wlock` for the full `hg forget` duration Otherwise, there is a race condition window between the time we resolve the file to forget with the matcher and the time we lock the repo and modify the dirstate. For example, the working copy might have been updated away, or purged, and the matched files would no longer be correct.

locking: take the `wlock` for the full `hg remove` duration Otherwise, there is a race condition window between the time we resolve the file to remove with the matcher and the time we lock the repo and modify the dirstate. For example, the working copy might have been updated away, or purged, and the matched files would no longer be correct.

locking: take the `wlock` for the full `hg add` duration Otherwise, there is a race condition window between the time we resolve the file to add with the matcher and the time we lock the repo and modify the dirstate. For example, the working copy might have been updated away, or purged, and the matched files would no longer be correct.

dirstate: drop some very fishy looking piece of code This piece of code is marking the **real** dirstate file as a temporary transaction file. This means it get deleted on transaction rollback. This this quite wrong, especially as the comment points out some `dirstate.pending` motivation and the `.pending` file should already be fully managed by the transaction. The only ready I can think of this behavior not having awful results right now is because other transaction logic restore backed up content above the one that got wrongfully deleted. Let us stop doing this anyway, All tests seems happy.

dirstate: do not write an empty dirstate just for backup This will get in the way when we get more strict about holding the lock when writing the dirstate. Instead, we simply don't copy dirstate files around if there are None at backup time. A couple of tests are impacted they no longer need to backup such "empty" dirstate.

dirstate: pre-indent some of the backup code This will make the next changeset clearer.

debugrebuilddirstate: double check that no transaction is open Since transaction impact dirstate write, we make sure nobody is trying anything strange with this internal command.

dirstate: explicitly write the dirstate after `debugrebuilddirstate` I am working on making the dirstate write patterns more predictable. This patch is part of a small series of similar patches that adds a explicit dirstate write in a handful of location where the dirstate is updated "a bit in a strange way". With this explicit write, we are no longer relying on implicite write of the dirstate on `wlock` release. This make the world a better place.

dirstate: explicitly write the dirstate after `keyword` "overwrite" I am working on making the dirstate write patterns more predictable. This patch is part of a small series of similar patches that adds a explicit dirstate write in a handful of location where the dirstate is updated "a bit in a strange way". With this explicit write, we are no longer relying on implicite write of the dirstate on `wlock` release. This make the world a better place.

dirstate: explicitly write the dirstate after `eol` dirstate manipulation I am working on making the dirstate write patterns more predictable. This patch is part of a small series of similar patches that adds a explicit dirstate write in a handful of location where the dirstate is updated "a bit in a strange way". With this explicit write, we are no longer relying on implicite write of the dirstate on `wlock` release. This make the world a better place.

dirstate: explicitly write the dirstate after mq dirstate rebuild I am working on making the dirstate write patterns more predictable. This patch is part of a small series of similar patches that adds a explicit dirstate write in a handful of location where the dirstate is updated "a bit in a strange way". With this explicit write, we are no longer relying on implicite write of the dirstate on `wlock` release. This make the world a better place.

transaction: quietly rollback if no other changes than temporary files If no actual change have been made, we don't really need to roll them back. We only have to cleanup some temporary files and it seems reasonable to do that quietly. This will help us to use the transaction in wider context¹ without impacting the user experience. [1] as in Python context managers that lives longer.

transaction: run abort callback in all cases Previously, these possibly important callback were "forgotten" when running a quick rollback. This is now fixed, as the tests shown.

transaction: clarify the "quick abort" scenario Right now, the transaction has a code-pass to do a "quick abort" that skip most¹ (too much) of the logic when the right condition are detected² We are about to improve this logic in multiple aspect. We clarify the code first. The conditional return in `_can_quick_abort` looks a bit weird because we are about to make them more complex very soon. [1] actually too much [2] actually not often enough

test: use a more direct form of interruption in fncache "recover" testing The previous test was relying on implementation details and harder to maintain. The new version is closer to the initial intend : "What happens if the process die without cleanup". This change is motivated by further changes around the transaction and dirstate logic that would break the fragile equilibrium that existed before this patch. Making this change early make it easier to review on its own and remove noise in future larger changes.

test: use a more direct approach to test racy mutation The previous test was relying on implementation details and harder to maintain. The new version is closer to the initial intend : "What happens the file get overwritten from under the current process" This change is motivated by further changes around the transaction and dirstate logic that would break the fragile equilibrium that existed before this patch. Making this change early make it easier to review on its own and remove noise in future larger changes.

test: create some history in test-dirstate-backup An empty repository, based on `null` is quite a corner cases. We create a more "natural" setup for this tests to make sure it can keep testing what it intend to test in the future.