Patchwork D7631: RFC: absorb: allowing committed changes to be absorbed into their ancestors

login
register
mail settings
Submitter phabricator
Date Dec. 13, 2019, 5:56 a.m.
Message ID <differential-rev-PHID-DREV-qbcvl27i2il44an6en4b-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/43776/
State New
Headers show

Comments

phabricator - Dec. 13, 2019, 5:56 a.m.
rdamazio created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This can also be combined with --interactive to absorb just part of a
  commit into its parents.
  
  This initial implementation has the shortcoming that it does not do anything
  with the absorbed commit:
  
  - With obsmarkers, this means the user still needs to evolve/rebase manually
  - Without obsmarkers, the old commits would not be stripped at all because their child was not replaced

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

AFFECTED FILES
  hgext/absorb.py
  tests/test-absorb-rev.t
  tests/test-absorb.t

CHANGE DETAILS




To: rdamazio, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 13, 2019, 6:21 p.m.
quark added a comment.


  `--rev` seems ambiguous since there might be different kinds of revisions to specify - target and revisions to edit. Maybe something like `--source`, `--from`, `--target`?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: quark, mercurial-devel
phabricator - Dec. 14, 2019, 6:02 a.m.
rdamazio added a comment.


  In D7631#112414 <https://phab.mercurial-scm.org/D7631#112414>, @quark wrote:
  
  > `--rev` seems ambiguous since there might be different kinds of revisions to specify - target and revisions to edit. Maybe something like `--source`, `--from`, `--target`?
  
  Done. Used `--source` to match `rebase`.
  
  I'm assuming no fundamental objections then? Removing the "RFC" part so it gets a proper review then.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: quark, mercurial-devel
phabricator - Dec. 17, 2019, 12:29 p.m.
pulkit added a comment.


  Can you describe the feature a bit in the commit message. Specific things which I feel are missing:
  
  - what happens to wdir changes
  - what happens if source is a commit is not a head
  
  I understand them but after reading the tests, so might be worth to add them to description.
  
  Also, it will be nice if you add an entry in releasenotes.

INLINE COMMENTS

> absorb.py:993
>              raise error.Abort(_(b'cannot absorb into a merge'))
> +        if len(targetctx.parents()) > 1:
> +            raise error.Abort(_(b'cannot absorb a merge'))

need to do more checks here about being public commit etc. `rewriteutil.precheck` should help.

> test-absorb-rev.t:73
> +
> +  $ grep 6 a
> +  6

`hg status|diff` should be a better command here.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: pulkit, quark, mercurial-devel
phabricator - Dec. 18, 2019, 4:42 a.m.
rdamazio added a comment.


  > Can you describe the feature a bit in the commit message. Specific things which I feel are missing:
  
  Done
  
  > Also, it will be nice if you add an entry in releasenotes.
  
  Done

INLINE COMMENTS

> pulkit wrote in absorb.py:993
> need to do more checks here about being public commit etc. `rewriteutil.precheck` should help.

Done. Notice that *technically* the user could do such an absorb while in the middle of a merge, but it sounds like a bad idea and inviting troubles, so I'm letting it also disallow that case. I'll be surprised if anyone even notices.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: pulkit, quark, mercurial-devel
phabricator - Dec. 18, 2019, 7:55 a.m.
pulkit added inline comments.

INLINE COMMENTS

> absorb.py:1113
> +    absorb analyzes each change in your working directory (or the revision given
> +    to `--rev`, if one is specified) and attempts to amend the changed lines
> +    into the changesets in your stack that first introduced those lines.

s/--rev/--source

> next:5
> +   the working directory changes, which continues to be the default unless
> +   `--rev`/`-r` is specified.
>  

s/--rev/--source

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: pulkit, quark, mercurial-devel
phabricator - Jan. 2, 2020, 8:21 p.m.
mharbison72 added a comment.
mharbison72 added subscribers: martinvonz, mharbison72.


  In D7631#112604 <https://phab.mercurial-scm.org/D7631#112604>, @rdamazio wrote:
  
  > In D7631#112414 <https://phab.mercurial-scm.org/D7631#112414>, @quark wrote:
  >
  >> `--rev` seems ambiguous since there might be different kinds of revisions to specify - target and revisions to edit. Maybe something like `--source`, `--from`, `--target`?
  >
  > Done. Used `--source` to match `rebase`.
  
  Is `--exact` from `hg fold` a better model?  I don't feel strongly; I only mention it because `hg rebase -s` will take that revision and its descendants, so it's more like "stack" in my mind.  I'm not sure how many other commands have `-s` off the top of my head, but @martinvonz 
  mentioned adding that to `hg fix` (probably in IRC), and I think mentioned the word "stack" in that context.  So I might not be the only one to get slightly tripped up by that.
  
  > I'm assuming no fundamental objections then? Removing the "RFC" part so it gets a proper review then.
  
  I like it.

INLINE COMMENTS

> absorb.py:1141
> +        # default to working copy
> +        ctx = scmutil.revsingle(repo, source, default=None)
> +

