There are a few use cases where pruning the original branch while using --hold has been desirable, though admittedly these are likely very specific to our internal processes.

I generally think this is too niche to upstream -- it quite possibly affects only your install. Although it sounds like it wasn't the behavior under Mercurial previously, the intent of --hold is to serve as a dry run and abort the workflow midway through, e.g. it is documented as:

With --hold, execution stops here, before the change is pushed.

...that is, execution stops before:

... Once everything has been pushed, cleanup occurs. ...

I'm comfortable with having --hold print out the prune commands, but I don't want to add more prompts and flags upstream to address use cases at this level of niche behavior. On the particular use cases:

When making fixes in older versions of a product, we require all changesets exist in the repos for newer versions before allowing it to be pushed to the older version.

This definitely feels extremely niche -- I can't recall seeing another organization that has different repos for different production versions, and requires that one repository be a strict subset of the other repository (approximately?). I'm sure there's some reasoning for this but it seems like "one repository, with more steps" at first glance.

There are some products which require re-computing new generated code based on the new state of the codebase.

This is a worthy and reasonable use case, but I imagined addressing it by giving arc the ability to run "codegen / post-merge-rebase" steps. It would then be able to handle code generation after rebases, and could get this right for cascading rebases, arc patch, etc. Phabricator has this issue too, although it never rose past the level of vague annoyance.

Some colleagues really want to inspect everything before pushing things upstream. Also here the conflict is in having to clean up the old branches.

I'd expect they can run arc land --hold as a "dry-run", then repeat the command without --hold to get to the desired state. This should (I think?) be less work than copy/pasting or retyping a push command, although maybe it takes slightly longer (a few seconds?).

Generally, pruning after --hold means that if they run arc land --hold, get up to grab a coffee, come back, push, and the remote has moved ahead so the push is rejected, it's nontrivial to fix things. You can't just run the same command again since none of your local refs exist. Maybe they're comfortable getting out of this state, but I think this is a tricky state for newer users in general.

I'd conceptually be comfortable with marking these refs as "maybe-landed" and letting some other process (like hypothetical arc cleanup in T3277) remove them, but I'm not comfortable with bringing a workflow upstream that can leave the repository in a state that (a) hasn't been published and (b) can not be published by re-running arc land (since refs were deleted). A great deal of the complexity in arc is trying to avoid states where users can't move forward by running arc land again after something unexpected happens.

This actual code is reasonable if you're comfortable with arc land putting the repository into a state that arc land can not get the repository out of. The change to ArcanistPrompt may not cover all prompt use cases (e.g., maybe setReturnsResponse() rather than setAbortsWorkflow() covers more ground in the long term?), but it's generally reasonable and I imagined prompts eventually including decisions rather than just workflow aborts. So there's nothing bad here technically, I just want to upstream arc land to continue having great difficulty putting working copies in states with no simple way forward.

Would the expectation then be that running arc land again would remove the rebased/squashed commits created by --hold, so the result would be as if they hadn't run --hold previously?

I'm not 100% sure what actually happens in Mercurial today, but in Git: the rebased/squashed commits aren't reachable from any refs, so Git automatically deletes them after a couple weeks. If you run arc land --hold and then arc land (and whatever you're landing onto hasn't moved ahead between your two executions), you'll end up with an exactly equivalent set of commits (exact same changes to the underlying code), so if you inspect the arc land --hold changes, any observations you make should apply exactly to the arc land changes that actually publish.

The commits created as a side effect of arc land --hold aren't actually deleted explicitly, but they're effectively gone in Git as soon as you update your working copy to any other state. I'm not sure how closely this holds (or can hold) in Mercurial.

In either VCS, if master or default or whatever has moved forward while you were inspecting changes, the second arc land may not produce exactly the same change the first one did (it will be rebasing onto a different base) but the changes should generally be trivial. If you'd used arc land --hold + inspect + manual push, your manual push would have failed anyway.

The arc cleanup sounds like it might be a good approach, though my understanding right now is that arc is effectively stateless on the client machines and implementing something like cleanup would require storing local state and ensuring it is regularly synced with the server.

