Patchwork [windows] windows: add an experimental option for long paths support

login
register
mail settings
Submitter Kostia Balytskyi
Date Oct. 9, 2017, 9:38 a.m.
Message ID <aa5c7de6a86ba8bbc7d4.1507541884@devvm1416.lla2.facebook.com>
Download mbox | patch
Permalink /patch/24655/
State Accepted
Headers show

Comments

Kostia Balytskyi - Oct. 9, 2017, 9:38 a.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1507541423 25200
#      Mon Oct 09 02:30:23 2017 -0700
# Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
# Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
windows: add an experimental option for long paths support

This commit adds an experimental --long-paths-support flag to build_hgexe
on Windows. It is off by default, but when supplied, causes setup.py to
embed some XML into the generated hg.exe, which in turn tells Windows to
allow this exe to use long paths (given that the appropriate registry setting
is enabled as well).
This was tested on Windows 10 14393 and 15063.

This commit introduces a badly-named initialize_options function, but its name
is dictated by distutils, rather than chosen.
Augie Fackler - Oct. 10, 2017, 12:54 a.m.
On Mon, Oct 09, 2017 at 02:38:04AM -0700, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1507541423 25200
> #      Mon Oct 09 02:30:23 2017 -0700
> # Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
> # Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
> windows: add an experimental option for long paths support

queued, thanks

Let's make sure this gets prominently noted in the release notes for
4.4, and if nobody notices issues with it we'll just make it default
for 4.5 or 4.6.
Augie Fackler - Oct. 10, 2017, 1:06 a.m.
On Mon, Oct 09, 2017 at 08:54:54PM -0400, Augie Fackler wrote:
> On Mon, Oct 09, 2017 at 02:38:04AM -0700, Kostia Balytskyi wrote:
> > # HG changeset patch
> > # User Kostia Balytskyi <ikostia@fb.com>
> > # Date 1507541423 25200
> > #      Mon Oct 09 02:30:23 2017 -0700
> > # Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
> > # Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
> > windows: add an experimental option for long paths support
>
> queued, thanks
>
> Let's make sure this gets prominently noted in the release notes for
> 4.4, and if nobody notices issues with it we'll just make it default
> for 4.5 or 4.6

BTW, you can put "# no-check-commit" in your commit message and then
test-check-commit.t won't have a sad. :)

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Matt Harbison - Oct. 10, 2017, 2:38 a.m.
On Mon, 09 Oct 2017 05:38:04 -0400, Kostia Balytskyi <ikostia@fb.com>  
wrote:

> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1507541423 25200
> #      Mon Oct 09 02:30:23 2017 -0700
> # Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
> # Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
> windows: add an experimental option for long paths support
>
> +
> +    def addlongpathsmanifest(self):
> +        """Add manifest pieces so that hg.exe understands long paths
> +
> +        This is an EXPERIMENTAL feature, use with care.
> +        To enable long paths support, one needs to do two things:
> +        - build Mercurial with --long-paths-support option
> +        - change HKLM\SYSTEM\CurrentControlSet\Control\FileSystem\
> +                 LongPathsEnabled to have value 1.

The way I read the documentation[1], it seems like either one alone is  
sufficient.  Did you find different in testing?

[1] https://msdn.microsoft.com/en-us/library/aa365247(v=vs.85).aspx#maxpath
Matt Harbison - Oct. 10, 2017, 2:56 a.m.
On Mon, 09 Oct 2017 20:54:54 -0400, Augie Fackler <raf@durin42.com> wrote:

> On Mon, Oct 09, 2017 at 02:38:04AM -0700, Kostia Balytskyi wrote:
>> # HG changeset patch
>> # User Kostia Balytskyi <ikostia@fb.com>
>> # Date 1507541423 25200
>> #      Mon Oct 09 02:30:23 2017 -0700
>> # Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
>> # Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
>> windows: add an experimental option for long paths support
>
> queued, thanks
>
> Let's make sure this gets prominently noted in the release notes for
> 4.4, and if nobody notices issues with it we'll just make it default
> for 4.5 or 4.6.

I assume FB rolls their own installers, but any idea how we (optionally)  
get this into the hands of other users?  I assume most Windows users  
aren't running from source, in order to build the wrapper.  Is this  
something that a pip install would be able to handle?

Even though I just questioned why both this and the registry setting are  
required, it may be a good idea to reference the registry setting in the  
release notes too, so the external diff tools don't puke.
Augie Fackler - Oct. 10, 2017, 6:53 p.m.
> On Oct 9, 2017, at 22:56, Matt Harbison <mharbison72@gmail.com> wrote:
> 
> On Mon, 09 Oct 2017 20:54:54 -0400, Augie Fackler <raf@durin42.com> wrote:
> 
>> On Mon, Oct 09, 2017 at 02:38:04AM -0700, Kostia Balytskyi wrote:
>>> # HG changeset patch
>>> # User Kostia Balytskyi <ikostia@fb.com>
>>> # Date 1507541423 25200
>>> #      Mon Oct 09 02:30:23 2017 -0700
>>> # Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
>>> # Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
>>> windows: add an experimental option for long paths support
>> 
>> queued, thanks
>> 
>> Let's make sure this gets prominently noted in the release notes for
>> 4.4, and if nobody notices issues with it we'll just make it default
>> for 4.5 or 4.6.
> 
> I assume FB rolls their own installers, but any idea how we (optionally) get this into the hands of other users?  I assume most Windows users aren't running from source, in order to build the wrapper.  Is this something that a pip install would be able to handle?
> 
> Even though I just questioned why both this and the registry setting are required, it may be a good idea to reference the registry setting in the release notes too, so the external diff tools don't puke.

Probably the best thing would be to have a custom msi that we could point people to. The pip-installable wheel probably won't have this set (I don't know if it even could.)
Pulkit Goyal - Feb. 28, 2019, 1 p.m.
On Tue, Oct 10, 2017 at 5:39 AM Matt Harbison <mharbison72@gmail.com> wrote:

> On Mon, 09 Oct 2017 05:38:04 -0400, Kostia Balytskyi <ikostia@fb.com>
> wrote:
>
> > # HG changeset patch
> > # User Kostia Balytskyi <ikostia@fb.com>
> > # Date 1507541423 25200
> > #      Mon Oct 09 02:30:23 2017 -0700
> > # Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
> > # Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
> > windows: add an experimental option for long paths support
> >
> > +
> > +    def addlongpathsmanifest(self):
> > +        """Add manifest pieces so that hg.exe understands long paths
> > +
> > +        This is an EXPERIMENTAL feature, use with care.
> > +        To enable long paths support, one needs to do two things:
> > +        - build Mercurial with --long-paths-support option
> > +        - change HKLM\SYSTEM\CurrentControlSet\Control\FileSystem\
> > +                 LongPathsEnabled to have value 1.
>
> The way I read the documentation[1], it seems like either one alone is
> sufficient.  Did you find different in testing?
>
> [1]
> https://msdn.microsoft.com/en-us/library/aa365247(v=vs.85).aspx#maxpath


I stumbled upon the same thing today. Then I read the 9th comment on
https://social.msdn.microsoft.com/Forums/en-US/fc85630e-5684-4df6-ad2f-5a128de3deef/260-character-explorer-path-length-limit?forum=windowsgeneraldevelopmentissues
which says you need to have the support enabled and also add the manifest
to hg.exe.