Should it abort if multiple revisions are given, instead of picking the latest?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - Jan. 7, 2020, 12:46 a.m.
rdamazio added a comment.


  In D7631#114393 <https://phab.mercurial-scm.org/D7631#114393>, @mharbison72 wrote:
  
  > In D7631#112604 <https://phab.mercurial-scm.org/D7631#112604>, @rdamazio wrote:
  >
  >> In D7631#112414 <https://phab.mercurial-scm.org/D7631#112414>, @quark wrote:
  >>
  >>> `--rev` seems ambiguous since there might be different kinds of revisions to specify - target and revisions to edit. Maybe something like `--source`, `--from`, `--target`?
  >>
  >> Done. Used `--source` to match `rebase`.
  >
  > Is `--exact` from `hg fold` a better model?  I don't feel strongly; I only mention it because `hg rebase -s` will take that revision and its descendants, so it's more like "stack" in my mind.  I'm not sure how many other commands have `-s` off the top of my head, but @martinvonz 
  > mentioned adding that to `hg fix` (probably in IRC), and I think mentioned the word "stack" in that context.  So I might not be the only one to get slightly tripped up by that.
  
  IMHO no, needing `--exact` is actually confusing to almost every user we've talked to, and they'd instead expect that to be the default behavior, with "fold up to this commit" being the one that needs a specific flag.
  
  >> I'm assuming no fundamental objections then? Removing the "RFC" part so it gets a proper review then.
  >
  > I like it.
  
  Thanks

INLINE COMMENTS

> mharbison72 wrote in absorb.py:1141
> Should it abort if multiple revisions are given, instead of picking the latest?

