[PATCH] win32 installer for bzr 0.9: rev.2

[John Arbash Meinel]

Alexander Belchenko wrote:
> Attached patch is new improved version of installer for bzr 0.9.
> Difference from previous version of patch:
> 
> - python-based installer create after installation entry in Start =>
> Program => Bazaar-NG menu with links to documentation, and to bzr
> - python-based install no more restricted to python 2.4 because python
> 2.5 is coming
> - improvements to doc-generator (man/rstx): show aliases for command in
> detailed section, listed more environment variables used by bzr
> - during installation of standalone bzr.exe installer show text of GPL
> license and require to user to select "I agree" before proceed actual
> installation.
> 
> This is really big patch and I assume that reviewing of this patch
> require to have some knowledge about windows installers, so probably is
> better to ask me if someone don't understand some parts of my patch.
> 
> Recently I see that on website bzr change their name from Bazaar-NG to
> Bazaar. If I need to change this name in installer please speak.
> 
> Also, I see that bzr has new icon. If this icon now is official bzr icon
> then I need two scaled images (png is good enough) 32x32 and 16x16 to
> make new bzr.ico for windows. Or (instead) I need ready-to-use bzr.ico
> for windows with new style.
> 
> -- 
> Alexander

...

> === added file 'gpl.txt'
> --- gpl.txt	1970-01-01 00:00:00 +0000
> +++ gpl.txt	2006-07-25 15:00:47 +0000
> @@ -0,0 +1,339 @@
> +		    GNU GENERAL PUBLIC LICENSE
> +		       Version 2, June 1991
> +
> + Copyright (C) 1989, 1991 Free Software Foundation, Inc.,
> + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + Everyone is permitted to copy and distribute verbatim copies
> + of this license document, but changing it is not allowed.

I do believe something like the GPL is generally added as either
COPYING.txt, or as GPL. I'd probably be happier with GPG.txt than 'gpl.txt'.

...

> +import os
> +import sys
> +import _winreg

...

> +    batch_path = "bzr.bat"
> +    prefix = sys.prefix
> +    try:
> +        ##
> +        # try to create
> +        scripts_dir = os.path.join(prefix, "Scripts")
> +        script_path = os.path.join(scripts_dir, "bzr")
> +        python_path = os.path.join(prefix, "python.exe")
> +        batch_str = "@%s %s %%*\n" % (python_path, script_path)

^- This is unsafe when paths contain spaces. It is pbetter to do:
batch_str = '@"%s" "%s" %%*\n' % (python_path, script_path)

I also feel like it would be better to use
sys.executable, rather than joining prefix + 'python.exe'.

...

> +        f = file(os.path.join(dst,'bazaar.conf'), 'w')
> +        f.write("# main configuration file of Bazaar-NG\n"
> +                "[DEFAULT]\n"
> +                "#email=Your Name <you at domain.com>\n")
> +        f.close()

I'm assuming your later patch addresses Bazaar-NG => Bazaar, so I won't
comment much. But I wanted to make one comment to make sure they are
handled.

...

> +    if add_shell_menu and not delete_shell_menu:
> +        hkey = None
> +        try:
> +            hkey = _winreg.CreateKey(_winreg.HKEY_CLASSES_ROOT,
> +                                     r'Folder\shell\bzr')
> +        except EnvironmentError:
> +            if not silent:
> +                MessageBoxA(None,
> +                            'Unable to create registry key for context menu',
> +                            'EnvironmentError',
> +                            MB_OK | MB_ICONERROR)
> +
> +        if not hkey is None:
> +            _winreg.SetValue(hkey, '', _winreg.REG_SZ, 'Bzr Here')
> +            hkey2 = _winreg.CreateKey(hkey, 'command')
> +            _winreg.SetValue(hkey2, '', _winreg.REG_SZ,
> +                             'cmd /K "%s"' % os.path.join(bzr_dir,
> +                                                          'start_bzr.bat'))
> +            _winreg.CloseKey(hkey2)
> +            _winreg.CloseKey(hkey)
> +

I like the shell menu. I realize it only does one thing at this point.
But it still helps.


> === added file 'tools/win32/copy_docs.py'
> --- tools/win32/copy_docs.py	1970-01-01 00:00:00 +0000
> +++ tools/win32/copy_docs.py	2006-06-29 08:41:31 +0000
> @@ -0,0 +1,15 @@
> +import glob
> +import os
> +import shutil
> +
> +TGT_DIR = 'win32_bzr.exe/doc'
> +
> +if not os.path.exists(TGT_DIR):
> +    os.makedirs(TGT_DIR)

This looks like it needs to be run in a specific directory. It would be
better to write this as a function that can take what directory it needs
to work in.

It probably should take the base directory of bzr, and the directory to
put the final documents in.

...

> +
> +# make bzr.exe for win32 with py2exe
> +exe:
> +	@echo Make bzr.exe
> +	setup.py py2exe > py2exe.log
> +	copy /Y tools\win32\start_bzr.bat win32_bzr.exe\start_bzr.bat
> +	copy /Y tools\win32\bazaar.url win32_bzr.exe\bazaar.url

