Patchwork D6164: wix: add a hook for a prebuild script to inject extra libraries

login
register
mail settings
Submitter phabricator
Date March 21, 2019, 12:55 p.m.
Message ID <differential-rev-PHID-DREV-gxfjdxiqb5syp7fnbqyb-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39350/
State Superseded
Headers show

Comments

phabricator - March 21, 2019, 12:55 p.m.
durin42 created this revision.
durin42 added a reviewer: indygreg.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I need this to build packages for Google so we can bundle some
  extensions in the installed image. My assumption is that this is most
  interesting for the .msi images so I only wired it up there. I'm not
  thrilled with the interface this provides, but it was an easy way to
  retain debug messages on Windows while also having enough structure to
  know what lines are actually module names for py2exe.
  
  Still pending on my end: I need to bundle a couple of config files,
  and at least one data file. I'm open to advice on how to do those
  things, and how to do this better.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  contrib/packaging/hgpackaging/py2exe.py
  contrib/packaging/hgpackaging/wix.py
  contrib/packaging/wix/build.py

CHANGE DETAILS




To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 23, 2019, 3:04 p.m.
indygreg added a comment.


  I like the flexibility. But I'm not super keen about the interface here. Using a script to inject custom options seems like it could be useful. But as it is currently implemented, the script simply prints out `\0` delimited package names. So one UI wart is `--extra-prebuild-script` being a somewhat generic name but that script only emits package names. `--extra-packages-script` would be a better name.
  
  What do you think about defining an `--extra-package` CLI argument that takes `nargs=*` or an `--extra-packages` that takes a comma-delimited list and then thread that through to the existing `extra_packages` keyword argument? That seems a bit simpler and easier to extend than scripts.

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 25, 2019, 4:09 p.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D6164#89881, @indygreg wrote:
  
  > I like the flexibility. But I'm not super keen about the interface here. Using a script to inject custom options seems like it could be useful. But as it is currently implemented, the script simply prints out `\0` delimited package names. So one UI wart is `--extra-prebuild-script` being a somewhat generic name but that script only emits package names. `--extra-packages-script` would be a better name.
  
  
  The script also has to install packages into the virtualenv so they're findable.
  
  > What do you think about defining an `--extra-package` CLI argument that takes `nargs=*` or an `--extra-packages` that takes a comma-delimited list and then thread that through to the existing `extra_packages` keyword argument? That seems a bit simpler and easier to extend than scripts.
  
  I'm open to having both an --extra-package *and* a script argument, if that's what you're proposing. Does that sound workable?

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 25, 2019, 4:25 p.m.
indygreg added a comment.


  In https://phab.mercurial-scm.org/D6164#89913, @durin42 wrote:
  
  >
  
  
  How about we add an argument to define the path(s) to the pip requirements file(s) to use? By default, it can use the `requirements.txt` in the repo. Would that solve your use case?

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 25, 2019, 4:37 p.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D6164#89914, @indygreg wrote:
  
  > In https://phab.mercurial-scm.org/D6164#89913, @durin42 wrote:
  >
  > >
  >
  >
  > How about we add an argument to define the path(s) to the pip requirements file(s) to use? By default, it can use the `requirements.txt` in the repo. Would that solve your use case?
  
  
  No, because I have some stuff which (out of tragic necessity at the moment) isn't `pip` installable, and I have to install it into the virtualenv by banging some rocks together. :(

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 25, 2019, 5:14 p.m.
indygreg added a comment.


  In https://phab.mercurial-scm.org/D6164#89915, @durin42 wrote:
  
  > In https://phab.mercurial-scm.org/D6164#89914, @indygreg wrote:
  >
  > > In https://phab.mercurial-scm.org/D6164#89913, @durin42 wrote:
  > >
  > > >
  > >
  > >
  > > How about we add an argument to define the path(s) to the pip requirements file(s) to use? By default, it can use the `requirements.txt` in the repo. Would that solve your use case?
  >
  >
  > No, because I have some stuff which (out of tragic necessity at the moment) isn't `pip` installable, and I have to install it into the virtualenv by banging some rocks together. :(
  
  
  Bleh. So you do need the power of a full script here.
  
  I'll queue with the naming change to "virtualenv populate script" or something along those lines.

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 25, 2019, 6:58 p.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D6164#89916, @indygreg wrote:
  
  > In https://phab.mercurial-scm.org/D6164#89915, @durin42 wrote:
  >
  > > In https://phab.mercurial-scm.org/D6164#89914, @indygreg wrote:
  > >
  > > > In https://phab.mercurial-scm.org/D6164#89913, @durin42 wrote:
  > > >
  > > > >
  > > >
  > > >
  > > > How about we add an argument to define the path(s) to the pip requirements file(s) to use? By default, it can use the `requirements.txt` in the repo. Would that solve your use case?
  > >
  > >
  > > No, because I have some stuff which (out of tragic necessity at the moment) isn't `pip` installable, and I have to install it into the virtualenv by banging some rocks together. :(
  >
  >
  > Bleh. So you do need the power of a full script here.
  
  
  Yep. :(
  
  > I'll queue with the naming change to "virtualenv populate script" or something along those lines.
  
  Thanks.

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - April 2, 2019, 5:41 p.m.
indygreg requested changes to this revision.
indygreg added a comment.
This revision now requires changes to proceed.


  Was a newer version of this patch with the requested changes going to be uploaded? It's weird to see additional patches in the series without this one updated...

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - April 2, 2019, 5:59 p.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D6164#90059, @indygreg wrote:
  
  > Was a newer version of this patch with the requested changes going to be uploaded? It's weird to see additional patches in the series without this one updated...
  
  
  You had indicated up-thread you were going to queue it, possibly with some in-flight tweaks. I had to rebase it as I was working on follow-ups, but I was merely waiting for you to land the patch as you'd indicated.
  
  Are you now saying you want me to fix things for you? That's fine, it's just not what you said.

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - April 2, 2019, 6:05 p.m.
indygreg added a comment.


  I would prefer you fix them :)

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - April 2, 2019, 7:14 p.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D6164#90073, @indygreg wrote:
  
  > I would prefer you fix them :)
  
  
  Okay, then what should we call it? `--extra-packages-script` since that's what it's for?

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - April 2, 2019, 7:25 p.m.
indygreg added a comment.


  In https://phab.mercurial-scm.org/D6164#90074, @durin42 wrote:
  
  > In https://phab.mercurial-scm.org/D6164#90073, @indygreg wrote:
  >
  > > I would prefer you fix them :)
  >
  >
  > Okay, then what should we call it? `--extra-packages-script` since that's what it's for?
  
  
  Sounds good!
  
  And maybe change it to emit a newline delimited list of packages? I don't see a benefit to using `\0` since we don't need binary safety. IMO `\0` just makes it harder to implement said scripts.

