locking a branch should not try to import pwd when running as a windows service

[Martin Packman]

If bzr is running as a service on windows, not only is no bzr user set, but the normal environmental variables that contain a user name are absent as well. This leads to tracebacks like the following if bzr tries to lock a branch:

Traceback (most recent call last):
  File "bzrlib\smart\request.pyo", line 355, in _call_converting_errors
  File "bzrlib\smart\request.pyo", line 140, in execute
  File "bzrlib\smart\branch.pyo", line 52, in do
  File "bzrlib\smart\branch.pyo", line 319, in do_with_branch
  File "bzrlib\branch.pyo", line 2457, in lock_write
  File "bzrlib\lockable_files.pyo", line 187, in lock_write
  File "bzrlib\lockdir.pyo", line 606, in lock_write
  File "bzrlib\lockdir.pyo", line 521, in wait_lock
  File "bzrlib\lockdir.pyo", line 482, in attempt_lock
  File "bzrlib\lockdir.pyo", line 224, in _attempt_lock
  File "bzrlib\lockdir.pyo", line 290, in _create_pending_dir
  File "bzrlib\lockdir.pyo", line 452, in _prepare_info
  File "bzrlib\osutils.pyo", line 2329, in getuser_unicode
  File "getpass.pyo", line 152, in getuser
ImportError: No module named pwd

In bzrlib.lockdir.LockDir._prepare_info we try and get the local username to put in the info file:

        try:
            user = config.username()
        except errors.NoWhoami:
            user = osutils.getuser_unicode()

But getpass.getuser falls through to importing pwd, which doesn't exist on windows, if the enviroment doesn't contain what it expects.

[Martin Pool (mbp)]

I would have thought on the server side we'd be using a username
supplied by the client anyhow, so this is doubly unnecessary.

[John A Meinel (jameinel)]

I'm pretty sure server side we just use local config (if the lock is taken via RPC rather than directly by the client).

Having worked on the launchpad conch server, it works around this by setting BZR_EMAIL before forking the bzr server.

[John A Meinel (jameinel)]

Oh, and I dug into this a bit in the past.

IIRC, the "import pwd" is actually done inside the *getuser* module. So Python's stdlib is broken here. (If it can't use the env vars, it falls back to pwd which never exists on win32.)

[Martin Packman (gz)]

See the linked question this bug report is based on for John's earlier analysis. Wasn't sure how prominently launchpad would join them so didn't call it out in the summary as well.

[Alexander Belchenko (bialix)]

I've just hit the same: https://bugs.launchpad.net/bzr/+bug/794954

Running `bzr serve` as service prevents me to push, but allows me to pull.

Running `bzr serve` as user -- everuthing is fine.

[Alexander Belchenko (bialix)]

It's pretty critical for me.

[Vincent Ladeuil (vila)]

Hmm, there should be two problematic areas in this case: log and locks.

In both cases, there should be somw workaround via env variables (which are a pain on windows but probably still the easiest).

This may lead to several different bugs so if you can document your own workarounds here that would be great.

[Alexander Belchenko (bialix)]

OK, as a workaround we can provide a fake pwd module in site-package.

[Alexander Belchenko (bialix)]

So, standard python module getpass.py has this function

def getuser():
    """Get the username from the environment or password database.

    First try various environment variables, then the password
    database. This works on Windows as long as USERNAME is set.

    """

    import os

    for name in ('LOGNAME', 'USER', 'LNAME', 'USERNAME'):
        user = os.environ.get(name)
        if user:
            return user

    # If this fails, the exception will "explain" why
    import pwd
    return pwd.getpwuid(os.getuid())[0]

Of course in the case of windows service there is no USERNAME, so that function will fall to import pwd.

[John A Meinel (jameinel)]

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

On 6/9/2011 12:25 PM, Vincent Ladeuil wrote:
> Hmm, there should be two problematic areas in this case: log and locks.
>
> In both cases, there should be somw workaround via env variables (which
> are a pain on windows but probably still the easiest).
>
> This may lead to several different bugs so if you can document your own
> workarounds here that would be great.
>

You can set %USER% for the service, and I think it makes everything
work. So there is a reasonable workaround.

We certainly can avoid pwd, but that just falls back into "we don't know
the user to take the lock as". We certainly should be able to give a
better error message. On Windows if we get to the 'pwd' section, we
could just raise an error saying "unable to determine user identity,
please set USER" or something to that effect.

AFAIK, we don't have any way to even get the service identity. If you
know of any Win32 api call that can give us something to use, I think
we'd be happy to use it.

We could fall back to "<generic-unknown-user>" or something like that,
but I think that isn't a very satisfactory answer, either.

There is one further change, but probably a different bug. Which is that
performing an RPC that would take a lock server-side, could pass client
information. So the lock would contain "locked-by SERVER on-behalf of
USER" sort of thing. I don't know that it is worth much just yet, though.

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

iEYEARECAAYFAk3wof4ACgkQJdeBCYSNAAO3KACgufs3XbL17adItNsLNmfrnCrd
jpcAoKq6HshUvhYpkGM/1OyQoYm22wyy
=utqe
-----END PGP SIGNATURE-----

[Alexander Belchenko (bialix)]

Related question: https://answers.launchpad.net/bzr/+question/123463

[Alexander Belchenko (bialix)]

Found that: http://stackoverflow.com/questions/305870/windows-services-and-environment-variables/2728574#2728574

It seems I should be able to add env variables by editing registry. Not a big deal if it'll work.

[Alexander Belchenko (bialix)]

What's finally work for me: set BZR_EMAIL in System environment variables (My Computer - Properties - Advanced - Environment Variables - System Variables: add new variable called BZR_EMAIL and set it to something like "MYSERVER").

[Alexander Belchenko (bialix)]

The patch has been landed to lp:bzr/2.3. I've been told that the patch will be eventually merged into 2.4 branch as well, so I set bug status for trunk as Fix Committed.