Patchwork [4,of,5,V2] simplemerge: use contexts to read file data from, if passed

# HG changeset patch
# User Phil Cohen <phillco@fb.com>
# Date 1499213534 25200
#      Tue Jul 04 17:12:14 2017 -0700
# Node ID fa38e77dd154d1dbabb2505dd27d972a2d7cc5cf
# Parent  02dab4eab9e9bbba645a5c2f9a5050bc55b34639
simplemerge: use contexts to read file data from, if passed

Phil Cohen <phillco@fb.com> writes:

> # HG changeset patch
> # User Phil Cohen <phillco@fb.com>
> # Date 1499213534 25200
> #      Tue Jul 04 17:12:14 2017 -0700
> # Node ID fa38e77dd154d1dbabb2505dd27d972a2d7cc5cf
> # Parent  02dab4eab9e9bbba645a5c2f9a5050bc55b34639
> simplemerge: use contexts to read file data from, if passed
>
> diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py
> --- a/mercurial/simplemerge.py
> +++ b/mercurial/simplemerge.py
> @@ -409,12 +409,30 @@
>  
>  def simplemerge(ui, localfile, basefile, otherfile,
>                  localctx=None, basectx=None, otherctx=None, repo=None, **opts):
> +    """Performs the simplemerge algorithm.
> +
> +    {local|base|other}ctx are optional. If passed, they (local/base/other) will
> +    be read from. You should pass explicit labels in this mode since the default
> +    is to use the file paths."""
>      def readfile(filename):
>          f = open(filename, "rb")
>          text = f.read()
>          f.close()
>          return _verifytext(text, filename, ui, opts)
>  
> +    def readctx(ctx):
> +        if not ctx:
> +            return None
> +        if not repo:
> +            raise error.ProgrammingError('simplemerge: repo must be passed if '
> +                                         'using contexts')
> +        # `wwritedata` is used to get the post-filter data from `ctx` (i.e.,
> +        # what would have been in the working copy). Since merges were run in
> +        # the working copy, and thus used post-filter data, we do the same to
> +        # maintain behavior.
> +        return repo.wwritedata(ctx.path(),
> +                               _verifytext(ctx.data(), ctx.path(), ui, opts))
> +
>      mode = opts.get('mode','merge')
>      if mode == 'union':
>          name_a = None
> @@ -435,9 +453,9 @@
>              raise error.Abort(_("can only specify three labels."))
>  
>      try:
> -        localtext = readfile(localfile)
> -        basetext = readfile(basefile)
> -        othertext = readfile(otherfile)
> +        localtext = readctx(localctx) if localctx else readfile(localfile)
> +        basetext = readctx(basectx) if basectx else readfile(basefile)
> +        othertext = readctx(otherctx) if otherctx else readfile(otherfile)

I see. You're keeping the old parameters for, presumably, deferring
patching contrib/simplemerge? I'd like to avoid having this difference
in behavior for this method (sending filenames xor sending contexts) but
if this is just a stepping stone, then that's fine with me.

>      except error.Abort:
>          return 1
>  
> diff --git a/tests/test-lfconvert.t b/tests/test-lfconvert.t
> --- a/tests/test-lfconvert.t
> +++ b/tests/test-lfconvert.t
> @@ -128,7 +128,7 @@
>    $ hg merge
>    merging sub/maybelarge.dat and stuff/maybelarge.dat to stuff/maybelarge.dat
>    merging sub/normal2 and stuff/normal2 to stuff/normal2
> -  warning: $TESTTMP/bigfile-repo/stuff/maybelarge.dat looks like a binary file. (glob)
> +  warning: stuff/maybelarge.dat looks like a binary file.

I think you accidentally changed this.

I was going to, just to keep our options open, but if it's undesirable
I can close that off and have contrib/simplemerge pass a fake context
and labels.

> > -  warning: $TESTTMP/bigfile-repo/stuff/maybelarge.dat looks like a binary file. (glob)
> > +  warning: stuff/maybelarge.dat looks like a binary file.
> I think you accidentally changed this.

Now that we're passing localctx.path() to that warning, it prints a
relative path, not an absolute one. I think that's ~0.001% better?

On Tue, Jul 4, 2017 at 7:06 PM, Sean Farley <sean@farley.io> wrote:
>
> Phil Cohen <phillco@fb.com> writes:
>
>> # HG changeset patch
>> # User Phil Cohen <phillco@fb.com>
>> # Date 1499213534 25200
>> #      Tue Jul 04 17:12:14 2017 -0700
>> # Node ID fa38e77dd154d1dbabb2505dd27d972a2d7cc5cf
>> # Parent  02dab4eab9e9bbba645a5c2f9a5050bc55b34639
>> simplemerge: use contexts to read file data from, if passed
>>
>> diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py
>> --- a/mercurial/simplemerge.py
>> +++ b/mercurial/simplemerge.py
>> @@ -409,12 +409,30 @@
>>
>>  def simplemerge(ui, localfile, basefile, otherfile,
>>                  localctx=None, basectx=None, otherctx=None, repo=None, **opts):
>> +    """Performs the simplemerge algorithm.
>> +
>> +    {local|base|other}ctx are optional. If passed, they (local/base/other) will
>> +    be read from. You should pass explicit labels in this mode since the default
>> +    is to use the file paths."""
>>      def readfile(filename):
>>          f = open(filename, "rb")
>>          text = f.read()
>>          f.close()
>>          return _verifytext(text, filename, ui, opts)
>>
>> +    def readctx(ctx):
>> +        if not ctx:
>> +            return None
>> +        if not repo:
>> +            raise error.ProgrammingError('simplemerge: repo must be passed if '
>> +                                         'using contexts')
>> +        # `wwritedata` is used to get the post-filter data from `ctx` (i.e.,
>> +        # what would have been in the working copy). Since merges were run in
>> +        # the working copy, and thus used post-filter data, we do the same to
>> +        # maintain behavior.
>> +        return repo.wwritedata(ctx.path(),
>> +                               _verifytext(ctx.data(), ctx.path(), ui, opts))
>> +
>>      mode = opts.get('mode','merge')
>>      if mode == 'union':
>>          name_a = None
>> @@ -435,9 +453,9 @@
>>              raise error.Abort(_("can only specify three labels."))
>>
>>      try:
>> -        localtext = readfile(localfile)
>> -        basetext = readfile(basefile)
>> -        othertext = readfile(otherfile)
>> +        localtext = readctx(localctx) if localctx else readfile(localfile)
>> +        basetext = readctx(basectx) if basectx else readfile(basefile)
>> +        othertext = readctx(otherctx) if otherctx else readfile(otherfile)
>
> I see. You're keeping the old parameters for, presumably, deferring
> patching contrib/simplemerge? I'd like to avoid having this difference
> in behavior for this method (sending filenames xor sending contexts) but
> if this is just a stepping stone, then that's fine with me.
>
>>      except error.Abort:
>>          return 1
>>
>> diff --git a/tests/test-lfconvert.t b/tests/test-lfconvert.t
>> --- a/tests/test-lfconvert.t
>> +++ b/tests/test-lfconvert.t
>> @@ -128,7 +128,7 @@
>>    $ hg merge
>>    merging sub/maybelarge.dat and stuff/maybelarge.dat to stuff/maybelarge.dat
>>    merging sub/normal2 and stuff/normal2 to stuff/normal2
>> -  warning: $TESTTMP/bigfile-repo/stuff/maybelarge.dat looks like a binary file. (glob)
>> +  warning: stuff/maybelarge.dat looks like a binary file.
>
> I think you accidentally changed this.
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>