Right. It doesn't necessarily have to be synchronized to the server -- I think it would be fine to say "local branch Q at commit R in this working copy was hold-landed into commit S on 2020-03-13, so arc cleanup can consider it safe to delete now if Q still points at R and S is present in the remote". I don't think most users would expect arc land --hold in one working copy to clean up branches with the same name in a different working copy (even the old accidental arc land behavior wasn't this powerful!).

A local cache would be good enough to let arc cleanup almost always do arc patch cleanups safely, too (e.g., change X was patched to branch Y at commit Z on date D, so require --unsafe --force --really --aggressive flags if D was very recent or Y no longer points at Z or the user happens to create a branch named patch-1234, and/or arc patch could then begin using human-readable names).

There's also some general discussion of this kind of additional local-or-local-and-remote commit mapping in T3875.

Also - I added some comments in here to ArcanistLandEngine which I think would be generally useful. Are you okay if I move those over into D21680 before landing that?

Yep, those look good to me.

Just thinking aloud to myself

Off-hand I don't know of a solution that could provide that same guarantee for cherry-picking. Our current process also requires that the engineer who authored the fix is the one responsible for merging it, rather than having release engineers or an unlucky engineer responsible for performing all merges into the subsequent versions.

This could probably be handled by having the server-side hook check for commits with the same task number, as we require all commits to start with a task number. It's not as "comfortable" as ensuring the same changeset exists in the branch but since the changesets require merge commits which can potentially introduce incorrect merges that comfort might be just as misleading.

I'm still piecing together Git's handling of branches in my mind as I have used Mercurial for so long.

I think the big difference is that in Git, commits don't have to be on any branch. Git's branches are more or less like Mercurial's bookmarks, and there's no direct analog to Mercurial's branches in Git.

Git branches and Mercurial bookmarks are roughly named labels that point at some commit, and which will move forward automatically to point at descendant commits in some circumstances. (Tags in Git are roughly named labels that point at some commit but don't move automatically.)

When a commit is on one or more branches in Git, it just means that the commit is an ancestor of the commit(s) those branch labels point at.

In Git, we can just create new commits floating out in the middle of nowhere -- not on any branch, and not the ancestor of any labeled commit -- that you can retrieve if you know the hash but aren't otherwise reachable or visible to users without using very low-level commands. This isn't normally useful for human users, but it's great for reducing side effects in arc land, since we can do all our work in a sort of temporary scratch area that automatically gets cleaned up later.

In Git, we can also move labels in the remote at push time, i.e. push to a particular label, independent of how a commit is labelled locally. So we can build a floating-in-the-middle of nowhere commit, then push it to "master" in the remote, saying "transfer the actual changes, then update the master label to point at this commit". On its own, this doesn't cause any local side effects (no new labels are created or moved). So we have a lot of control over exactly which local and remote labels get updated, and can do the entire land process with no side effects and no user-visible modifications to repository state until we actually commit to publishing changes. Once we're ready to publish into the remote, we can precisely control all the local and remote label updates.

I think this is generally good, since it means arc land is less surprising in error cases: it doesn't modify any working copy state or labels (unless you look deep, deep under the hood) and if something goes wrong you can almost always just run arc land again to converge to the desired state -- or at least reproduce the problem exactly, so it can be reported upstream and hopefully debugged.

This is trickier in Mercurial since it's harder or impossible to perform some of the operations narrowly in a side-effect-free way. We could perhaps imagine doing the work up to the point of --hold in some branch named __temporary_workspace_ignore_me and then moving commits to the real branch before publishing them, but this would probably be pretty messy in practice and if --hold is to allow the user to directly hg push, we can't leave them in a state where they have to move commits between branches.

Maybe a cleaner approach is storing local metadata ("commit C on label L is an arc land --hold commit from landing Q") and having arc land Q --onto L check if L currently points at some commit known to be a leftover of a previous land operation. Then it could just strip it safely rather than erroring out ("local L has unpublished commits"), and arc cleanup or similar could also strip these, and arc branches and such could show these labels (and arc patch branches/bookmarks, and landed-but-not-deleted local feature labels) as tainted/unclean/ripe for cleanup.

In Git, we can just create new commits floating out in the middle of nowhere -- not on any branch, and not the ancestor of any labeled commit -- that you can retrieve if you know the hash but aren't otherwise reachable or visible to users without using very low-level commands. This isn't normally useful for human users, but it's great for reducing side effects in arc land, since we can do all our work in a sort of temporary scratch area that automatically gets cleaned up later.

Woah, yea I don't think there's anything like this in Mercurial. I take it these disparate trees have some limitations? Do they always apply to the working directory or something?

Mercurial has "secret" phase though that doesn't hide it from the user at all. Maybe the closest thing would be obsolete markers from the evolve extension, though commits with obsolete markers aren't cleaned up automatically and I don't think you can operate on those changesets (I just tried rebasing an obsolete commit onto another obsolete commit and got some error about "no successors").

In Git, we can also move labels in the remote at push time, i.e. push to a particular label, independent of how a commit is labelled locally. So we can build a floating-in-the-middle of nowhere commit, then push it to "master" in the remote, saying "transfer the actual changes, then update the master label to point at this commit". On its own, this doesn't cause any local side effects (no new labels are created or moved). So we have a lot of control over exactly which local and remote labels get updated, and can do the entire land process with no side effects and no user-visible modifications to repository state until we actually commit to publishing changes. Once we're ready to publish into the remote, we can precisely control all the local and remote label updates.

I think this is generally good, since it means arc land is less surprising in error cases: it doesn't modify any working copy state or labels (unless you look deep, deep under the hood) and if something goes wrong you can almost always just run arc land again to converge to the desired state -- or at least reproduce the problem exactly, so it can be reported upstream and hopefully debugged.

Yea this does sound really useful here. Does this mean that when using arc land without --hold it will push from the phantom branch as well? From a quick test it looks like the --hold instructions do this, which I think are the same ones used during land. So then a full arc land would then pull the resulting changes from remote and strip out the local source branch.

This is trickier in Mercurial since it's harder or impossible to perform some of the operations narrowly in a side-effect-free way. We could perhaps imagine doing the work up to the point of --hold in some branch named __temporary_workspace_ignore_me and then moving commits to the real branch before publishing them, but this would probably be pretty messy in practice and if --hold is to allow the user to directly hg push, we can't leave them in a state where they have to move commits between branches.

When using --hold with Mercurial it seems to do roughly the same thing as Git however, A) the phantom branch is more corporeal, and B) the master bookmark is advanced to the new tip (both before and after the modernization update). I think (B) might be where the difference in expectation comes from, as I've (we've) been used to having --hold already operate on the local master branch where in Git that doesn't happen. Additionally I'm also used to having the evolve extension installed where I'm much more quick to prune branches/commits to get them out of my way but can always resurrect them if they're needed.