I suspect other places may want something similar (e.g. it'd make sense in `rebase --dest`, so I changed revsingle to add the behavior.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - Jan. 13, 2020, 4:05 p.m.
pulkit added inline comments.

INLINE COMMENTS

> rdamazio wrote in absorb.py:993
> Done. Notice that *technically* the user could do such an absorb while in the middle of a merge, but it sounds like a bad idea and inviting troubles, so I'm letting it also disallow that case. I'll be surprised if anyone even notices.

Sorry, I misunderstood the patch earlier. `rewriteutil.precheck` on target rev is not very helpful as we are not obsolete-ing that in this rev, but we are re-writing it's ancestors. So, if target-rev is a head, and `evolution.alloworphans=False` is set, it will still create orphans.

Not sure what's the best way forward, maybe we should do `rewriteutil.precheck` for the parent instead until we start obsoleting this rev.

> test-absorb-rev.t:72
>  Run absorb:
>  
> +  $ hg absorb --apply-changes -s .

I locally added some `hg log --graph` calls before and after absorb call to understand what happens. It will be nice to add them as it will make things easier for others to understand.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - Jan. 13, 2020, 5:31 p.m.
martinvonz added a comment.


  In D7631#112604 <https://phab.mercurial-scm.org/D7631#112604>, @rdamazio wrote:
  
  > In D7631#112414 <https://phab.mercurial-scm.org/D7631#112414>, @quark wrote:
  >
  >> `--rev` seems ambiguous since there might be different kinds of revisions to specify - target and revisions to edit. Maybe something like `--source`, `--from`, `--target`?
  >
  > Done. Used `--source` to match `rebase`.
  
  Sorry I didn't notice until now, but `--source` makes me think it will behave like `hg rebase --source` and absorb from the given commit and all its descendants. I would have preferred `--from` (and maybe a `--into` for choosing which commits to absorb into in the future).

INLINE COMMENTS

> pulkit wrote in absorb.py:993
> Sorry, I misunderstood the patch earlier. `rewriteutil.precheck` on target rev is not very helpful as we are not obsolete-ing that in this rev, but we are re-writing it's ancestors. So, if target-rev is a head, and `evolution.alloworphans=False` is set, it will still create orphans.
> 
> Not sure what's the best way forward, maybe we should do `rewriteutil.precheck` for the parent instead until we start obsoleting this rev.

Maybe I'm also misunderstanding what this patch does in that case. `hg absorb -r A` will not obsolete A? I would think it definitely should do that. Perhaps the successors or the absorbed commit should be all the nodes absorbed into as well as any potential leftovers (which were not absorbed).

> test-absorb-rev.t:63
> +
> +  $ hg absorb --apply-changes -s .^+.
> +  abort: revision set matched multiple revisions

nit: I think I've heard that `^` needs to be quoted on Windows, so maybe `-s '.^+.'`

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - Jan. 16, 2020, 4:49 p.m.
mharbison72 added inline comments.

INLINE COMMENTS

> martinvonz wrote in test-absorb-rev.t:63
> nit: I think I've heard that `^` needs to be quoted on Windows, so maybe `-s '.^+.'`

That's true in cmd.exe, but msys doesn't seem to care.  (That said, I thought check-code enforced quoting around `^`.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - Feb. 1, 2020, 7:10 a.m.
rdamazio added a comment.


  Sorry for the delay in replying here.

INLINE COMMENTS

> martinvonz wrote in absorb.py:993
> Maybe I'm also misunderstanding what this patch does in that case. `hg absorb -r A` will not obsolete A? I would think it definitely should do that. Perhaps the successors or the absorbed commit should be all the nodes absorbed into as well as any potential leftovers (which were not absorbed).

See the child commit (D7630 <https://phab.mercurial-scm.org/D7630>), which adds the "evolve" operation.

Because of the invariant about parent phases, checking that the revision being absorbed is not public also ensures that everything it's absorbing into is not public. Is that what you were looking for? If the commit A being absorbed is a draft and its parent is public, then absorb just won't find anywhere to absorb the lines and will leave everything in A.

About setting obsmarkers from the absorbed commit into the targets, while that's technically correct, I suspect it'll become a hard-to-navigate mess which adds very little. Do you want me to add that?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers
Cc: mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - Feb. 11, 2020, 6:25 p.m.
This revision now requires changes to proceed.
martinvonz added a comment.
martinvonz requested changes to this revision.


  Sorry about the delay in responding. Please remember to rebase the series to the latest @ on hg-committed (the release notes are otherwise likely to conflict).

INLINE COMMENTS

> rdamazio wrote in absorb.py:993
> See the child commit (D7630 <https://phab.mercurial-scm.org/D7630>), which adds the "evolve" operation.
> 
> Because of the invariant about parent phases, checking that the revision being absorbed is not public also ensures that everything it's absorbing into is not public. Is that what you were looking for? If the commit A being absorbed is a draft and its parent is public, then absorb just won't find anywhere to absorb the lines and will leave everything in A.
> 
> About setting obsmarkers from the absorbed commit into the targets, while that's technically correct, I suspect it'll become a hard-to-navigate mess which adds very little. Do you want me to add that?

I think we should at least have a TODO about adding them.

By the way, without the next patch's auto-evolve feature, I'm not sure we should add such markers. I think they would trick `hg evolve` into moving any descendant commits onto the topmost commit that was absorbed into, but that's probably not what the user wants (they probably want child commits to be moved onto the absorbed commit's parent).

> 5.3:4-7
> + * The `absorb` extension can now absorb existing changesets, in addition to
> +   the working directory changes, which continues to be the default unless
> +   `--source`/`-s` is specified.
>  

please revert (sorry about this annoying effect of copy detection)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers, martinvonz
Cc: mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - Feb. 13, 2020, 11:31 p.m.
marmoute added a comment.
marmoute requested changes to this revision.


  As explained in this comment https://phab.mercurial-scm.org/D8030#120771 I find the idea of usign a changeset in place of the working copy interresting. However, this is a larger big UX change that seems to deserve a wider discussion with more of the community involved. In particular I am not convinced about the `--rev` flag usage.
  
  See the linked comment for more details.
  
  (Sorry for the delay this will involves, I wish the idea had explicitly surfaced earlier).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers, martinvonz, marmoute
Cc: marmoute, mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - March 10, 2020, 5:41 a.m.
martinvonz added a comment.


  In D7631#120777 <https://phab.mercurial-scm.org/D7631#120777>, @marmoute wrote:
  
  > As explained in this comment https://phab.mercurial-scm.org/D8030#120771 I find the idea of usign a changeset in place of the working copy interresting. However, this is a larger big UX change that seems to deserve a wider discussion with more of the community involved. In particular I am not convinced about the `--rev` flag usage.
  > See the linked comment for more details.
  > (Sorry for the delay this will involves, I wish the idea had explicitly surfaced earlier).
  
  The sprint is not happening and I don't think we should wait for the next sprint (fall?) to move forward with this. Here's a Plan Page I wrote down in response to @marmoute's request D8030 <https://phab.mercurial-scm.org/D8030>: on https://www.mercurial-scm.org/wiki/RevisionAsWDirPlan. The decision we reached was to use the name `--at-rev` on that patch. However, and if I understood correctly, the general sentiment was not that all similar arguments (like the on in this patch) should use that but rather that we should pick a name that sounded for each command. So in the case of this patch, I suppose `--from` is fine? We should mark it EXPERIMENTAL, though.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers, martinvonz, marmoute
Cc: marmoute, mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - March 11, 2020, 3:15 a.m.
mjacob added a comment.


  I like the name `--from` for the option. It would also make sense in combination with a possible future `--into` option.

INLINE COMMENTS

> martinvonz wrote in absorb.py:993
> I think we should at least have a TODO about adding them.
> 
> By the way, without the next patch's auto-evolve feature, I'm not sure we should add such markers. I think they would trick `hg evolve` into moving any descendant commits onto the topmost commit that was absorbed into, but that's probably not what the user wants (they probably want child commits to be moved onto the absorbed commit's parent).

My preferred option would be to add obsmarkers from the absorbed changeset to the changed changesets. This is what I would usually do if I do a poor man’s version of this functionality: I uncommit the "source" changeset, absorb, and prune the "source" changeset with the changed changesets as the successor.

It is true that `hg evolve` is likely to not behave in a useful manner in this case. But as someone who almost always specifies a successor when pruning, I already run into these problems regularly. Fixing that is probably out of scope for this discussion.

I tend to say that adding obsmarkers should be part of this patch, but I’m not very familiar with Mercurial’s preferred workflow for things like this.

> absorb.py:983
>  
> -def absorb(ui, repo, stack=None, targetctx=None, pats=None, opts=None):
> +def absorb(ui, repo, targetctx, stack=None, pats=None, opts=None):
>      """pick fixup chunks from targetctx, apply them to stack.

I know that it was named like this before, but the name `targetctx` is quite confusing here, especially considering that the user-facing option will be something like `--from` or `--source`.

Can it be changed, in a separate patch, after there is consensus about how the option should be called?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers, martinvonz, marmoute
Cc: mjacob, marmoute, mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - March 11, 2020, 4:04 a.m.
mharbison72 added a comment.


  In D7631#123318 <https://phab.mercurial-scm.org/D7631#123318>, @mjacob wrote:
  
  > I like the name `--from` for the option. It would also make sense in combination with a possible future `--into` option.
  
  The unfortunate thing about `--from` is that it implies a range in `hg fold`, whereas here it seems to be a single(?) commit.  It does make a lot of sense within the context of this command though (i.e. taking stuff //from// this commit).  And it took me awhile to get comfortable with the meaning in `hg fold`, so maybe it can be changed to something else there?  In any event, I don't want to hold this up over an experimental name, just thinking out loud.
  
  I tend to agree with @martinvonz that setting the successor back to the commit it was absorbed into would be confusing, and seems like a way to split the stack into two stacks on that destination, with the corresponding merge conflicts.  (Would the new "`hg evolve` => stabilize all descendants functionality" even stop at the break?  I don't think it would.)  I get what you're going for though if you want to track content.  But it seems like it would cause non power users a lot of confusion.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers, martinvonz, marmoute
Cc: mjacob, marmoute, mharbison72, martinvonz, pulkit, quark, mercurial-devel
phabricator - March 11, 2020, 10:20 p.m.
mjacob added a comment.


  In D7631#123321 <https://phab.mercurial-scm.org/D7631#123321>, @mharbison72 wrote:
  
  > In D7631#123318 <https://phab.mercurial-scm.org/D7631#123318>, @mjacob wrote:
  >
  >> I like the name `--from` for the option. It would also make sense in combination with a possible future `--into` option.
  >
  > The unfortunate thing about `--from` is that it implies a range in `hg fold`, whereas here it seems to be a single(?) commit.  It does make a lot of sense within the context of this command though (i.e. taking stuff //from// this commit).  And it took me awhile to get comfortable with the meaning in `hg fold`, so maybe it can be changed to something else there?  In any event, I don't want to hold this up over an experimental name, just thinking out loud.
  
  The `--from` in `hg fold` does not only have a different meaning, it’s also a flag that does not take an argument. The `hg fold` command itself takes revisions as arguments, so people might think they pass a revision to `--from` while they actually pass a revision to `hg fold` (having the same meaning as if it is passed to `-r`). Coincidentally, I was asked for help today from someone misusing the command that way. The `hg rewind` command also takes a `--from` option, this time with an argument. I expect that at least one of it might get changed eventually and I think we should not block this patch on that discussion.
  
  Of course, if someone comes up with a name that doesn’t have these problems and is intuitive, we should use that name.
  
  > I tend to agree with @martinvonz that setting the successor back to the commit it was absorbed into would be confusing, and seems like a way to split the stack into two stacks on that destination, with the corresponding merge conflicts.  (Would the new "`hg evolve` => stabilize all descendants functionality" even stop at the break?  I don't think it would.)  I get what you're going for though if you want to track content.  But it seems like it would cause non power users a lot of confusion.
  
  I prepared a patch that helps to prevent that the stack gets split: https://foss.heptapod.net/mercurial/evolve/merge_requests/126
  
  As an example:
  
    echo a > a; hg add a; hg ci -m a
    echo b > b; hg add b; hg ci -m b
    echo c > c; hg add c; hg ci -m c
    echo a1 > a; echo b1 > b; hg ci -m 'change a and b'
    echo d > d; hg add d; hg ci -m d
  
  Graph:
  
    @  (4) d
    |
    o  (3) change a and b
    |
    o  (2) c
    |
    o  (1) b
    |
    o  (0) a
  
  First, let’s emulate a run of `hg absorb --from 3` that adds markers.
  
    hg up 3
    hg uncommit --all
    hg up 2 --merge
    hg absorb --apply-changes
    hg prune -r 5 -s '6 + 7' --split
  
  Graph:
  
    @  (8) c
    |
    o  (7) b
    |
    o  (6) a
    
    *  (4) d
    |
    x  (3) change a and b — split using prune, uncommit as 6, 7
    |
    x  (2) c — rebased using absorb as 8
    |
    x  (1) b — rewritten using absorb as 7
    |
    x  (0) a — amended using absorb as 6
  
  Now, `hg evolve -r 4` would have the following result without the above patch:
  
    o  (9) d
    |
    | @  (8) c
    |/
    o  (7) b
    |
    o  (6) a
  
  and with the above patch:
  
    o  (9) d
    |
    @  (8) c
    |
    o  (7) b
    |
    o  (6) a

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7631/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7631

To: rdamazio, #hg-reviewers, martinvonz, marmoute
Cc: mjacob, marmoute, mharbison72, martinvonz, pulkit, quark, mercurial-devel

Patch

diff --git a/tests/test-absorb.t b/tests/test-absorb.t
--- a/tests/test-absorb.t
+++ b/tests/test-absorb.t
@@ -143,7 +143,7 @@ 
   nothing applied
   [1]
 
-Insertaions:
+Insertions:
 
   $ cat > a << EOF
   > insert before 2b
diff --git a/tests/test-absorb.t b/tests/test-absorb-rev.t
copy from tests/test-absorb.t
copy to tests/test-absorb-rev.t
--- a/tests/test-absorb.t
+++ b/tests/test-absorb-rev.t
@@ -1,26 +1,14 @@ 
   $ cat >> $HGRCPATH << EOF
   > [extensions]
   > absorb=
+  > rebase=
+  > [experimental]
+  > evolution=createmarkers
   > EOF
 
-  $ sedi() { # workaround check-code
-  > pattern="$1"
-  > shift
-  > for i in "$@"; do
-  >     sed "$pattern" "$i" > "$i".tmp
-  >     mv "$i".tmp "$i"
-  > done
-  > }
-
   $ hg init repo1
   $ cd repo1
 
-Do not crash with empty repo:
-
-  $ hg absorb
-  abort: no mutable changeset to change
-  [255]
-
 Make some commits:
 
   $ for i in 1 2 3 4 5; do
@@ -45,9 +33,13 @@ 
   > 5e
   > EOF
 
+Commit that, too.
+
+  $ hg commit -qm "commit to absorb"
+
 Preview absorb changes:
 
-  $ hg absorb --print-changes --dry-run
+  $ hg absorb --print-changes --dry-run -r .
   showing changes for a
           @@ -0,2 +0,2 @@
   4ec16f8 -1
@@ -66,462 +58,111 @@ 
   5c5f952 commit 2
   4ec16f8 commit 1
 
+Add an uncommitted working directory change:
+
+  $ echo 6 >> a
+
 Run absorb:
 
-  $ hg absorb --apply-changes
-  saved backup bundle to * (glob)
+  $ hg absorb --apply-changes -r .
+  1 new orphan changesets
   2 of 2 chunk(s) applied
-  $ hg annotate a
-  0: 1a
-  1: 2b
-  2: 3
-  3: 4d
-  4: 5e
+
+Check that the pending wdir change was left alone:
+
+  $ grep 6 a
+  6
+  $ hg update -Cq .
+
+Rebase the absorbed revision on top of the destination (as evolve would):
+TODO: the evolve-type operation should happen automatically for the changeset
+being absorbed, and even through that the pending wdir change should be left
+alone.
+
+  $ hg rebase -d tip -r .
+  rebasing 5:1631091f9648 "commit to absorb"
+  note: not rebasing 5:1631091f9648 "commit to absorb", its destination already has all its changes
 
-Delete a few lines and related commits will be removed if they will be empty:
+  $ hg log -G -T '{node|short} {desc} {instabilities}'
+  @  2f7ba78d6abc commit 5
+  |
+  o  04c8ba6df782 commit 4
+  |
+  o  484c6ac0cea3 commit 3
+  |
+  o  9b19176bb127 commit 2
+  |
+  o  241ace8326d0 commit 1
+  
+  $ hg annotate -c a
+  241ace8326d0: 1a
+  9b19176bb127: 2b
+  484c6ac0cea3: 3
+  04c8ba6df782: 4d
+  2f7ba78d6abc: 5e
+
+Do it again, but this time with an unrelated commit checked out (plus working
+directory changes on top):
 
   $ cat > a <<EOF
+  > 1a
   > 2b
-  > 4d
+  > 3
+  > 4f
+  > 5g
   > EOF
-  $ echo y | hg absorb --config ui.interactive=1
+  $ hg commit -qm "commit to absorb 2"
+  $ TOABSORB=$(hg id -i)
+
+  $ hg update -q 241ace8326d0
+  $ echo committed unrelated >> a
+  $ hg commit -qm "unrelated commit"
+  $ echo pending wdir change >> a
+
+  $ hg absorb --apply-changes --print-changes -r ${TOABSORB}
   showing changes for a
-          @@ -0,1 +0,0 @@
-  f548282 -1a
-          @@ -2,1 +1,0 @@
-  ff5d556 -3
-          @@ -4,1 +2,0 @@
-  84e5416 -5e
+          @@ -3,2 +3,2 @@
+  04c8ba6 -4d
+  2f7ba78 -5e
+  04c8ba6 +4f
+  2f7ba78 +5g
   
-  3 changesets affected
-  84e5416 commit 5
-  ff5d556 commit 3
-  f548282 commit 1
-  apply changes (yn)?  y
-  saved backup bundle to * (glob)
-  3 of 3 chunk(s) applied
-  $ hg annotate a
-  1: 2b
-  2: 4d
-  $ hg log -T '{rev} {desc}\n' -Gp
-  @  2 commit 4
-  |  diff -r 1cae118c7ed8 -r 58a62bade1c6 a
-  |  --- a/a	Thu Jan 01 00:00:00 1970 +0000
-  |  +++ b/a	Thu Jan 01 00:00:00 1970 +0000
-  |  @@ -1,1 +1,2 @@
-  |   2b
-  |  +4d
+  2 changesets affected
+  2f7ba78 commit 5
+  04c8ba6 commit 4
+  1 new orphan changesets
+  1 of 1 chunk(s) applied
+
+  $ hg annotate -c a -r 'wdir()'
+  241ace8326d0 : 1a
+  dbce69d9fe03 : committed unrelated
+  dbce69d9fe03+: pending wdir change
+
+
+  $ hg update -Cq .
+
+  $ hg rebase -d tip -r ${TOABSORB}
+  rebasing \d+:[0-9a-f]+ "commit to absorb 2" (re)
+  note: not rebasing \d+:[0-9a-f]+ "commit to absorb 2", its destination already has all its changes (re)
+
+  $ hg log -G -T '{node|short} {desc} {instabilities}'
+  o  789b01face13 commit 5
   |
-  o  1 commit 2
-  |  diff -r 84add69aeac0 -r 1cae118c7ed8 a
-  |  --- a/a	Thu Jan 01 00:00:00 1970 +0000
-  |  +++ b/a	Thu Jan 01 00:00:00 1970 +0000
-  |  @@ -0,0 +1,1 @@
-  |  +2b
+  o  9c83c60f49f2 commit 4
   |
-  o  0 commit 1
+  | @  dbce69d9fe03 unrelated commit
+  | |
+  o |  484c6ac0cea3 commit 3
+  | |
+  o |  9b19176bb127 commit 2
+  |/
+  o  241ace8326d0 commit 1
   
 
-Non 1:1 map changes will be ignored:
-
-  $ echo 1 > a
-  $ hg absorb --apply-changes
-  nothing applied
-  [1]
-
-The prompt is not given if there are no changes to be applied, even if there
-are some changes that won't be applied:
-
-  $ hg absorb
-  showing changes for a
-          @@ -0,2 +0,1 @@
-          -2b
-          -4d
-          +1
-  
-  0 changesets affected
-  nothing applied
-  [1]
-
-Insertaions:
-
-  $ cat > a << EOF
-  > insert before 2b
-  > 2b
-  > 4d
-  > insert aftert 4d
-  > EOF
-  $ hg absorb -q --apply-changes
-  $ hg status
-  $ hg annotate a
-  1: insert before 2b
-  1: 2b
-  2: 4d
-  2: insert aftert 4d
-
-Bookmarks are moved:
-
-  $ hg bookmark -r 1 b1
-  $ hg bookmark -r 2 b2
-  $ hg bookmark ba
-  $ hg bookmarks
-     b1                        1:b35060a57a50
-     b2                        2:946e4bc87915
-   * ba                        2:946e4bc87915
-  $ sedi 's/insert/INSERT/' a
-  $ hg absorb -q --apply-changes
-  $ hg status
-  $ hg bookmarks
-     b1                        1:a4183e9b3d31
-     b2                        2:c9b20c925790
-   * ba                        2:c9b20c925790
-
-Non-modified files are ignored:
-
-  $ touch b
-  $ hg commit -A b -m b
-  $ touch c
-  $ hg add c
-  $ hg rm b
-  $ hg absorb --apply-changes
-  nothing applied
-  [1]
-  $ sedi 's/INSERT/Insert/' a
-  $ hg absorb --apply-changes
-  saved backup bundle to * (glob)
-  2 of 2 chunk(s) applied
-  $ hg status
-  A c
-  R b
-
-Public commits will not be changed:
-
-  $ hg phase -p 1
-  $ sedi 's/Insert/insert/' a
-  $ hg absorb -pn
-  showing changes for a
-          @@ -0,1 +0,1 @@
-          -Insert before 2b
-          +insert before 2b
-          @@ -3,1 +3,1 @@
-  85b4e0e -Insert aftert 4d
-  85b4e0e +insert aftert 4d
-  
-  1 changesets affected
-  85b4e0e commit 4
-  $ hg absorb --apply-changes
-  saved backup bundle to * (glob)
-  1 of 2 chunk(s) applied
-  $ hg diff -U 0
-  diff -r 1c8eadede62a a
-  --- a/a	Thu Jan 01 00:00:00 1970 +0000
-  +++ b/a	* (glob)
-  @@ -1,1 +1,1 @@
-  -Insert before 2b
-  +insert before 2b
-  $ hg annotate a
-  1: Insert before 2b
-  1: 2b
-  2: 4d
-  2: insert aftert 4d
-
-  $ hg co -qC 1
-  $ sedi 's/Insert/insert/' a
-  $ hg absorb --apply-changes
-  abort: no mutable changeset to change
-  [255]
-
-Make working copy clean:
-
-  $ hg co -qC ba
-  $ rm c
-  $ hg status
-
-Merge commit will not be changed:
-
-  $ echo 1 > m1
-  $ hg commit -A m1 -m m1
-  $ hg bookmark -q -i m1
-  $ hg update -q '.^'
-  $ echo 2 > m2
-  $ hg commit -q -A m2 -m m2
-  $ hg merge -q m1
-  $ hg commit -m merge
-  $ hg bookmark -d m1
-  $ hg log -G -T '{rev} {desc} {phase}\n'
-  @    6 merge draft
-  |\
-  | o  5 m2 draft
-  | |
-  o |  4 m1 draft
-  |/
-  o  3 b draft
-  |
-  o  2 commit 4 draft
-  |
-  o  1 commit 2 public
-  |
-  o  0 commit 1 public
-  
-  $ echo 2 >> m1
-  $ echo 2 >> m2
-  $ hg absorb --apply-changes
-  abort: cannot absorb into a merge
-  [255]
-  $ hg revert -q -C m1 m2
-
-Use a new repo:
-
-  $ cd ..
-  $ hg init repo2
-  $ cd repo2
-
-Make some commits to multiple files:
-
-  $ for f in a b; do
-  >   for i in 1 2; do
-  >     echo $f line $i >> $f
-  >     hg commit -A $f -m "commit $f $i" -q
-  >   done
-  > done
-
-Use pattern to select files to be fixed up:
-
-  $ sedi 's/line/Line/' a b
-  $ hg status
-  M a
-  M b
-  $ hg absorb --apply-changes a
-  saved backup bundle to * (glob)
-  1 of 1 chunk(s) applied
-  $ hg status
-  M b
-  $ hg absorb --apply-changes --exclude b
-  nothing applied
-  [1]
-  $ hg absorb --apply-changes b
-  saved backup bundle to * (glob)
-  1 of 1 chunk(s) applied
-  $ hg status
-  $ cat a b
-  a Line 1
-  a Line 2
-  b Line 1
-  b Line 2
-
-Test config option absorb.max-stack-size:
+  $ hg annotate -c a -r tip
+  241ace8326d0: 1a
+  9b19176bb127: 2b
+  484c6ac0cea3: 3
+  9c83c60f49f2: 4f
+  789b01face13: 5g
 
-  $ sedi 's/Line/line/' a b
-  $ hg log -T '{rev}:{node} {desc}\n'
-  3:712d16a8f445834e36145408eabc1d29df05ec09 commit b 2
-  2:74cfa6294160149d60adbf7582b99ce37a4597ec commit b 1
-  1:28f10dcf96158f84985358a2e5d5b3505ca69c22 commit a 2
-  0:f9a81da8dc53380ed91902e5b82c1b36255a4bd0 commit a 1
-  $ hg --config absorb.max-stack-size=1 absorb -pn
-  absorb: only the recent 1 changesets will be analysed
-  showing changes for a
-          @@ -0,2 +0,2 @@
-          -a Line 1
-          -a Line 2
-          +a line 1
-          +a line 2
-  showing changes for b
-          @@ -0,2 +0,2 @@
-          -b Line 1
-  712d16a -b Line 2
-          +b line 1
-  712d16a +b line 2
-  
-  1 changesets affected
-  712d16a commit b 2
-
-Test obsolete markers creation:
-
-  $ cat >> $HGRCPATH << EOF
-  > [experimental]
-  > evolution=createmarkers
-  > [absorb]
-  > add-noise=1
-  > EOF
-
-  $ hg --config absorb.max-stack-size=3 absorb -a
-  absorb: only the recent 3 changesets will be analysed
-  2 of 2 chunk(s) applied
-  $ hg log -T '{rev}:{node|short} {desc} {get(extras, "absorb_source")}\n'
-  6:3dfde4199b46 commit b 2 712d16a8f445834e36145408eabc1d29df05ec09
-  5:99cfab7da5ff commit b 1 74cfa6294160149d60adbf7582b99ce37a4597ec
-  4:fec2b3bd9e08 commit a 2 28f10dcf96158f84985358a2e5d5b3505ca69c22
-  0:f9a81da8dc53 commit a 1 
-  $ hg absorb --apply-changes
-  1 of 1 chunk(s) applied
-  $ hg log -T '{rev}:{node|short} {desc} {get(extras, "absorb_source")}\n'
-  10:e1c8c1e030a4 commit b 2 3dfde4199b4610ea6e3c6fa9f5bdad8939d69524
-  9:816c30955758 commit b 1 99cfab7da5ffdaf3b9fc6643b14333e194d87f46
-  8:5867d584106b commit a 2 fec2b3bd9e0834b7cb6a564348a0058171aed811
-  7:8c76602baf10 commit a 1 f9a81da8dc53380ed91902e5b82c1b36255a4bd0
-
-Executable files:
-
-  $ cat >> $HGRCPATH << EOF
-  > [diff]
-  > git=True
-  > EOF
-  $ cd ..
-  $ hg init repo3
-  $ cd repo3
-
-#if execbit
-  $ echo > foo.py
-  $ chmod +x foo.py
-  $ hg add foo.py
-  $ hg commit -mfoo
-#else
-  $ hg import -q --bypass - <<EOF
-  > # HG changeset patch
-  > foo
-  > 
-  > diff --git a/foo.py b/foo.py
-  > new file mode 100755
-  > --- /dev/null
-  > +++ b/foo.py
-  > @@ -0,0 +1,1 @@
-  > +
-  > EOF
-  $ hg up -q
-#endif
-
-  $ echo bla > foo.py
-  $ hg absorb --dry-run --print-changes
-  showing changes for foo.py
-          @@ -0,1 +0,1 @@
-  99b4ae7 -
-  99b4ae7 +bla
-  
-  1 changesets affected
-  99b4ae7 foo
-  $ hg absorb --dry-run --interactive --print-changes
-  diff -r 99b4ae712f84 foo.py
-  1 hunks, 1 lines changed
-  examine changes to 'foo.py'?
-  (enter ? for help) [Ynesfdaq?] y
-  
-  @@ -1,1 +1,1 @@
-  -
-  +bla
-  record this change to 'foo.py'?
-  (enter ? for help) [Ynesfdaq?] y
-  
-  showing changes for foo.py
-          @@ -0,1 +0,1 @@
-  99b4ae7 -
-  99b4ae7 +bla
-  
-  1 changesets affected
-  99b4ae7 foo
-  $ hg absorb --apply-changes
-  1 of 1 chunk(s) applied
-  $ hg diff -c .
-  diff --git a/foo.py b/foo.py
-  new file mode 100755
-  --- /dev/null
-  +++ b/foo.py
-  @@ -0,0 +1,1 @@
-  +bla
-  $ hg diff
-
-Remove lines may delete changesets:
-
-  $ cd ..
-  $ hg init repo4
-  $ cd repo4
-  $ cat > a <<EOF
-  > 1
-  > 2
-  > EOF
-  $ hg commit -m a12 -A a
-  $ cat > b <<EOF
-  > 1
-  > 2
-  > EOF
-  $ hg commit -m b12 -A b
-  $ echo 3 >> b
-  $ hg commit -m b3
-  $ echo 4 >> b
-  $ hg commit -m b4
-  $ echo 1 > b
-  $ echo 3 >> a
-  $ hg absorb -pn
-  showing changes for a
-          @@ -2,0 +2,1 @@
-  bfafb49 +3
-  showing changes for b
-          @@ -1,3 +1,0 @@
-  1154859 -2
-  30970db -3
-  a393a58 -4
-  
-  4 changesets affected
-  a393a58 b4
-  30970db b3
-  1154859 b12
-  bfafb49 a12
-  $ hg absorb -av | grep became
-  0:bfafb49242db: 1 file(s) changed, became 4:1a2de97fc652
-  1:115485984805: 2 file(s) changed, became 5:0c930dfab74c
-  2:30970dbf7b40: became empty and was dropped
-  3:a393a58b9a85: became empty and was dropped
-  $ hg log -T '{rev} {desc}\n' -Gp
-  @  5 b12
-  |  diff --git a/b b/b
-  |  new file mode 100644
-  |  --- /dev/null
-  |  +++ b/b
-  |  @@ -0,0 +1,1 @@
-  |  +1
-  |
-  o  4 a12
-     diff --git a/a b/a
-     new file mode 100644
-     --- /dev/null
-     +++ b/a
-     @@ -0,0 +1,3 @@
-     +1
-     +2
-     +3
-  
-
-Use revert to make the current change and its parent disappear.
-This should move us to the non-obsolete ancestor.
-
-  $ cd ..
-  $ hg init repo5
-  $ cd repo5
-  $ cat > a <<EOF
-  > 1
-  > 2
-  > EOF
-  $ hg commit -m a12 -A a
-  $ hg id
-  bfafb49242db tip
-  $ echo 3 >> a
-  $ hg commit -m a123 a
-  $ echo 4 >> a
-  $ hg commit -m a1234 a
-  $ hg id
-  82dbe7fd19f0 tip
-  $ hg revert -r 0 a
-  $ hg absorb -pn
-  showing changes for a
-          @@ -2,2 +2,0 @@
-  f1c23dd -3
-  82dbe7f -4
-  
-  2 changesets affected
-  82dbe7f a1234
-  f1c23dd a123
-  $ hg absorb --apply-changes --verbose
-  1:f1c23dd5d08d: became empty and was dropped
-  2:82dbe7fd19f0: became empty and was dropped
-  a: 1 of 1 chunk(s) applied
-  $ hg id
-  bfafb49242db tip
diff --git a/hgext/absorb.py b/hgext/absorb.py
--- a/hgext/absorb.py
+++ b/hgext/absorb.py
@@ -979,18 +979,20 @@ 
     return overlaycontext(memworkingcopy, ctx)
 
 
-def absorb(ui, repo, stack=None, targetctx=None, pats=None, opts=None):
+def absorb(ui, repo, targetctx, stack=None, pats=None, opts=None):
     """pick fixup chunks from targetctx, apply them to stack.
 
-    if targetctx is None, the working copy context will be used.
     if stack is None, the current draft stack will be used.
     return fixupstate.
     """
     if stack is None:
         limit = ui.configint(b'absorb', b'max-stack-size')
-        headctx = repo[b'.']
+        headctx = targetctx.p1()
         if len(headctx.parents()) > 1:
             raise error.Abort(_(b'cannot absorb into a merge'))
+        if len(targetctx.parents()) > 1:
+            raise error.Abort(_(b'cannot absorb a merge'))
+
         stack = getdraftstack(headctx, limit)
         if limit and len(stack) >= limit:
             ui.warn(
@@ -1002,8 +1004,6 @@ 
             )
     if not stack:
         raise error.Abort(_(b'no mutable changeset to change'))
-    if targetctx is None:  # default to working copy
-        targetctx = repo[None]
     if pats is None:
         pats = ()
     if opts is None:
@@ -1088,6 +1088,13 @@ 
                 b'(EXPERIMENTAL)'
             ),
         ),
+        (
+            b'r',
+            b'rev',
+            b'',
+            _(b'the revision to absorb changes from, if not the working '
+              b'directory'),
+        ),
     ]
     + commands.dryrunopts
     + commands.templateopts
@@ -1099,9 +1106,9 @@ 
 def absorbcmd(ui, repo, *pats, **opts):
     """incorporate corrections into the stack of draft changesets
 
-    absorb analyzes each change in your working directory and attempts to
-    amend the changed lines into the changesets in your stack that first
-    introduced those lines.
+    absorb analyzes each change in your working directory (or the revision given
+    to `--rev`, if one is specified) and attempts to amend the changed lines
+    into the changesets in your stack that first introduced those lines.
 
     If absorb cannot find an unambiguous changeset to amend for a change,
     that change will be left in the working directory, untouched. They can be
@@ -1126,6 +1133,10 @@ 
         if not opts[b'dry_run']:
             cmdutil.checkunfinished(repo)
 
-        state = absorb(ui, repo, pats=pats, opts=opts)
+        rev = opts[b'rev']
+        # default to working copy
+        ctx = scmutil.revsingle(repo, rev, default=None)
+
+        state = absorb(ui, repo, ctx, pats=pats, opts=opts)
         if sum(s[0] for s in state.chunkstats.values()) == 0:
             return 1