Submitter | phabricator |
---|---|
Date | Feb. 13, 2018, 7:40 p.m. |
Message ID | <differential-rev-PHID-DREV-cindb7rezfah66ywwkpd-req@phab.mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/27840/ |
State | Superseded |
Headers | show |
Comments
indygreg added inline comments. INLINE COMMENTS > narrowbundle2.py:331 > + ui = repo.ui > + username = ui.shortuser(ui.environ.get('REMOTE_USER') or ui.username()) > + user_includes = ui.configlist( `REMOTE_USER` is an HTTP-ism and I think it is a layering violation to look for `REMOTE_USER` in code that is supposed to be protocol agnostic. What I'm trying to say is I think we'll need to tweak this ACL code further when it is moved to core. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2231 To: idlsoft, #hg-reviewers, durin42 Cc: indygreg, mercurial-devel
martinvonz added inline comments. INLINE COMMENTS > indygreg wrote in narrowbundle2.py:331 > `REMOTE_USER` is an HTTP-ism and I think it is a layering violation to look for `REMOTE_USER` in code that is supposed to be protocol agnostic. > > What I'm trying to say is I think we'll need to tweak this ACL code further when it is moved to core. But you're okay with this version for now? This commit is currently the bottom-most draft commit in the "committed" repo, so please accept it if you're okay with it. I would have accepted, but I saw this comment and wasn't sure if you thought this was bad enough to not queue it. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2231 To: idlsoft, #hg-reviewers, durin42 Cc: martinvonz, indygreg, mercurial-devel
indygreg added a comment. I think it's fine for now. I accepted this commit. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2231 To: idlsoft, #hg-reviewers, durin42 Cc: martinvonz, indygreg, mercurial-devel
idlsoft added inline comments. INLINE COMMENTS > martinvonz wrote in narrowbundle2.py:331 > But you're okay with this version for now? This commit is currently the bottom-most draft commit in the "committed" repo, so please accept it if you're okay with it. I would have accepted, but I saw this comment and wasn't sure if you thought this was bad enough to not queue it. Ideally I would have liked to have a `userid` method in repo.ui (or something like that), and have it used by both `narrow` and `acl` extension. But I think this deserves a longer discussion, hence the quick fix. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2231 To: idlsoft, #hg-reviewers, durin42 Cc: martinvonz, indygreg, mercurial-devel
Patch
diff --git a/hgext/narrow/narrowbundle2.py b/hgext/narrow/narrowbundle2.py --- a/hgext/narrow/narrowbundle2.py +++ b/hgext/narrow/narrowbundle2.py @@ -327,13 +327,14 @@ part.addparam('treemanifest', '1') def applyacl_narrow(repo, kwargs): - username = repo.ui.shortuser(repo.ui.username()) - user_includes = repo.ui.configlist( + ui = repo.ui + username = ui.shortuser(ui.environ.get('REMOTE_USER') or ui.username()) + user_includes = ui.configlist( _NARROWACL_SECTION, username + '.includes', - repo.ui.configlist(_NARROWACL_SECTION, 'default.includes')) - user_excludes = repo.ui.configlist( + ui.configlist(_NARROWACL_SECTION, 'default.includes')) + user_excludes = ui.configlist( _NARROWACL_SECTION, username + '.excludes', - repo.ui.configlist(_NARROWACL_SECTION, 'default.excludes')) + ui.configlist(_NARROWACL_SECTION, 'default.excludes')) if not user_includes: raise error.Abort(_("{} configuration for user {} is empty") .format(_NARROWACL_SECTION, username))