REPOSITORY
  rHG Mercurial

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

To: durin42, indygreg, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/contrib/packaging/wix/build.py b/contrib/packaging/wix/build.py
--- a/contrib/packaging/wix/build.py
+++ b/contrib/packaging/wix/build.py
@@ -34,6 +34,8 @@ 
                         help='URL of timestamp server to use for signing')
     parser.add_argument('--version',
                         help='Version string to use')
+    parser.add_argument('--extra-prebuild-script',

+                        help='Extra script to run after creating virtualenv.')

 
     args = parser.parse_args()
 
@@ -54,6 +56,9 @@ 
         'version': args.version,
     }
 
+    if args.extra_prebuild_script:

+        kwargs['extra_prebuild_script'] = args.extra_prebuild_script

+

     if args.sign_sn or args.sign_cert:
         fn = build_signed_installer
         kwargs['name'] = args.name
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
@@ -177,7 +177,8 @@ 
 
 
 def build_installer(source_dir: pathlib.Path, python_exe: pathlib.Path,
-                    msi_name='mercurial', version=None, post_build_fn=None):
+                    msi_name='mercurial', version=None, post_build_fn=None,
+                    extra_prebuild_script=None):
     """Build a WiX MSI installer.
 
     ``source_dir`` is the path to the Mercurial source tree to use.
@@ -200,7 +201,8 @@ 
 
     build_py2exe(source_dir, hg_build_dir,
                  python_exe, 'wix', requirements_txt,
-                 extra_packages=EXTRA_PACKAGES)
+                 extra_packages=EXTRA_PACKAGES,
+                 extra_prebuild_script=extra_prebuild_script)
 
     version = version or normalize_version(find_version(source_dir))
     print('using version string: %s' % version)
@@ -280,7 +282,7 @@ 
 def build_signed_installer(source_dir: pathlib.Path, python_exe: pathlib.Path,
                            name: str, version=None, subject_name=None,
                            cert_path=None, cert_password=None,
-                           timestamp_url=None):
+                           timestamp_url=None, extra_prebuild_script=None):
     """Build an installer with signed executables."""
 
     post_build_fn = make_post_build_signing_fn(
@@ -292,7 +294,8 @@ 
 
     info = build_installer(source_dir, python_exe=python_exe,
                            msi_name=name.lower(), version=version,
-                           post_build_fn=post_build_fn)
+                           post_build_fn=post_build_fn,
+                           extra_prebuild_script=extra_prebuild_script)
 
     description = '%s %s' % (name, version)
 
diff --git a/contrib/packaging/hgpackaging/py2exe.py b/contrib/packaging/hgpackaging/py2exe.py
--- a/contrib/packaging/hgpackaging/py2exe.py
+++ b/contrib/packaging/hgpackaging/py2exe.py
@@ -25,7 +25,8 @@ 
                  python_exe: pathlib.Path, build_name: str,
                  venv_requirements_txt: pathlib.Path,
                  extra_packages=None, extra_excludes=None,
-                 extra_dll_excludes=None):
+                 extra_dll_excludes=None,
+                 extra_prebuild_script=None):
     """Build Mercurial with py2exe.
 
     Build files will be placed in ``build_dir``.
@@ -105,6 +106,16 @@ 
     env['DISTUTILS_USE_SDK'] = '1'
     env['MSSdk'] = '1'
 
+    if extra_prebuild_script:
+        more_packages = set(subprocess.check_output(
+            extra_prebuild_script,
+            cwd=build_dir).split(b'\0')[-1].strip().decode('utf-8').splitlines())
+        if more_packages:
+            if not extra_packages:
+                extra_packages = more_packages
+            else:
+                extra_packages |= more_packages
+
     if extra_packages:
         env['HG_PY2EXE_EXTRA_PACKAGES'] = ' '.join(sorted(extra_packages))
     if extra_excludes: