Patchwork [2,of,2] remotefilelog: prevent this extension from loading on Windows

login
register
mail settings
Submitter Matt Harbison
Date Nov. 28, 2018, 3:46 a.m.
Message ID <aac9ae9f512f53a907dc.1543376800@Envy>
Download mbox | patch
Permalink /patch/36822/
State Accepted
Headers show

Comments

Matt Harbison - Nov. 28, 2018, 3:46 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1543376352 18000
#      Tue Nov 27 22:39:12 2018 -0500
# Node ID aac9ae9f512f53a907dc799d2d6050931001e6e1
# Parent  c743bb1c19513a61925ce4663d33323aea7016cc
remotefilelog: prevent this extension from loading on Windows

See 0800d9e6e216.

This doesn't actually abort.  Rather, it prints a stacktrace, says the extension
failed to setup, and then runs the command.  Probably good enough for something
marked "highly experimental", and hopefully somebody can figure out how to fix
what doesn't work.

There actually is code and comments indicating workarounds for quirks on
Windows.  But some of it is puzzling- for example, shallowutil.unlinkfile()
handles unlinking read only files, but so does win32.unlink().  So I'm not sure
why the former exists.
Augie Fackler - Nov. 28, 2018, 6:59 p.m.
> On Nov 27, 2018, at 22:46, Matt Harbison <mharbison72@gmail.com> wrote:
> 
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1543376352 18000
> #      Tue Nov 27 22:39:12 2018 -0500
> # Node ID aac9ae9f512f53a907dc799d2d6050931001e6e1
> # Parent  c743bb1c19513a61925ce4663d33323aea7016cc
> remotefilelog: prevent this extension from loading on Windows

Hm, I have *vivid* memories of speaking up earlier, but I must not have sent it to the list. Fail on my part. Anyway: I *know* remotefilelog works on Windows. I'll probably tackle getting the tests to pass since they don't, but I'd appreciate it if we could leave RFL working there even if we can't test it.

> See 0800d9e6e216.
> 
> This doesn't actually abort.  Rather, it prints a stacktrace, says the extension
> failed to setup, and then runs the command.  Probably good enough for something
> marked "highly experimental", and hopefully somebody can figure out how to fix
> what doesn't work.

That'd be one thing if you did this *after* the monkeypatches, but instead this is before the monkeypatches, so you've *definitely* made the situation worse on Windows - things worked before this patch, and now most of the uisetup won't happen so you won't be able to invoke a remotefilelog clone.

> There actually is code and comments indicating workarounds for quirks on
> Windows.  But some of it is puzzling- for example, shallowutil.unlinkfile()
> handles unlinking read only files, but so does win32.unlink().  So I'm not sure
> why the former exists.

That's because it works on Windows. As for the weird shallowutil.unlinkfile, that's probably someone having been sloppy and/or wanting to avoid a dependency for some reason lost to history.

> 
> diff --git a/hgext/remotefilelog/__init__.py b/hgext/remotefilelog/__init__.py
> --- a/hgext/remotefilelog/__init__.py
> +++ b/hgext/remotefilelog/__init__.py
> @@ -257,6 +257,9 @@ def uisetup(ui):
>     """Wraps user facing Mercurial commands to swap them out with shallow
>     versions.
>     """
> +    if pycompat.iswindows:
> +        raise error.Abort(_("remotefilelog is not supported on Windows"))
> +
>     hg.wirepeersetupfuncs.append(fileserverclient.peersetup)
> 
>     entry = extensions.wrapcommand(commands.table, 'clone', cloneshallow)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Matt Harbison - Nov. 28, 2018, 11:41 p.m.
> On Nov 28, 2018, at 1:59 PM, Augie Fackler <raf@durin42.com> wrote:
> 
>> On Nov 27, 2018, at 22:46, Matt Harbison <mharbison72@gmail.com> wrote:
>> 
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1543376352 18000
>> #      Tue Nov 27 22:39:12 2018 -0500
>> # Node ID aac9ae9f512f53a907dc799d2d6050931001e6e1
>> # Parent  c743bb1c19513a61925ce4663d33323aea7016cc
>> remotefilelog: prevent this extension from loading on Windows
> 
> Hm, I have *vivid* memories of speaking up earlier, but I must not have sent it to the list. Fail on my part. Anyway: I *know* remotefilelog works on Windows. I'll probably tackle getting the tests to pass since they don't, but I'd appreciate it if we could leave RFL working there even if we can't test it.

