[PATCH] win32 installer for bzr 0.9: rev.2
Alexander Belchenko
bialix at ukr.net
Thu Jul 27 17:07:45 BST 2006
John Arbash Meinel пишет:
>> === 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'.
It's name from gnu.org site. I'm not invent them. So what the best
variant? COPYING (either with extension .txt or without) is good for
installer and for me. I rename it to COPYING. You agree?
>> +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'.
OK. I'll fix it.
>
> ...
>
>> + 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.
It's in branding.diff part.
>> + 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.
Thanks. I spy this feature in Cygwin.
>> === 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.
This script runs only from Makefile and makefile should be runs from
root directory of bzr source tree. I can rework this but it's simple
helper script.
>> +
>> +# 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 use native win32 port of GNU make. It's windows-only part. It's not
designed to be used on Linux because I suppose py2exe is windows-only tool.
> 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.
The simplest one method to run Inno Setup compiler is to have his
installation directory in the $PATH. I prefer to change to:
iscc /Q tools\win32\bzr.iss
> 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)
>
I don't think we need too complex solutions.
>>
>> === 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?
Yes.
>
>> === 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.
OK. I change this English sentences. Thanks for correcting.
>> +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.
I'm also start to want this. I plan to implement reading actusal version
number from bzrlib package. I glad that you positively for this.
>
> ...
>
>> +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.
OK.
>> -**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?
Well, I send this improvements for separate review but no one speak up.
https://lists.ubuntu.com/archives/bazaar-ng/2006q3/015052.html
What the best way to rework this sentences?
--
Alexander
More information about the bazaar
mailing list