Patchwork [STABLE] record: prevent commits that don't pick up dirty subrepo changes (issue6102)

login
register
mail settings
Submitter Matt Harbison
Date March 17, 2019, 2:58 a.m.
Message ID <op.zyrzv0hp9lwrgf@envy>
Download mbox | patch
Permalink /patch/39305/
State New
Headers show

Comments

Matt Harbison - March 17, 2019, 2:58 a.m.
On Sat, 16 Mar 2019 22:01:05 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> On Sat, 16 Mar 2019 18:20:37 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1552761621 14400
>> #      Sat Mar 16 14:40:21 2019 -0400
>> # Branch stable
>> # Node ID 0d48bef566bf86ade809225d7e8b08c37914beb4
>> # Parent  25fc5b96d1c30468417ee0d690c2979db362edd0
>> record: prevent commits that don't pick up dirty subrepo changes  
>> (issue6102)
>
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -248,6 +248,21 @@ def dorecord(ui, repo, commitfunc, cmdsu
>>      if not opts.get('user'):
>>          ui.username() # raise exception, username not provided
>>
>> +    wctx = repo[None]
>> +    match = scmutil.match(wctx, pats, opts)
>> +    status = repo.status(match=match)
>> +
>> +    overrides = {(b'ui', b'commitsubrepos'): True}
>> +
>> +    with repo.ui.configoverride(overrides, b'record'):
>> +        # Force allows -X subrepo.
>> +        subs, commitsubs, newstate = subrepoutil.precommit(
>> +            repo.ui, wctx, status, match, force=True)
>> +        for s in subs:
>> +            if s in commitsubs:
>> +                dirtyreason = wctx.sub(s).dirtyreason(True)
>> +                raise error.Abort(dirtyreason)
>
> Can we move these into recordfunc() where locks should be held properly?
>
>>      def recordfunc(ui, repo, message, match, opts):
>>          """This is generic record driver.

I tried it, but the MQ test fails with weird errors:
Yuya Nishihara - March 17, 2019, 3:14 a.m.
On Sat, 16 Mar 2019 22:58:38 -0400, Matt Harbison wrote:
> On Sat, 16 Mar 2019 22:01:05 -0400, Yuya Nishihara <yuya@tcha.org> wrote:
> > On Sat, 16 Mar 2019 18:20:37 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> # Date 1552761621 14400
> >> #      Sat Mar 16 14:40:21 2019 -0400
> >> # Branch stable
> >> # Node ID 0d48bef566bf86ade809225d7e8b08c37914beb4
> >> # Parent  25fc5b96d1c30468417ee0d690c2979db362edd0
> >> record: prevent commits that don't pick up dirty subrepo changes  
> >> (issue6102)
> >
> >> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> >> --- a/mercurial/cmdutil.py
> >> +++ b/mercurial/cmdutil.py
> >> @@ -248,6 +248,21 @@ def dorecord(ui, repo, commitfunc, cmdsu
> >>      if not opts.get('user'):
> >>          ui.username() # raise exception, username not provided
> >>
> >> +    wctx = repo[None]
> >> +    match = scmutil.match(wctx, pats, opts)
> >> +    status = repo.status(match=match)
> >> +
> >> +    overrides = {(b'ui', b'commitsubrepos'): True}
> >> +
> >> +    with repo.ui.configoverride(overrides, b'record'):
> >> +        # Force allows -X subrepo.
> >> +        subs, commitsubs, newstate = subrepoutil.precommit(
> >> +            repo.ui, wctx, status, match, force=True)
> >> +        for s in subs:
> >> +            if s in commitsubs:
> >> +                dirtyreason = wctx.sub(s).dirtyreason(True)
> >> +                raise error.Abort(dirtyreason)
> >
> > Can we move these into recordfunc() where locks should be held properly?
> >
> >>      def recordfunc(ui, repo, message, match, opts):
> >>          """This is generic record driver.
> 
> I tried it, but the MQ test fails with weird errors:
> 
> --- c:/Users/Matt/hg/tests/test-mq-subrepo.t
> +++ c:/Users/Matt/hg/tests/test-mq-subrepo.t.err
> @@ -301,52 +301,37 @@
>     A .hgsub
>     A sub/a
>     % qrecord --config ui.interactive=1 -m0 0.diff
> -  diff --git a/.hgsub b/.hgsub
> -  new file mode 100644
> -  examine changes to '.hgsub'? [Ynesfdaq?] y
> -
> -  @@ -0,0 +1,1 @@
> -  +sub = sub
> -  record this change to '.hgsub'? [Ynesfdaq?] y
> -
> -  warning: subrepo spec file '.hgsub' not found
> -  warning: subrepo spec file '.hgsub' not found
> +  abort: $TESTTMP\repo-2499-qrecord\.hgsubstate: $ENOENT$
>     path sub
>      source   sub
> -   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
> +   revision
>     $ testmod qrecord --config ui.interactive=1 -m1 1.diff <<EOF
> <snip>

We might have to pass in a copy of status tuple to subrepoutil.precommit(),
which updates the status argument accordingly.

Patch

--- c:/Users/Matt/hg/tests/test-mq-subrepo.t
+++ c:/Users/Matt/hg/tests/test-mq-subrepo.t.err
@@ -301,52 +301,37 @@ 
    A .hgsub
    A sub/a
    % qrecord --config ui.interactive=1 -m0 0.diff
-  diff --git a/.hgsub b/.hgsub
-  new file mode 100644
-  examine changes to '.hgsub'? [Ynesfdaq?] y
-
-  @@ -0,0 +1,1 @@
-  +sub = sub
-  record this change to '.hgsub'? [Ynesfdaq?] y
-
-  warning: subrepo spec file '.hgsub' not found
-  warning: subrepo spec file '.hgsub' not found
+  abort: $TESTTMP\repo-2499-qrecord\.hgsubstate: $ENOENT$
    path sub
     source   sub
-   revision b2fdb12cd82b021c3b7053d67802e77b6eeaee31
+   revision
    $ testmod qrecord --config ui.interactive=1 -m1 1.diff <<EOF
<snip>

I'll try to find some time tomorrow to dig into this more.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel