[MERGE] optionally install tbzr binaries with bazaar binaries on Windows

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark Hammond wrote:
> I'm attaching a new bundle which is targetted specifically at 1.6, as there is a degree of urgency in having the patch land there.  However, I'd also like it to land in .dev.  Apologies if I should have done things differently.
> 
>> Probably my biggest problem with using ENV vars is that you may not
>> really know :
>>
>> 1) That they exist at all
>> 2) What you should set them too when you do know about them.
> 
> Fair enough - but I haven't made that change in the attached bundle - it's still using env vars.

You seem to be warning about them in a reasonable fashion, as they seem to be
more-or-less required values for building the (official) win32 installers.

> 
>> locally. The environment variable is rather cryptic, though. I'm
>> guessing it isn't a standard name, why not:
>>
>> TORTOISE_OVERLAYS_MSI_PATH =
>>
>> WIN32 is redundant with MSI. I would usually request a BZR_ at the
>> beginning, but these aren't *bzr* tortoise overlays.
> 
> Actually, there are different MSI files for 32 and 64bits - I've gone with TORTOISE_OVERLAYS_MSI_WIN32 and tweaked the comments and error messages to better describe what is required.
> 

Sounds fine.

>> ^- PEP8 asks for 2 blank lines between top-level functions/classes
> ...
>> ^- same here (just add another blank line)
> 
> Done.
> 
>> +// [[[cog
>> +// if "TBZR" in os.environ:
>> +//     import os
>> +//     cog.outl("tovmsi := '%s';" %
>> os.path.basename(os.environ["TOVMSI_WIN32"]))
>> +// else:
>> +//     cog.outl("tovmsi := '';")
>> +// ]]]
>> +// [[[end]]]
>>
>> ^- We generally don't keep commented out sections, unless you have a
>> reason to include this. (Like you'll need it soon, etc.)
> 
> It is used - it just looks commented due to the way cog works.  As this cog code is in the [code] section '//' is used as comment markers rather than the ';' used in the other sections.  If the installer wasn't build with Tortoise, we will not hit this block, so we don't need to handle 'tovmsi' being empty.
> 
>> I honestly feel we should explore a
>> chain-install that sets up a python install (optionally, as a user may
>> have it), and then builds up into that, rather than using py2exe.
>> Mostly
>> because there will be a few people that upgrade continuously, and
>> having
>> them redownload all dependencies for every upgrade seems unfortunate.
>>
>> I know Python itself is a .msi installer. I wonder if we couldn't
>> leverage that, and maybe work out a setuptools based installation that
>> can download whatever dependencies we need, rather than have to bundle
>> everything all-together.
> 
> Agreed, that would be the ideal situation.  However, my short term focus is Tortoise, so I'm really just tyring to do the least possible to support that (but then got sucked into bzr-svn etc :)  There are probably a few options for doing what you describe in the longer term though...
> 

Sure. You just seemed to already be chaining at least some msi files...

> A new change in this version of the patch is that pretty much all files get installed with the inno 'restartreplace uninsrestartdelete' flag to ensure we can install and uninstall files in use (at the cost of needing a reboot for it to happen fully - but the user is free to decline to reboot at that moment)
> 
> Please let me know if any further tweaks are required.

I've seen installers that only need to reboot if there was actually a file it
couldn't modify. Do you know if that is the case here? I certainly prefer them
to ones that always say "reboot to continue" even when there was nothing held
open.

> 
> Cheers,
> 
> Mark


+def get_tbzr_py2exe_info(includes, excludes, packages, console_targets,
+                         gui_targets):

^- In the long run, it seems to make a lot more sense to change this function
into:

from tbzrlib import setup_fu
return setup_fu(*args, **kwargs)

In other words, make the packaging for tbzr exist in the tbzr project, and we
just import that into the bzr packaging.

Otherwise it seems you run version skew issues. (The existing tbzr that you
have right now has been updated, but bzr hasn't been told about it, etc.)

+def get_qbzr_py2exe_info(includes, excludes, packages):

^- Similarly here. We can certainly have a simple function that goes out and
asks tbzr/qbzr for what they want to install. But it really doesn't seem like
the real logic belongs in bzrlib.

- -            if not i.endswith('.py'):
+            if os.path.splitext(i)[1] not in [".py", ".pyd", ".dll"]:

^- I believe there is a more official set of the extensions that can be
imported in the 'imp' module. (For example, this leaves off .pyc and
module.dll, etc.)

You do seem to assume that all plugins are directories
(plugin_name/__init__.py). Which isn't strictly true (I believe the cia was a
single file for a long time.)

I would say it is the recommended way, though.

+    if 'qbzr' in plugins:
+        get_qbzr_py2exe_info(includes, excludes, packages)
+

^- So it sounds like anything we checkout into bzrlib/plugins/* will get
bundled in as a plugin. Is that correct?

I'm a bit worried, then, that your explicit whitelisting of only .py, .pyd,
.dll is incomplete. Certainly there *could* be required data files (.ico,
.png, .dat, etc.)

I know you didn't write it, but I would be much more interested in having ways
to ask plugins how to install themselves, and have *them* figure out what
files need to be included. This also extends well for how to get TBZR bundled
with the rest.


As this is the code that *you* need to build the complete win32 installer for
bzr-1.6, I'll merge it. I just would like you to take my comments into
consideration for a post 1.6 release. I really get the feeling that the
installer is a bit crufty, as people just keep bolting on "this one other
thing" to be installed, rather than really laying it out in a nice manner. I
don't know if it is just how setup.py works, or if there is some piece we are
missing.

BB:approve

And yes, it is good to write this against the 1.6 branch. I can merge it
there, and then I'll merge 1.6 into bzr.dev as part of the final release.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIo5t9JdeBCYSNAAMRAqaPAJ4rXTLcY/KsAADzuZI/byZKAU+xoQCfQOr6
abTg620cctH9izhHwYCdV9s=
=0jn6
-----END PGP SIGNATURE-----