Maybe a cleaner approach is storing local metadata ("commit C on label L is an arc land --hold commit from landing Q") and having arc land Q --onto L check if L currently points at some commit known to be a leftover of a previous land operation. Then it could just strip it safely rather than erroring out ("local L has unpublished commits"), and arc cleanup or similar could also strip these, and arc branches and such could show these labels (and arc patch branches/bookmarks, and landed-but-not-deleted local feature labels) as tainted/unclean/ripe for cleanup.

Yea that does sound great; having local metadata does seem like a good next big step for Arcanist. Slightly related, I know there's also some desire for arc patch be more aware about existing patched changes (with regards to dependent revisions I think) and it's bookmarks, which I think will also need this type of metadata to operate effectively.

It's taken me a few times reading through but I think I'm getting a better grasp on the arc land design and process. Thanks for taking the time to write up these responses.

I take it these disparate trees have some limitations? Do they always apply to the working directory or something?

There aren't really any limitations that I can think of, Git just exposes lower-level operations than Mercurial does with "plumbing" commands that regular users almost never interact with.

In Git, you can create any low-level object (blob, tree, commit) without interacting with the working copy at all by using git hash-object. All Git objects have an internal string representation, and a third-party program can build that string itself (or use other plumbing commands which help build the string) and then write it into the repository with git hash-object without interacting with the current working copy state at all. git hash-object prints out the hash it wrote, so the caller can continue doing useful work from there.

Arcanist does technically use hash-object in one case (when you arc land into an empty repository, see here), but in other cases it doesn't need to go quite this low level. Various higher-level-but-still-not-normally-user-facing commands and workflows also exist, like git commit-tree (create a commit from a repository state other than the repository state currently in the working copy).

More simply: if you have some normal directory full of files and other directories, you can turn it into a Git state by using git hash-object to build Git "blobs" from each file, then Git "trees" from all the blobs you built, then a commit from the tree, and produce a normal Git commit which exactly represents the original directory state, all without modifying the working copy (or even needing to have those files on disk at all -- the content could be generated synthetically by a program instead). The high-level Git commands mostly do exactly this, by calling the low-level Git commands, at least conceptually if not in practice.

If you do this, all these files and trees and commits and stuff are just "rows in Git's database", and you're just using git insert-a-row-into-the-database, effectively. As long as the data you provide is in approximately the right format for the type of object you're inserting (see T11537 for an unusual case), Git doesn't care if the filesystem objects that data represents never actually existed anywhere on disk.

Does this mean that when using arc land without --hold it will push from the phantom branch as well?

Yes. In Git, we (almost) don't need to modify (user-visible) local state until the remote server tells us that the correct landed state has been successfully published. If something goes wrong before then, we can nearly exit directly into the original state.

(This isn't entirely true because we do change the working copy state to do some merge operations, and there are some details with dirty working copies (where users may have local uncommitted changes or untracked files) that we can most easily deal with by saving/restoring state. So we do still save/restore state, but not very much, and generally don't have to "rewind" any work we've done since all our changes are ephemeral until they hit the remote.)

This is also shedding light on T13107 :)