Matt any chance you checked whether we need the manifest or just enabling
the long path support on windows will work.
Matt Harbison - Feb. 28, 2019, 2:58 p.m.
> On Feb 28, 2019, at 8:00 AM, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> 
> 
> 
>> On Tue, Oct 10, 2017 at 5:39 AM Matt Harbison <mharbison72@gmail.com> wrote:
>> On Mon, 09 Oct 2017 05:38:04 -0400, Kostia Balytskyi <ikostia@fb.com>  
>> wrote:
>> 
>> > # HG changeset patch
>> > # User Kostia Balytskyi <ikostia@fb.com>
>> > # Date 1507541423 25200
>> > #      Mon Oct 09 02:30:23 2017 -0700
>> > # Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
>> > # Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
>> > windows: add an experimental option for long paths support
>> >
>> > +
>> > +    def addlongpathsmanifest(self):
>> > +        """Add manifest pieces so that hg.exe understands long paths
>> > +
>> > +        This is an EXPERIMENTAL feature, use with care.
>> > +        To enable long paths support, one needs to do two things:
>> > +        - build Mercurial with --long-paths-support option
>> > +        - change HKLM\SYSTEM\CurrentControlSet\Control\FileSystem\
>> > +                 LongPathsEnabled to have value 1.
>> 
>> The way I read the documentation[1], it seems like either one alone is  
>> sufficient.  Did you find different in testing?
>> 
>> [1] https://msdn.microsoft.com/en-us/library/aa365247(v=vs.85).aspx#maxpath
> 
> I stumbled upon the same thing today. Then I read the 9th comment on https://social.msdn.microsoft.com/Forums/en-US/fc85630e-5684-4df6-ad2f-5a128de3deef/260-character-explorer-path-length-limit?forum=windowsgeneraldevelopmentissues which says you need to have the support enabled and also add the manifest to hg.exe.
> 
> Matt any chance you checked whether we need the manifest or just enabling the long path support on windows will work.

I played with it a little bit this morning.  I didn’t enable the policy setting, but it looks like the manifest entry alone doesn’t work.  I used py3 to test (which does have the manifest entry).

I’m not sure if they did anything to support it in py3, but the entry is missing from py2.

Given that is doesn’t work with the manifest alone, I wonder if we should just add it to the manifest unconditionally. (It’s behind an extra build argument right now.)
Pulkit Goyal - March 4, 2019, 10:32 p.m.
On Thu, Feb 28, 2019 at 8:28 PM Matt Harbison <mharbison72@gmail.com> wrote:

>
> On Feb 28, 2019, at 8:00 AM, Pulkit Goyal <7895pulkit@gmail.com> wrote:
>
>
>
> On Tue, Oct 10, 2017 at 5:39 AM Matt Harbison <mharbison72@gmail.com>
> wrote:
>
>> On Mon, 09 Oct 2017 05:38:04 -0400, Kostia Balytskyi <ikostia@fb.com>
>> wrote:
>>
>> > # HG changeset patch
>> > # User Kostia Balytskyi <ikostia@fb.com>
>> > # Date 1507541423 25200
>> > #      Mon Oct 09 02:30:23 2017 -0700
>> > # Node ID aa5c7de6a86ba8bbc7d46bb414ac7ee6fbde3a4a
>> > # Parent  a57c938e7ac8f391a62de6c7c4d5cf0e81b2dcf4
>> > windows: add an experimental option for long paths support
>> >
>> > +
>> > +    def addlongpathsmanifest(self):
>> > +        """Add manifest pieces so that hg.exe understands long paths
>> > +
>> > +        This is an EXPERIMENTAL feature, use with care.
>> > +        To enable long paths support, one needs to do two things:
>> > +        - build Mercurial with --long-paths-support option
>> > +        - change HKLM\SYSTEM\CurrentControlSet\Control\FileSystem\
>> > +                 LongPathsEnabled to have value 1.
>>
>> The way I read the documentation[1], it seems like either one alone is
>> sufficient.  Did you find different in testing?
>>
>> [1]
>> https://msdn.microsoft.com/en-us/library/aa365247(v=vs.85).aspx#maxpath
>
>
> I stumbled upon the same thing today. Then I read the 9th comment on
> https://social.msdn.microsoft.com/Forums/en-US/fc85630e-5684-4df6-ad2f-5a128de3deef/260-character-explorer-path-length-limit?forum=windowsgeneraldevelopmentissues
> which says you need to have the support enabled and also add the manifest
> to hg.exe.
>
> Matt any chance you checked whether we need the manifest or just enabling
> the long path support on windows will work.
>
>
> I played with it a little bit this morning.  I didn’t enable the policy
> setting, but it looks like the manifest entry alone doesn’t work.  I used
> py3 to test (which does have the manifest entry).
>
> I’m not sure if they did anything to support it in py3, but the entry is
> missing from py2.
>

I tested it today on python 2. First I enabled the setting from
ComputerPolicy/ComputerCofiguration/.... . Only enabling this option did
not work.

Then I manually added the manifest to hg.exe using mt.exe and it worked.

>
> Given that is doesn’t work with the manifest alone, I wonder if we should
> just add it to the manifest unconditionally. (It’s behind an extra build
> argument right now.)
>

I don't have an opinion here. I am too new to windows to say anything about
that.

Patch

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -516,6 +516,26 @@  class buildhgextindex(Command):
 
 class buildhgexe(build_ext):
     description = 'compile hg.exe from mercurial/exewrapper.c'
+    user_options = build_ext.user_options + [
+        ('long-paths-support', None, 'enable support for long paths on '
+                                     'Windows (off by default and '
+                                     'experimental)'),
+    ]
+
+    LONG_PATHS_MANIFEST = """
+    <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+    <assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+        <application>
+            <windowsSettings
+            xmlns:ws2="http://schemas.microsoft.com/SMI/2016/WindowsSettings">
+                <ws2:longPathAware>true</ws2:longPathAware>
+            </windowsSettings>
+        </application>
+    </assembly>"""
+
+    def initialize_options(self):
+        build_ext.initialize_options(self)
+        self.long_paths_support = False
 
     def build_extensions(self):
         if os.name != 'nt':
@@ -557,10 +577,45 @@  class buildhgexe(build_ext):
         objects = self.compiler.compile(['mercurial/exewrapper.c'],
                                          output_dir=self.build_temp)
         dir = os.path.dirname(self.get_ext_fullpath('dummy'))
-        target = os.path.join(dir, 'hg')
-        self.compiler.link_executable(objects, target,
+        self.hgtarget = os.path.join(dir, 'hg')
+        self.compiler.link_executable(objects, self.hgtarget,
                                       libraries=[],
                                       output_dir=self.build_temp)
+        if self.long_paths_support:
+            self.addlongpathsmanifest()
+
+    def addlongpathsmanifest(self):
+        """Add manifest pieces so that hg.exe understands long paths
+
+        This is an EXPERIMENTAL feature, use with care.
+        To enable long paths support, one needs to do two things:
+        - build Mercurial with --long-paths-support option
+        - change HKLM\SYSTEM\CurrentControlSet\Control\FileSystem\
+                 LongPathsEnabled to have value 1.
+
+        Please ignore 'warning 81010002: Unrecognized Element "longPathAware"';
+        it happens because Mercurial uses mt.exe circa 2008, which is not
+        yet aware of long paths support in the manifest (I think so at least).
+        This does not stop mt.exe from embedding/merging the XML properly.
+
+        Why resource #1 should be used for .exe manifests? I don't know and
+        wasn't able to find an explanation for mortals. But it seems to work.
+        """
+        exefname = self.compiler.executable_filename(self.hgtarget)
+        fdauto, manfname = tempfile.mkstemp(suffix='.hg.exe.manifest')
+        os.close(fdauto)
+        with open(manfname, 'w') as f:
+            f.write(self.LONG_PATHS_MANIFEST)
+        log.info("long paths manifest is written to '%s'" % manfname)
+        inputresource = '-inputresource:%s;#1' % exefname
+        outputresource = '-outputresource:%s;#1' % exefname
+        log.info("running mt.exe to update hg.exe's manifest in-place")
+        # supplying both -manifest and -inputresource to mt.exe makes
+        # it merge the embedded and supplied manifests in the -outputresource
+        self.spawn(['mt.exe', '-nologo', '-manifest', manfname,
+                    inputresource, outputresource])
+        log.info("done updating hg.exe's manifest")
+        os.remove(manfname)
 
     @property
     def hgexepath(self):