I’m fine with dropping this one.  I was going to suggest to Yuya that I would wait on this a bit in case anybody wanted to try to work on it.  But, I figured I would forget to follow up, and I was here anyway.

Let me know if you need something, like access to Windows.
Yuya Nishihara - Nov. 29, 2018, 11:24 a.m.
On Wed, 28 Nov 2018 18:41:05 -0500, Matt Harbison wrote:
> 
> > On Nov 28, 2018, at 1:59 PM, Augie Fackler <raf@durin42.com> wrote:
> > 
> >> On Nov 27, 2018, at 22:46, Matt Harbison <mharbison72@gmail.com> wrote:
> >> 
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> # Date 1543376352 18000
> >> #      Tue Nov 27 22:39:12 2018 -0500
> >> # Node ID aac9ae9f512f53a907dc799d2d6050931001e6e1
> >> # Parent  c743bb1c19513a61925ce4663d33323aea7016cc
> >> remotefilelog: prevent this extension from loading on Windows
> > 
> > Hm, I have *vivid* memories of speaking up earlier, but I must not have sent it to the list. Fail on my part. Anyway: I *know* remotefilelog works on Windows. I'll probably tackle getting the tests to pass since they don't, but I'd appreciate it if we could leave RFL working there even if we can't test it.
> 
> I’m fine with dropping this one.  I was going to suggest to Yuya that I would wait on this a bit in case anybody wanted to try to work on it.  But, I figured I would forget to follow up, and I was here anyway.

Okay, dropped these. We should backout 0800d9e6e216 to re-enable the failing
tests. Otherwise, no one would fix them.
Matt Harbison - Nov. 29, 2018, 1:03 p.m.
> On Nov 29, 2018, at 6:24 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
>> On Wed, 28 Nov 2018 18:41:05 -0500, Matt Harbison wrote:
>> 
>>>> On Nov 28, 2018, at 1:59 PM, Augie Fackler <raf@durin42.com> wrote:
>>>> 
>>>> On Nov 27, 2018, at 22:46, Matt Harbison <mharbison72@gmail.com> wrote:
>>>> 
>>>> # HG changeset patch
>>>> # User Matt Harbison <matt_harbison@yahoo.com>
>>>> # Date 1543376352 18000
>>>> #      Tue Nov 27 22:39:12 2018 -0500
>>>> # Node ID aac9ae9f512f53a907dc799d2d6050931001e6e1
>>>> # Parent  c743bb1c19513a61925ce4663d33323aea7016cc
>>>> remotefilelog: prevent this extension from loading on Windows
>>> 
>>> Hm, I have *vivid* memories of speaking up earlier, but I must not have sent it to the list. Fail on my part. Anyway: I *know* remotefilelog works on Windows. I'll probably tackle getting the tests to pass since they don't, but I'd appreciate it if we could leave RFL working there even if we can't test it.
>> 
>> I’m fine with dropping this one.  I was going to suggest to Yuya that I would wait on this a bit in case anybody wanted to try to work on it.  But, I figured I would forget to follow up, and I was here anyway.
> 
> Okay, dropped these. We should backout 0800d9e6e216 to re-enable the failing
> tests. Otherwise, no one would fix them.

I get that, but I’d like to leave the tests skipped so no other problems creep in.  They’ve been broken for several weeks now without getting much attention, probably because something is often broken on Windows.
Augie Fackler - Nov. 29, 2018, 1:08 p.m.
On Thu, Nov 29, 2018, 08:03 Matt Harbison <mharbison72@gmail.com wrote:

>
> > On Nov 29, 2018, at 6:24 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> >
> >> On Wed, 28 Nov 2018 18:41:05 -0500, Matt Harbison wrote:
> >>
> >>>> On Nov 28, 2018, at 1:59 PM, Augie Fackler <raf@durin42.com> wrote:
> >>>>
> >>>> On Nov 27, 2018, at 22:46, Matt Harbison <mharbison72@gmail.com>
> wrote:
> >>>>
> >>>> # HG changeset patch
> >>>> # User Matt Harbison <matt_harbison@yahoo.com>
> >>>> # Date 1543376352 18000
> >>>> #      Tue Nov 27 22:39:12 2018 -0500
> >>>> # Node ID aac9ae9f512f53a907dc799d2d6050931001e6e1
> >>>> # Parent  c743bb1c19513a61925ce4663d33323aea7016cc
> >>>> remotefilelog: prevent this extension from loading on Windows
> >>>
> >>> Hm, I have *vivid* memories of speaking up earlier, but I must not
> have sent it to the list. Fail on my part. Anyway: I *know* remotefilelog
> works on Windows. I'll probably tackle getting the tests to pass since they
> don't, but I'd appreciate it if we could leave RFL working there even if we
> can't test it.
> >>
> >> I’m fine with dropping this one.  I was going to suggest to Yuya that I
> would wait on this a bit in case anybody wanted to try to work on it.  But,
> I figured I would forget to follow up, and I was here anyway.
> >
> > Okay, dropped these. We should backout 0800d9e6e216 to re-enable the
> failing
> > tests. Otherwise, no one would fix them.
>
> I get that, but I’d like to leave the tests skipped so no other problems
> creep in.  They’ve been broken for several weeks now without getting much
> attention, probably because something is often broken on Windows.
>