What Make version are you running? cygwin or mingw?

I'm wondering because 'copy' is not cross platform safe. I both cygwin
and mingw come with 'cp'. Which means you can write something that would
work on Linux as well as win32.

...

> +# win32 installer for bzr.exe
> +installer: exe copy_docs
> +	@echo Make windows installer
> +	cog.py -d -o tools\win32\bzr.iss tools\win32\bzr.iss.cog
> +	"C:\Program Files\Inno Setup 5\iscc" /Q tools\win32\bzr.iss

There are other ways to start 'bzr.iss' without requiring the exact path
to Inno Setup.

I don't know the right way to do it from a Makefile, but in python I use
either:
win32api.ShellExecute(0, 'compile', filename, None, None, 0)

Or with ctypes:
ctypes.windll.shell32.ShellExecuteA(0, 'compile', filename,
                                    None, None, 0)

> +
> +# win32 python's distutils-based installer
> +# require to have python interpreter installed on win32
> +python-installer: docs
> +	python setup.py bdist_wininst --title="Bazaar-NG" --install-script="bzr-win32-bdist-postinstall.py"
> +
> +
> +# clean on win32 all installer-related files and directories
> +clean_win32:
> +	if exist build rmdir /S /Q build
> +	if exist win32_bzr.exe rmdir /S /Q win32_bzr.exe
> +	if exist py2exe.log del /Q py2exe.log
> +	if exist doc\*.htm del /Q doc\*.htm
> +	if exist doc\bzr_man.txt del /Q doc\bzr_man.txt
> +	if exist tools\win32\bzr.iss del /Q tools\win32\bzr.iss
> +	if exist bzr-setup*.exe del /Q bzr-setup*.exe
> +	if exist dist rmdir /S /Q dist
> 
> === modified file 'bzrlib/doc/api/__init__.py'
> --- bzrlib/doc/api/__init__.py	2006-06-06 08:09:09 +0000
> +++ bzrlib/doc/api/__init__.py	2006-06-29 08:41:31 +0000
> @@ -28,7 +28,11 @@
>  import os
>      
>  def test_suite():
> -    candidates = os.listdir(os.path.dirname(__file__))
> +    dir_ = os.path.dirname(__file__)
> +    if os.path.isdir(dir_):
> +        candidates = os.listdir(dir_)
> +    else:
> +        candidates = []
>      scripts = [candidate for candidate in candidates
>                 if candidate.endswith('.txt')]
>      return doctest.DocFileSuite(*scripts)

^- Why is this failing? Because the standalone doesn't have a 'doc/api'
directory?

> 
> === modified file 'bzrlib/tests/test_setup.py'
> --- bzrlib/tests/test_setup.py	2006-05-05 01:15:53 +0000
> +++ bzrlib/tests/test_setup.py	2006-06-29 08:41:31 +0000
> @@ -32,8 +32,10 @@
>  
>      def test_build(self):
>          """ test cmd `python setup.py build`
> -        
> -        This typically catches new subdirectories which weren't added to setup.py
> +
> +        This test ensure that build process run correct.

> +        Ensure that man generator works correct.
> +        Also can catches new subdirectories which weren't added to setup.py

This tests that the build process and man generator run correctly.
It also can catch new subdirectories that weren't added to setup.py.

...

> +META_INFO = {'name':         'bzr',
> +             'version':      '0.9pre',
> +             'author':       'Canonical Ltd',
> +             'author_email': 'bazaar-ng at lists.ubuntu.com',
> +             'url':          'http://www.bazaar-vcs.org/',
> +             'description':  'Friendly distributed version control system',
> +             'license':      'GNU GPL v2',
> +            }

It seems like we should be setting 'version':'' to the version string
from bzrlib.

...

> +elif 'py2exe' in sys.argv:
> +    # py2exe setup
> +    import py2exe
> +
> +    # pick real bzr version
> +    import bzrlib
> +
> +    version_number = []
> +    for i in bzrlib.version_info[:4]:
> +        try:
> +            i = int(i)
> +        except ValueError:
> +            i = 0
> +        version_number.append(str(i))
> +    version_str = '.'.join(version_number)

I see that you get it here, but only for 'py2exe'. It would seem better
to always update it from 'bzrlib'. I would really prefer not to have to
track through the source code to find references to the version number
whenever we release a new version.

So I would prefer if the default was '<unknown>', and it was just
updated as part of this script.

...

> -**On Linux**:  ``~/.bazaar/bazaar.conf/``
> +**On Linux**:  ``~/.bazaar/bazaar.conf``
>  
>  **On Windows**: ``C:\\Documents and Settings\\username\\Application Data\\bazaar\\2.0\\bazaar.conf``
>  
> -Contains the default user config. Only one section, ``[DEFAULT]`` is allowed.
> +Contains the default user config. At least one section, ``[DEFAULT]`` is required.
>  A typical default config file may be similiar to::
>  
>      [DEFAULT]


I don't believe that [DEFAULT] is actually required.
In fact, I believe if you only have entries in the file, they
automatically go into the 'DEFAULT' section.

Does your experience contradict this?

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060727/39e619eb/attachment.pgp