Patchwork D8477: packaging: support building WiX installers with PyOxidizer

login
register
mail settings
Submitter phabricator
Date April 22, 2020, 2:39 a.m.
Message ID <differential-rev-PHID-DREV-m7t4cpivqdjz72klxehe-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46206/
State Superseded
Headers show

Comments

phabricator - April 22, 2020, 2:39 a.m.
indygreg created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  We initially implemented PyOxidizer support for Inno installers.
  That did most of the heavy work of integrating PyOxidizer into
  the packaging system. Implementing WiX installer support was
  pretty straightforward.
  
  Aspects of this patch look very similar to Inno's.
  
  The main difference is the handling of the Visual C++
  Redistributable Runtime files.
  
  The WiX installer was formerly using merge modules to
  install the VC++ 9.0 runtime because this feature is
  supported by the WiX installer (it isn't easily available
  to Inno installers).
  
  Our strategy for the runtime files is to install the
  vcruntime140.dll file next to hg.exe just like any other
  file. While we could leverage WiX's functionality for invoking
  a VCRedist installer, I don't want to deal with the complexity
  at this juncture. So, we let run_pyoxidizer() copy vcruntime140.dll
  into the staging directory (like it does for Inno) and our
  dynamic WiX XML generator picks it up as a regular file and
  installs it.
  
  We did, however, have to teach mercurial.wxs how to conditionally
  use the merge modules. But this was rather straightforward.
  
  Comparing the file layout of the WiX installers before and
  after:
  
  - Various lib/*.{pyd, dll} files no longer exist
  - python27.dll was replaced by python37.dll
  - vcruntime140.dll was added
  
  All these changes are expected due to the transition to
  Python 3 and to PyOxidizer, which embeded the .pyd and .dll files
  in hg.exe.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

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

AFFECTED FILES
  contrib/packaging/hgpackaging/cli.py
  contrib/packaging/hgpackaging/wix.py
  contrib/packaging/wix/mercurial.wxs

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
Gregory Szorc - Nov. 18, 2020, 11:03 p.m.
On Wed, Nov 18, 2020 at 2:40 PM Augie Fackler <raf@durin42.com> wrote:

>
>
> > On Apr 30, 2020, at 01:53, indygreg (Gregory Szorc) <
> phabricator@mercurial-scm.org> wrote:
> >
> > REVISION DETAIL
> >  https://phab.mercurial-scm.org/D8477
> >
> > AFFECTED FILES
> >  contrib/packaging/hgpackaging/cli.py
> >  contrib/packaging/hgpackaging/wix.py
> >  contrib/packaging/wix/mercurial.wxs
> >
>
> [snip other diff regions]
>
> > diff --git a/contrib/packaging/hgpackaging/cli.py
> b/contrib/packaging/hgpackaging/cli.py
> > --- a/contrib/packaging/hgpackaging/cli.py
> > +++ b/contrib/packaging/hgpackaging/cli.py
> > @@ -50,6 +50,7 @@
> [snip misc hunks]
>
> >     if extra_packages_script:
> > +        if pyoxidizer_target:
> > +            raise Exception(
> > +                "pyoxidizer does not support --extra-packages-script"
> > +            )
>
> Is this a fundamental limitation, or is there just some code to write to
> make this? I need to inject our custom extension into the MSI we distribute
> to our users, and I figure this is probably roughly the way. I'm finally to
> the point that I appear to have the rest of the process working, so now I
> "just" need to wire up including the Google-custom bits.
>

The problem is that there's no easy way to pass conditional state into the
Starlark execution environment outside of changing the name of the target
you want to build and having different targets for different configurations.

One way to address this would be to expose environment variables as
Starlark variables. Or a `--var key=value` syntax to register custom
variables. These features may or may not have been requested in the
PyOxidizer issue tracker. Both are reasonable IMO. Would you mind
commenting on or filing a GitHub issue representing the feature you think
works best?
Augie Fackler - Nov. 19, 2020, 4:36 p.m.
(resending to keep the list, sorry for the double-send Greg!)

> On Nov 18, 2020, at 18:03, Gregory Szorc <gregory.szorc@gmail.com <mailto:gregory.szorc@gmail.com>> wrote:
> 
> On Wed, Nov 18, 2020 at 2:40 PM Augie Fackler <raf@durin42.com <mailto:raf@durin42.com>> wrote:
> 
> 
> > On Apr 30, 2020, at 01:53, indygreg (Gregory Szorc) <phabricator@mercurial-scm.org <mailto:phabricator@mercurial-scm.org>> wrote:
> > 
> > REVISION DETAIL
> >  https://phab.mercurial-scm.org/D8477 <https://phab.mercurial-scm.org/D8477>
> > 
> > AFFECTED FILES
> >  contrib/packaging/hgpackaging/cli.py
> >  contrib/packaging/hgpackaging/wix.py
> >  contrib/packaging/wix/mercurial.wxs
> > 
> 
> [snip other diff regions]
> 
> > diff --git a/contrib/packaging/hgpackaging/cli.py b/contrib/packaging/hgpackaging/cli.py
> > --- a/contrib/packaging/hgpackaging/cli.py
> > +++ b/contrib/packaging/hgpackaging/cli.py
> > @@ -50,6 +50,7 @@
> [snip misc hunks]
> 
> >     if extra_packages_script:
> > +        if pyoxidizer_target:
> > +            raise Exception(
> > +                "pyoxidizer does not support --extra-packages-script"
> > +            )
> 
> Is this a fundamental limitation, or is there just some code to write to make this? I need to inject our custom extension into the MSI we distribute to our users, and I figure this is probably roughly the way. I'm finally to the point that I appear to have the rest of the process working, so now I "just" need to wire up including the Google-custom bits.
> 
> The problem is that there's no easy way to pass conditional state into the Starlark execution environment outside of changing the name of the target you want to build and having different targets for different configurations.

(I haven't dug yet, so please forgive me if this is a bad idea) Would it work to have an "extra stuff" tree somewhere that PyOxidizer includes? That's pretty much what we do for the existing installer setup - that is, all we do is drop extra files in the right places and then WiX picks them up. Could we do something similar with PyOxidizer?

> One way to address this would be to expose environment variables as Starlark variables. Or a `--var key=value` syntax to register custom variables. These features may or may not have been requested in the PyOxidizer issue tracker. Both are reasonable IMO. Would you mind commenting on or filing a GitHub issue representing the feature you think works best?

I can't find anything obvious. I think my preference would be to add some sort of --var syntax so it's explicit rather than environmental. Do you have a preference?

(Searching issue trackers is hard!)
Augie Fackler - Nov. 19, 2020, 5:29 p.m.
Okay, looking some more, I have an idea. Please shoot it down if it's bad!

Basically I think I can (for now) maintain a local patch that adds a call to `PythonExecutable.read_package_root()` and gives it our "google" extension in the packages argument. Does that sound about right?

(I'll probably poke at this after lunch, but any feedback on the merits of this hackery welcomed.)

> On Nov 19, 2020, at 11:36, Augie Fackler <raf@durin42.com> wrote:
> 
> (resending to keep the list, sorry for the double-send Greg!)
> 
>> On Nov 18, 2020, at 18:03, Gregory Szorc <gregory.szorc@gmail.com <mailto:gregory.szorc@gmail.com>> wrote:
>> 
>> On Wed, Nov 18, 2020 at 2:40 PM Augie Fackler <raf@durin42.com <mailto:raf@durin42.com>> wrote:
>> 
>> 
>> > On Apr 30, 2020, at 01:53, indygreg (Gregory Szorc) <phabricator@mercurial-scm.org <mailto:phabricator@mercurial-scm.org>> wrote:
>> > 
>> > REVISION DETAIL
>> >  https://phab.mercurial-scm.org/D8477 <https://phab.mercurial-scm.org/D8477>
>> > 
>> > AFFECTED FILES
>> >  contrib/packaging/hgpackaging/cli.py
>> >  contrib/packaging/hgpackaging/wix.py
>> >  contrib/packaging/wix/mercurial.wxs
>> > 
>> 
>> [snip other diff regions]
>> 
>> > diff --git a/contrib/packaging/hgpackaging/cli.py b/contrib/packaging/hgpackaging/cli.py
>> > --- a/contrib/packaging/hgpackaging/cli.py
>> > +++ b/contrib/packaging/hgpackaging/cli.py
>> > @@ -50,6 +50,7 @@
>> [snip misc hunks]
>> 
>> >     if extra_packages_script:
>> > +        if pyoxidizer_target:
>> > +            raise Exception(
>> > +                "pyoxidizer does not support --extra-packages-script"
>> > +            )
>> 
>> Is this a fundamental limitation, or is there just some code to write to make this? I need to inject our custom extension into the MSI we distribute to our users, and I figure this is probably roughly the way. I'm finally to the point that I appear to have the rest of the process working, so now I "just" need to wire up including the Google-custom bits.
>> 
>> The problem is that there's no easy way to pass conditional state into the Starlark execution environment outside of changing the name of the target you want to build and having different targets for different configurations.
> 
> (I haven't dug yet, so please forgive me if this is a bad idea) Would it work to have an "extra stuff" tree somewhere that PyOxidizer includes? That's pretty much what we do for the existing installer setup - that is, all we do is drop extra files in the right places and then WiX picks them up. Could we do something similar with PyOxidizer?
> 
>> One way to address this would be to expose environment variables as Starlark variables. Or a `--var key=value` syntax to register custom variables. These features may or may not have been requested in the PyOxidizer issue tracker. Both are reasonable IMO. Would you mind commenting on or filing a GitHub issue representing the feature you think works best?
> 
> I can't find anything obvious. I think my preference would be to add some sort of --var syntax so it's explicit rather than environmental. Do you have a preference?
> 
> (Searching issue trackers is hard!)
Augie Fackler - Nov. 19, 2020, 7:59 p.m.
Last post for a while, I'm going to run with this a bit:

I'm going to try putting module_search_path = ["$ORIGIN/extra_packages"] or similar, and then locally patch the scripts to drop extra stuff there. Will report back in a few days, perhaps after the US Thanksgiving holiday.

AF

> On Nov 19, 2020, at 12:29, Augie Fackler <raf@durin42.com> wrote:
> 
> Okay, looking some more, I have an idea. Please shoot it down if it's bad!
> 
> Basically I think I can (for now) maintain a local patch that adds a call to `PythonExecutable.read_package_root()` and gives it our "google" extension in the packages argument. Does that sound about right?
> 
> (I'll probably poke at this after lunch, but any feedback on the merits of this hackery welcomed.)
> 
>> On Nov 19, 2020, at 11:36, Augie Fackler <raf@durin42.com <mailto:raf@durin42.com>> wrote:
>> 
>> (resending to keep the list, sorry for the double-send Greg!)
>> 
>>> On Nov 18, 2020, at 18:03, Gregory Szorc <gregory.szorc@gmail.com <mailto:gregory.szorc@gmail.com>> wrote:
>>> 
>>> On Wed, Nov 18, 2020 at 2:40 PM Augie Fackler <raf@durin42.com <mailto:raf@durin42.com>> wrote:
>>> 
>>> 
>>> > On Apr 30, 2020, at 01:53, indygreg (Gregory Szorc) <phabricator@mercurial-scm.org <mailto:phabricator@mercurial-scm.org>> wrote:
>>> > 
>>> > REVISION DETAIL
>>> >  https://phab.mercurial-scm.org/D8477 <https://phab.mercurial-scm.org/D8477>
>>> > 
>>> > AFFECTED FILES
>>> >  contrib/packaging/hgpackaging/cli.py
>>> >  contrib/packaging/hgpackaging/wix.py
>>> >  contrib/packaging/wix/mercurial.wxs
>>> > 
>>> 
>>> [snip other diff regions]
>>> 
>>> > diff --git a/contrib/packaging/hgpackaging/cli.py b/contrib/packaging/hgpackaging/cli.py
>>> > --- a/contrib/packaging/hgpackaging/cli.py
>>> > +++ b/contrib/packaging/hgpackaging/cli.py
>>> > @@ -50,6 +50,7 @@
>>> [snip misc hunks]
>>> 
>>> >     if extra_packages_script:
>>> > +        if pyoxidizer_target:
>>> > +            raise Exception(
>>> > +                "pyoxidizer does not support --extra-packages-script"
>>> > +            )
>>> 
>>> Is this a fundamental limitation, or is there just some code to write to make this? I need to inject our custom extension into the MSI we distribute to our users, and I figure this is probably roughly the way. I'm finally to the point that I appear to have the rest of the process working, so now I "just" need to wire up including the Google-custom bits.
>>> 
>>> The problem is that there's no easy way to pass conditional state into the Starlark execution environment outside of changing the name of the target you want to build and having different targets for different configurations.
>> 
>> (I haven't dug yet, so please forgive me if this is a bad idea) Would it work to have an "extra stuff" tree somewhere that PyOxidizer includes? That's pretty much what we do for the existing installer setup - that is, all we do is drop extra files in the right places and then WiX picks them up. Could we do something similar with PyOxidizer?
>> 
>>> One way to address this would be to expose environment variables as Starlark variables. Or a `--var key=value` syntax to register custom variables. These features may or may not have been requested in the PyOxidizer issue tracker. Both are reasonable IMO. Would you mind commenting on or filing a GitHub issue representing the feature you think works best?
>> 
>> I can't find anything obvious. I think my preference would be to add some sort of --var syntax so it's explicit rather than environmental. Do you have a preference?
>> 
>> (Searching issue trackers is hard!)
>

Patch

diff --git a/contrib/packaging/wix/mercurial.wxs b/contrib/packaging/wix/mercurial.wxs
--- a/contrib/packaging/wix/mercurial.wxs
+++ b/contrib/packaging/wix/mercurial.wxs
@@ -79,16 +79,21 @@ 
         </Directory>
       </Directory>
 
-      <?if $(var.Platform) = "x86" ?>
-        <Merge Id='VCRuntime' DiskId='1' Language='1033'
-              SourceFile='$(var.VCRedistSrcDir)\microsoft.vcxx.crt.x86_msm.msm' />
-        <Merge Id='VCRuntimePolicy' DiskId='1' Language='1033'
-              SourceFile='$(var.VCRedistSrcDir)\policy.x.xx.microsoft.vcxx.crt.x86_msm.msm' />
-      <?else?>
-        <Merge Id='VCRuntime' DiskId='1' Language='1033'
-              SourceFile='$(var.VCRedistSrcDir)\microsoft.vcxx.crt.x64_msm.msm' />
-        <Merge Id='VCRuntimePolicy' DiskId='1' Language='1033'
-              SourceFile='$(var.VCRedistSrcDir)\policy.x.xx.microsoft.vcxx.crt.x64_msm.msm' />
+      <!-- Install VCRedist merge modules on Python 2. On Python 3,
+           vcruntimeXXX.dll is part of the install layout and gets picked up
+           as a regular file. -->
+      <?if $(var.PythonVersion) = "2" ?>
+        <?if $(var.Platform) = "x86" ?>
+          <Merge Id='VCRuntime' DiskId='1' Language='1033'
+                SourceFile='$(var.VCRedistSrcDir)\microsoft.vcxx.crt.x86_msm.msm' />
+          <Merge Id='VCRuntimePolicy' DiskId='1' Language='1033'
+                SourceFile='$(var.VCRedistSrcDir)\policy.x.xx.microsoft.vcxx.crt.x86_msm.msm' />
+        <?else?>
+          <Merge Id='VCRuntime' DiskId='1' Language='1033'
+                SourceFile='$(var.VCRedistSrcDir)\microsoft.vcxx.crt.x64_msm.msm' />
+          <Merge Id='VCRuntimePolicy' DiskId='1' Language='1033'
+                SourceFile='$(var.VCRedistSrcDir)\policy.x.xx.microsoft.vcxx.crt.x64_msm.msm' />
+        <?endif?>
       <?endif?>
     </Directory>
 
@@ -101,10 +106,14 @@ 
         <ComponentGroupRef Id="hg.group.ROOT" />
         <ComponentGroupRef Id="hg.group.defaultrc" />
         <ComponentGroupRef Id="hg.group.helptext" />
-        <ComponentGroupRef Id="hg.group.lib" />
+        <?ifdef MercurialHasLib?>
+          <ComponentGroupRef Id="hg.group.lib" />
+        <?endif?>
         <ComponentGroupRef Id="hg.group.templates" />
-        <MergeRef Id='VCRuntime' />
-        <MergeRef Id='VCRuntimePolicy' />
+        <?if $(var.PythonVersion) = "2" ?>
+          <MergeRef Id='VCRuntime' />
+          <MergeRef Id='VCRuntimePolicy' />
+        <?endif?>
       </Feature>
       <?ifdef MercurialExtraFeatures?>
         <?foreach EXTRAFEAT in $(var.MercurialExtraFeatures)?>
diff --git a/contrib/packaging/hgpackaging/wix.py b/contrib/packaging/hgpackaging/wix.py
--- a/contrib/packaging/hgpackaging/wix.py
+++ b/contrib/packaging/hgpackaging/wix.py
@@ -22,6 +22,7 @@ 
     build_py2exe,
     stage_install,
 )
+from .pyoxidizer import run_pyoxidizer
 from .util import (
     extract_zip_to_directory,
     normalize_windows_version,
@@ -284,7 +285,7 @@ 
     return doc.toprettyxml()
 
 
-def build_installer(
+def build_installer_py2exe(
     source_dir: pathlib.Path,
     python_exe: pathlib.Path,
     msi_name='mercurial',
@@ -294,7 +295,7 @@ 
     extra_features: typing.Optional[typing.List[str]] = None,
     signing_info: typing.Optional[typing.Dict[str, str]] = None,
 ):
-    """Build a WiX MSI installer.
+    """Build a WiX MSI installer using py2exe.
 
     ``source_dir`` is the path to the Mercurial source tree to use.
     ``arch`` is the target architecture. either ``x86`` or ``x64``.
@@ -355,6 +356,50 @@ 
         staging_dir,
         arch,
         version=version,
+        python2=True,
+        msi_name=msi_name,
+        extra_wxs=extra_wxs,
+        extra_features=extra_features,
+        signing_info=signing_info,
+    )
+
+
+def build_installer_pyoxidizer(
+    source_dir: pathlib.Path,
+    target_triple: str,
+    msi_name='mercurial',
+    version=None,
+    extra_wxs: typing.Optional[typing.Dict[str, str]] = None,
+    extra_features: typing.Optional[typing.List[str]] = None,
+    signing_info: typing.Optional[typing.Dict[str, str]] = None,
+):
+    """Build a WiX MSI installer using PyOxidizer."""
+    hg_build_dir = source_dir / "build"
+    build_dir = hg_build_dir / ("wix-%s" % target_triple)
+    staging_dir = build_dir / "stage"
+
+    arch = "x64" if "x86_64" in target_triple else "x86"
+
+    build_dir.mkdir(parents=True, exist_ok=True)
+    run_pyoxidizer(source_dir, staging_dir, target_triple)
+
+    # We also install some extra files.
+    process_install_rules(EXTRA_INSTALL_RULES, source_dir, staging_dir)
+
+    # And remove some files we don't want.
+    for f in STAGING_REMOVE_FILES:
+        p = staging_dir / f
+        if p.exists():
+            print('removing %s' % p)
+            p.unlink()
+
+    return run_wix_packaging(
+        source_dir,
+        build_dir,
+        staging_dir,
+        arch,
+        version,
+        python2=False,
         msi_name=msi_name,
         extra_wxs=extra_wxs,
         extra_features=extra_features,
@@ -368,6 +413,7 @@ 
     staging_dir: pathlib.Path,
     arch: str,
     version: str,
+    python2: bool,
     msi_name: typing.Optional[str] = "mercurial",
     extra_wxs: typing.Optional[typing.Dict[str, str]] = None,
     extra_features: typing.Optional[typing.List[str]] = None,
@@ -406,7 +452,8 @@ 
     if not wix_path.exists():
         extract_zip_to_directory(wix_pkg, wix_path)
 
-    ensure_vc90_merge_modules(build_dir)
+    if python2:
+        ensure_vc90_merge_modules(build_dir)
 
     source_build_rel = pathlib.Path(os.path.relpath(source_dir, build_dir))
 
@@ -425,7 +472,16 @@ 
     source = wix_dir / 'mercurial.wxs'
     defines['Version'] = version
     defines['Comments'] = 'Installs Mercurial version %s' % version
-    defines['VCRedistSrcDir'] = str(build_dir)
+
+    if python2:
+        defines["PythonVersion"] = "2"
+        defines['VCRedistSrcDir'] = str(build_dir)
+    else:
+        defines["PythonVersion"] = "3"
+
+    if (staging_dir / "lib").exists():
+        defines["MercurialHasLib"] = "1"
+
     if extra_features:
         assert all(';' not in f for f in extra_features)
         defines['MercurialExtraFeatures'] = ';'.join(extra_features)
diff --git a/contrib/packaging/hgpackaging/cli.py b/contrib/packaging/hgpackaging/cli.py
--- a/contrib/packaging/hgpackaging/cli.py
+++ b/contrib/packaging/hgpackaging/cli.py
@@ -50,6 +50,7 @@ 
 
 def build_wix(
     name=None,
+    pyoxidizer_target=None,
     python=None,
     version=None,
     sign_sn=None,
@@ -60,16 +61,29 @@ 
     extra_wxs=None,
     extra_features=None,
 ):
+    if not pyoxidizer_target and not python:
+        raise Exception("--python required unless building with PyOxidizer")
+
+    if python and not os.path.isabs(python):
+        raise Exception("--python arg must be an absolute path")
+
     kwargs = {
         "source_dir": SOURCE_DIR,
-        "python_exe": pathlib.Path(python),
         "version": version,
     }
 
-    if not os.path.isabs(python):
-        raise Exception("--python arg must be an absolute path")
+    if pyoxidizer_target:
+        fn = wix.build_installer_pyoxidizer
+        kwargs["target_triple"] = pyoxidizer_target
+    else:
+        fn = wix.build_installer_py2exe
+        kwargs["python_exe"] = pathlib.Path(python)
 
     if extra_packages_script:
+        if pyoxidizer_target:
+            raise Exception(
+                "pyoxidizer does not support --extra-packages-script"
+            )
         kwargs["extra_packages_script"] = extra_packages_script
     if extra_wxs:
         kwargs["extra_wxs"] = dict(
@@ -87,7 +101,7 @@ 
             "timestamp_url": sign_timestamp_url,
         }
 
-    wix.build_installer(**kwargs)
+    fn(**kwargs)
 
 
 def get_parser():
@@ -115,8 +129,11 @@ 
     )
     sp.add_argument("--name", help="Application name", default="Mercurial")
     sp.add_argument(
-        "--python", help="Path to Python executable to use", required=True
+        "--pyoxidizer-target",
+        choices={"i686-pc-windows-msvc", "x86_64-pc-windows-msvc"},
+        help="Build with PyOxidizer targeting this host triple",
     )
+    sp.add_argument("--python", help="Path to Python executable to use")
     sp.add_argument(
         "--sign-sn",
         help="Subject name (or fragment thereof) of certificate "