Patchwork [1,of,2] acl: replace bare getpass.getuser() by platform function

login
register
mail settings
Submitter Yuya Nishihara
Date Feb. 25, 2018, 2:43 a.m.
Message ID <03eff66adb3b53f97766.1519526581@mimosa>
Download mbox | patch
Permalink /patch/28353/
State Accepted
Headers show

Comments

Yuya Nishihara - Feb. 25, 2018, 2:43 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1519524781 -32400
#      Sun Feb 25 11:13:01 2018 +0900
# Node ID 03eff66adb3b53f9776628f83b6433ee7b57ee52
# Parent  38f4805020437f126f5c1c8f41d78445f9ab6547
acl: replace bare getpass.getuser() by platform function

Follows up dbadf28d4db0. bytestr() shouldn't be applied here because getuser()
isn't guaranteed to be all in ASCII.

This change means GetUserNameA() is used on Windows, but that's probably
better than trying to get the current user name in UNIX way.
Pulkit Goyal - Feb. 25, 2018, 8:57 a.m.
Missed the followup. Thanks for it. Looks good to me.

On Sun, Feb 25, 2018 at 8:13 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1519524781 -32400
> #      Sun Feb 25 11:13:01 2018 +0900
> # Node ID 03eff66adb3b53f9776628f83b6433ee7b57ee52
> # Parent  38f4805020437f126f5c1c8f41d78445f9ab6547
> acl: replace bare getpass.getuser() by platform function
>
> Follows up dbadf28d4db0. bytestr() shouldn't be applied here because getuser()
> isn't guaranteed to be all in ASCII.
>
> This change means GetUserNameA() is used on Windows, but that's probably
> better than trying to get the current user name in UNIX way.
>
> diff --git a/hgext/acl.py b/hgext/acl.py
> --- a/hgext/acl.py
> +++ b/hgext/acl.py
> @@ -193,14 +193,11 @@ 3) Deny access to a file to anyone but u
>
>  from __future__ import absolute_import
>
> -import getpass
> -
>  from mercurial.i18n import _
>  from mercurial import (
>      error,
>      extensions,
>      match,
> -    pycompat,
>      registrar,
>      util,
>  )
> @@ -341,7 +338,7 @@ def hook(ui, repo, hooktype, node=None,
>              user = urlreq.unquote(url[3])
>
>      if user is None:
> -        user = pycompat.bytestr(getpass.getuser())
> +        user = util.getuser()
>
>      ui.debug('acl: checking access for user "%s"\n' % user)
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - Feb. 25, 2018, 8:46 p.m.
On Sat, Feb 24, 2018 at 6:43 PM, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1519524781 -32400
> #      Sun Feb 25 11:13:01 2018 +0900
> # Node ID 03eff66adb3b53f9776628f83b6433ee7b57ee52
> # Parent  38f4805020437f126f5c1c8f41d78445f9ab6547
> acl: replace bare getpass.getuser() by platform function
>

Queued these 2. Thanks for the follow-ups.


>
> Follows up dbadf28d4db0. bytestr() shouldn't be applied here because
> getuser()
> isn't guaranteed to be all in ASCII.
>
> This change means GetUserNameA() is used on Windows, but that's probably
> better than trying to get the current user name in UNIX way.
>
> diff --git a/hgext/acl.py b/hgext/acl.py
> --- a/hgext/acl.py
> +++ b/hgext/acl.py
> @@ -193,14 +193,11 @@ 3) Deny access to a file to anyone but u
>
>  from __future__ import absolute_import
>
> -import getpass
> -
>  from mercurial.i18n import _
>  from mercurial import (
>      error,
>      extensions,
>      match,
> -    pycompat,
>      registrar,
>      util,
>  )
> @@ -341,7 +338,7 @@ def hook(ui, repo, hooktype, node=None,
>              user = urlreq.unquote(url[3])
>
>      if user is None:
> -        user = pycompat.bytestr(getpass.getuser())
> +        user = util.getuser()
>
>      ui.debug('acl: checking access for user "%s"\n' % user)
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Matt Harbison - Feb. 27, 2018, 5:18 a.m.
On Sat, 24 Feb 2018 21:43:01 -0500, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1519524781 -32400
> #      Sun Feb 25 11:13:01 2018 +0900
> # Node ID 03eff66adb3b53f9776628f83b6433ee7b57ee52
> # Parent  38f4805020437f126f5c1c8f41d78445f9ab6547
> acl: replace bare getpass.getuser() by platform function
>
> Follows up dbadf28d4db0. bytestr() shouldn't be applied here because  
> getuser()
> isn't guaranteed to be all in ASCII.
>
> This change means GetUserNameA() is used on Windows, but that's probably
> better than trying to get the current user name in UNIX way.

This ends up using the real Windows user, which breaks test-acl.t.  I  
didn't look past the first failure, but maybe a common-pattern.py  
substitution can be cooked up?  I assume globbing the user away totally is  
wrong.
Yuya Nishihara - Feb. 27, 2018, 1:58 p.m.
On Tue, 27 Feb 2018 00:18:39 -0500, Matt Harbison wrote:
> On Sat, 24 Feb 2018 21:43:01 -0500, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1519524781 -32400
> > #      Sun Feb 25 11:13:01 2018 +0900
> > # Node ID 03eff66adb3b53f9776628f83b6433ee7b57ee52
> > # Parent  38f4805020437f126f5c1c8f41d78445f9ab6547
> > acl: replace bare getpass.getuser() by platform function
> >
> > Follows up dbadf28d4db0. bytestr() shouldn't be applied here because  
> > getuser()
> > isn't guaranteed to be all in ASCII.
> >
> > This change means GetUserNameA() is used on Windows, but that's probably
> > better than trying to get the current user name in UNIX way.
> 
> This ends up using the real Windows user, which breaks test-acl.t.  I  
> didn't look past the first failure, but maybe a common-pattern.py  
> substitution can be cooked up?  I assume globbing the user away totally is  
> wrong.

Perhaps we can mock it up. I'll send a patch shortly.

Patch

diff --git a/hgext/acl.py b/hgext/acl.py
--- a/hgext/acl.py
+++ b/hgext/acl.py
@@ -193,14 +193,11 @@  3) Deny access to a file to anyone but u
 
 from __future__ import absolute_import
 
-import getpass
-
 from mercurial.i18n import _
 from mercurial import (
     error,
     extensions,
     match,
-    pycompat,
     registrar,
     util,
 )
@@ -341,7 +338,7 @@  def hook(ui, repo, hooktype, node=None, 
             user = urlreq.unquote(url[3])
 
     if user is None:
-        user = pycompat.bytestr(getpass.getuser())
+        user = util.getuser()
 
     ui.debug('acl: checking access for user "%s"\n' % user)