I agree. I'll work on the tests anyway, and I don't want them hiding good
signal on Windows.

>
Yuya Nishihara - Nov. 29, 2018, 1:18 p.m.
On Thu, 29 Nov 2018 08:08:06 -0500, Augie Fackler wrote:
> On Thu, Nov 29, 2018, 08:03 Matt Harbison <mharbison72@gmail.com wrote:
> 
> >
> > > On Nov 29, 2018, at 6:24 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > >
> > >> On Wed, 28 Nov 2018 18:41:05 -0500, Matt Harbison wrote:
> > >>
> > >>>> On Nov 28, 2018, at 1:59 PM, Augie Fackler <raf@durin42.com> wrote:
> > >>>>
> > >>>> On Nov 27, 2018, at 22:46, Matt Harbison <mharbison72@gmail.com>
> > wrote:
> > >>>>
> > >>>> # HG changeset patch
> > >>>> # User Matt Harbison <matt_harbison@yahoo.com>
> > >>>> # Date 1543376352 18000
> > >>>> #      Tue Nov 27 22:39:12 2018 -0500
> > >>>> # Node ID aac9ae9f512f53a907dc799d2d6050931001e6e1
> > >>>> # Parent  c743bb1c19513a61925ce4663d33323aea7016cc
> > >>>> remotefilelog: prevent this extension from loading on Windows
> > >>>
> > >>> Hm, I have *vivid* memories of speaking up earlier, but I must not
> > have sent it to the list. Fail on my part. Anyway: I *know* remotefilelog
> > works on Windows. I'll probably tackle getting the tests to pass since they
> > don't, but I'd appreciate it if we could leave RFL working there even if we
> > can't test it.
> > >>
> > >> I’m fine with dropping this one.  I was going to suggest to Yuya that I
> > would wait on this a bit in case anybody wanted to try to work on it.  But,
> > I figured I would forget to follow up, and I was here anyway.
> > >
> > > Okay, dropped these. We should backout 0800d9e6e216 to re-enable the
> > failing
> > > tests. Otherwise, no one would fix them.
> >
> > I get that, but I’d like to leave the tests skipped so no other problems
> > creep in.  They’ve been broken for several weeks now without getting much
> > attention, probably because something is often broken on Windows.
> >
> 
> I agree. I'll work on the tests anyway, and I don't want them hiding good
> signal on Windows.

Can't we blacklist them on the buildbot then?

#require is hard requirement, which make not possible to run the remotefilelog
tests on Windows.
Augie Fackler - Nov. 29, 2018, 6:02 p.m.
> On Nov 29, 2018, at 08:18, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> Can't we blacklist them on the buildbot then?
> 
> #require is hard requirement, which make not possible to run the remotefilelog
> tests on Windows.

I'm happy to patch out the #require on a per-test basis as I tackle making them work on Windows. We can switch to a blacklist on the buildbot if you feel strongly, but I'm happy to incur a little extra work on my end so the tests stay obviously-passing for people that aren't working on fixing remotefilelog tests on Windows.

Patch

diff --git a/hgext/remotefilelog/__init__.py b/hgext/remotefilelog/__init__.py
--- a/hgext/remotefilelog/__init__.py
+++ b/hgext/remotefilelog/__init__.py
@@ -257,6 +257,9 @@  def uisetup(ui):
     """Wraps user facing Mercurial commands to swap them out with shallow
     versions.
     """
+    if pycompat.iswindows:
+        raise error.Abort(_("remotefilelog is not supported on Windows"))
+
     hg.wirepeersetupfuncs.append(fileserverclient.peersetup)
 
     entry = extensions.wrapcommand(commands.table, 'clone', cloneshallow)