[MERGE] win98 support

Alexander Belchenko bialix at ukr.net
Fri Feb 2 05:43:23 GMT 2007


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

John Arbash Meinel пишет:
> Alexander Belchenko wrote:
>> This patch provide support for OS Windows 98.
>> bzr already mostly works on win98 but have some
>> incompatibility in the methods how bzr determine
>> some info about platform itself.

>> I test it on the win98 machine that I have access to.

I assume I incorrect saying: I was tested this patch on real win98 machine.
So I can guarantee that it works on that machine.

> 
> v- We don't generally do "Author:" tags. There isn't a strict style
> statement, and I've noticed that Robert tends to put his name.

It's my bad habit from the days when I don't have PQM access. I'll remove this.

> 
> v- You might also want to look at 'os.name'. But as long as this works
> for you.

No, no, no.
My previous assumption (before I wrote this patch) was that os.name reflects
Windows version. This assumption was completely wrong. os.name has value 'nt'
on any Windows platform I can test: 98, 2000, XP. So the only one way to determine
real windows version is to ask sys.getwindowsversion().
> 
> +
> +# Windows version
> +if sys.platform == 'win32':
> +    _major,_minor,_build,_platform,_text = sys.getwindowsversion()
> 
> v- hmm.. a bare Exception isn't very nice, but whatever. I would also
> recommend "Unknown platform (unsupported): %s' % (sys.getwindowsversion(),)
> 
> Or something like that to make it clear what is going on, and maybe give
> the user a way to figure out a work around.
> 

Actually this exception is a little bit paranoid check. I'd like to remove this
completely with check:

if _platform < 2:
    winver = 'Windows 98'
else:
    winver = 'Windows NT'

> Also, using "if _platform not in (0, 1): _platform == 'Windows NT'
> doesn't seem quite right.
> What is Vista going to return?

Can't predict and can't check because I don't have access to Vista
in my environment. But by analogy with current ranking of _platform
it's will be either 2 or 3.

> 
> You may want 'os.name' since it might be more accurate for your purposes.
> 

As I explain above using os.name is completely wrong.

> +def get_appdata_location():
> +    """Return Application Data location.
> +    Return None if we cannot obtain location.
> +
> +    Returned value can be unicode or plain sring.
> +    To convert plain string to unicode use
> +    s.decode(bzrlib.user_encoding)
> +    """
> +    if has_ctypes:
> +        try:
> +            SHGetSpecialFolderPath = \
> +                ctypes.windll.shell32.SHGetSpecialFolderPathW
> +        except AttributeError:
> +            pass
> +        else:
> +            buf = ctypes.create_unicode_buffer(MAX_PATH)
> +            if SHGetSpecialFolderPath(None,buf,CSIDL_APPDATA,0):
> +                return buf.value
> +    # from env variable
> +    appdata = os.environ.get('APPDATA')
> +    if appdata:
> +        return appdata
> +    # if we fall to this point we on win98
> +    # at least try C:/WINDOWS/Application Data
> +    windir = os.environ.get('windir')
> +    if windir:
> +        appdata = os.path.join(windir, 'Application Data')
> +        if os.path.isdir(appdata):
> +            return appdata
> +    # did not find anything
> +    return None
> 
> ^- I would imagine this might do really weird things with an
> Internationalized version of Win98 (like they do with Program Files,
> etc). But as long as it works some of the time, it is probably better
> than what we have now.

Exactly. I never saw Internationalized version of Win98, but
Russian Win98 has only My Documents folder translated to Russian,
and other system folder stay in English. So I assume it's mostly safe.
Again, if we have bug report in the future we can deal with new info
and update this part.

> 
> 
> ....
> 
> +    # at least return windows root directory
> +    windir = os.environ.get('windir')
> +    if windir:
> +        return os.path.splitdrive(windir) + '/'
> +    # otherwise C:\ is good enough for 98% users
> +    return 'C:/'
> 
> ^- I thought there was something like "windrive". 

On my Win98 there is only 'windir' and 'winbootdir', but in MSDN I found
mention only about windir. WinXP Home has only 'windir'.

> I know I have a friend
> who installed his bootable windows onto E:/ and he has no end of pain
> from apps that assume there *is* a 'C:/' and that it is the default drive.
> But if "windir" gets us most of the way, we can go with it.

I need return something when other attempts fails. If such user don't have
C: drive at all, then bzr only print warning about inability of creating
C:\.bzr.log and go on. So even in worst case bzr don't fails with error
or traceback.

> ....
> 
> v- I would actually prefer to do ~/bzr.log on Windows, since $HOME does
> not have the same meaning.

On Win98 '~' cannot be expanded to anything useful. On Win2000/XP
there is env variables $HOMEDRIVE and $HOMEPATH that in conjunction
used to substitute '~'. But on different platforms it could point either
to C:\ or to C:\Document and Settings\USERNAME. It's a little bit
weird to explain every time to people who send the bug report
where it could to find .bzr.log.

So moving .bzr.log to My Documents I consider to be the improvements.

> 
> What I would really rather see is us changing from ~/.bzr.log to
> ~/.bazaar/bzr.log and %APPDATA%/Bazaar/2.0/bzr.log

I really think that %APPDATA%/Bazaar/2.0/bzr.log is BAD idea.
Despite the fact that APPDATA does not exists on Win98, on Windows 2000/XP
the folder Application Data is marked as system. So when user open
C:\Document and Settings\USERNAME folder he/she usually don't see
Application Data at all. Until he/she change settings of folder view
and turn off option like 'Hide system and hidden folder'.
So I seriously think that user don't need to look into this folder.
Especially on Win98 where it resides inside C:\Windows directory with
all system files.

I wrote bzr-config utility to handle various bzr settings, so now
we can hide bazaar.conf from the view of novices.

> 
> That said, this is still better than what we've had.
> 
> +    if tracefilename is None:
> +        if sys.platform == 'win32':
> +            from bzrlib import win32utils
> +            home = win32utils.get_home_location()
> +        else:
> +            home = os.path.expanduser('~')
> +        tracefilename = os.path.join(home, '.bzr.log')
> +
> +    trace_fname = os.path.expanduser(tracefilename)
> 
> ....
> You can actually do "tf.seek(0, 1)", which tells it to move 0 characters
> from the current position. Or you can do "tf.seek(0, 2)" which tells it
> to seek to the end of the file.
> That is a little better than writing out something first.

Why?
I think adding explicitly empty line between consequent logs is improvements.
I was very frustrating to see the .bzr.log on linux machine. There is no easy
way to detect where one record ends and another begins. It's very hard to
copy paste from .bzr.log. At least for me.

> 
> +        # tf.tell() on windows always return 0 until some writing done
> +        tf.write('\n')
> +        if tf.tell() <= 2:
> +            tf.write("this is a debug log for diagnosing/reporting
> problems in bzr\n")
>              tf.write("you can delete or truncate this file, or include
> sections in\n")
>              tf.write("bug reports to bazaar at lists.canonical.com\n\n")
>          _file_handler = logging.StreamHandler(tf)
> 
> 
> Otherwise I'm happy with the changes. None of my comments have to be
> done. So I'll leave it up to you to decide how you want to proceed.

I try to explain some my decisions above. If you don't have other warnings
I'll merge this with removing 'Author:' and exception during detecting
Windows version.

I also assume I need to add NEWS entry.

- --
Alexander

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFws97zYr338mxwCURAufWAKCL8xTq3djodKD6+zMnVpb72s29rwCePA1Y
TvFJfCveZpMsTXqlNZB1v3A=
=/igN
-----END PGP SIGNATURE-----




More information about the bazaar mailing list