D5299: phabricator: fallback reading arcanist config files
philpep added a comment. Thanks for reviewing @Kwan ! Actually I think parsing/merging arcconfig files will be quite hard to maintain because they are not fully documented and subject to changes. For my needs, just having hg being able to get phabricator.url and phabricator.callsign from .arcconfig in the repo root is enough. Then I just need to match this with hg auth config. I'll try to have a smaller patch not trying to read other files than .arcconfig and not reading the token. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5299 To: philpep, #hg-reviewers, durin42 Cc: durin42, Kwan, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5299: phabricator: fallback reading arcanist config files
philpep marked 3 inline comments as done. philpep added inline comments. INLINE COMMENTS > mharbison72 wrote in phabricator.py:187 > s/.encoding/.environ/ ? Woops... Thanks for catching this! > mharbison72 wrote in phabricator.py:200 > Should this be using vfs to open, instead of raw open? Using the vfs layer > allows the class that provides posix-like functionality on Windows to be used. Yes, done. > mharbison72 wrote in phabricator.py:290 > It might be clearer to return None, since the function is to fetch the repoid. Indeed, I fixed this. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5299 To: philpep, #hg-reviewers Cc: mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5299: phabricator: fallback reading arcanist config files
philpep updated this revision to Diff 14253. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5299?vs=14175&id=14253 REVISION DETAIL https://phab.mercurial-scm.org/D5299 AFFECTED FILES hgext/phabricator.py CHANGE DETAILS diff --git a/hgext/phabricator.py b/hgext/phabricator.py --- a/hgext/phabricator.py +++ b/hgext/phabricator.py @@ -37,6 +37,9 @@ # API token. Get it from https://$HOST/conduit/login/ example.phabtoken = cli- + +As a fallback, read config from arc config files (.arcconfig, ~/.arcrc and +/etc/arcconfig) """ from __future__ import absolute_import @@ -46,6 +49,7 @@ import json import operator import re +import os from mercurial.node import bin, nullid from mercurial.i18n import _ @@ -60,13 +64,15 @@ parser, patch, phases, +pycompat, registrar, scmutil, smartset, tags, templateutil, url as urlmod, util, +vfs as vfsmod, ) from mercurial.utils import ( procutil, @@ -171,16 +177,49 @@ process(b'', params) return util.urlreq.urlencode(flatparams) +def readarcconfig(repo): +"""Return url, token, callsign read from arcanist config files + +This read and merge content of /etc/arcconfig, ~/.arcrc and .arconfig. +""" +if pycompat.iswindows: +paths = [ +vfsmod.vfs(encoding.environ['ProgramData']).join( + 'Phabricator', 'Arcanist', 'config'), +vfsmod.vfs(encoding.environ['AppData']).join('.arcrc') +] +else: +paths = [ +vfsmod.vfs('/etc').join('.arconfig'), +os.path.expanduser('~/.arcrc'), +] +paths.append(vfsmod.vfs(repo.root).join('.arcconfig')) +config = {} +for path in paths: +if vfsmod.vfs(path).exists(): +with vfsmod.vfs(path).open(None, auditpath=False) as f: +config.update(json.load(f)) +callsign = config.get('repository.callsign') +conduit_uri = config.get('conduit_uri', config.get('config', {}).get('default')) +if conduit_uri is not None: +token = config.get('hosts', {}).get(conduit_uri, {}).get('token') +url = conduit_uri.rstrip('/api/') +return url, token, callsign + def readurltoken(repo): """return conduit url, token and make sure they exist -Currently read from [auth] config section. In the future, it might -make sense to read from .arcconfig and .arcrc as well. +Currently read from [auth] config section and fallback to reading arc +config files. """ url = repo.ui.config(b'phabricator', b'url') if not url: -raise error.Abort(_(b'config %s.%s is required') - % (b'phabricator', b'url')) +url, token, __ = readarcconfig(repo) +if not url or not token: +raise error.Abort(_(b'unable to read phabricator conduit url and ' +b'token from config %s.%s or from arc config ' +b'files') % (b'phabricator', b'url')) +return url, token res = httpconnectionmod.readauthforuri(repo.ui, url, util.url(url).user) token = None @@ -246,7 +285,9 @@ return repophid callsign = repo.ui.config(b'phabricator', b'callsign') if not callsign: -return None +__, __, callsign = readarcconfig(repo) +if not callsign: +return None query = callconduit(repo, b'diffusion.repository.search', {b'constraints': {b'callsigns': [callsign]}}) if len(query[r'data']) == 0: To: philpep, #hg-reviewers Cc: mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5299: phabricator: fallback reading arcanist config files
philpep marked 2 inline comments as done. philpep added a comment. Thanks for the review and sorry for taking so long time to update the patch... I made changes to use encoding.environ and vfs like you suggested. Please let me known what I can do to introduce this (or at least part of) this feature, thanks! REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5299 To: philpep, #hg-reviewers Cc: mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5299: phabricator: fallback reading arcanist config files
philpep updated this revision to Diff 14175. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5299?vs=14174&id=14175 REVISION DETAIL https://phab.mercurial-scm.org/D5299 AFFECTED FILES hgext/phabricator.py CHANGE DETAILS diff --git a/hgext/phabricator.py b/hgext/phabricator.py --- a/hgext/phabricator.py +++ b/hgext/phabricator.py @@ -37,6 +37,9 @@ # API token. Get it from https://$HOST/conduit/login/ example.phabtoken = cli- + +As a fallback, read config from arc config files (.arcconfig, ~/.arcrc and +/etc/arcconfig) """ from __future__ import absolute_import @@ -46,6 +49,7 @@ import json import operator import re +import os from mercurial.node import bin, nullid from mercurial.i18n import _ @@ -60,13 +64,15 @@ parser, patch, phases, +pycompat, registrar, scmutil, smartset, tags, templateutil, url as urlmod, util, +vfs as vfsmod, ) from mercurial.utils import ( procutil, @@ -171,16 +177,49 @@ process(b'', params) return util.urlreq.urlencode(flatparams) +def readarcconfig(repo): +"""Return url, token, callsign read from arcanist config files + +This read and merge content of /etc/arcconfig, ~/.arcrc and .arconfig. +""" +if pycompat.iswindows: +paths = [ +vfsmod.vfs(encoding.encoding['ProgramData']).join( + 'Phabricator', 'Arcanist', 'config'), +vfsmod.vfs(encoding.environ['AppData']).join('.arcrc') +] +else: +paths = [ +vfsmod.vfs('/etc').join('.arconfig'), +os.path.expanduser('~/.arcrc'), +] +paths.append(vfsmod.vfs(repo.root).join('.arcconfig')) +config = {} +for path in paths: +if vfsmod.vfs(path).exists(): +with open(path, 'rb') as f: +config.update(json.load(f)) +callsign = config.get('repository.callsign') +conduit_uri = config.get('conduit_uri', config.get('config', {}).get('default')) +if conduit_uri is not None: +token = config.get('hosts', {}).get(conduit_uri, {}).get('token') +url = conduit_uri.rstrip('/api/') +return url, token, callsign + def readurltoken(repo): """return conduit url, token and make sure they exist -Currently read from [auth] config section. In the future, it might -make sense to read from .arcconfig and .arcrc as well. +Currently read from [auth] config section and fallback to reading arc +config files. """ url = repo.ui.config(b'phabricator', b'url') if not url: -raise error.Abort(_(b'config %s.%s is required') - % (b'phabricator', b'url')) +url, token, __ = readarcconfig(repo) +if not url or not token: +raise error.Abort(_(b'unable to read phabricator conduit url and ' +b'token from config %s.%s or from arc config ' +b'files') % (b'phabricator', b'url')) +return url, token res = httpconnectionmod.readauthforuri(repo.ui, url, util.url(url).user) token = None @@ -246,7 +285,9 @@ return repophid callsign = repo.ui.config(b'phabricator', b'callsign') if not callsign: -return None +__, __, callsign = readarcconfig(repo) +if not callsign: +return callsign query = callconduit(repo, b'diffusion.repository.search', {b'constraints': {b'callsigns': [callsign]}}) if len(query[r'data']) == 0: To: philpep, #hg-reviewers Cc: mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5299: phabricator: fallback reading arcanist config files
philpep updated this revision to Diff 14174. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5299?vs=12590&id=14174 REVISION DETAIL https://phab.mercurial-scm.org/D5299 AFFECTED FILES hgext/phabricator.py CHANGE DETAILS diff --git a/hgext/phabricator.py b/hgext/phabricator.py --- a/hgext/phabricator.py +++ b/hgext/phabricator.py @@ -37,6 +37,9 @@ # API token. Get it from https://$HOST/conduit/login/ example.phabtoken = cli- + +As a fallback, read config from arc config files (.arcconfig, ~/.arcrc and +/etc/arcconfig) """ from __future__ import absolute_import @@ -46,6 +49,7 @@ import json import operator import re +import os from mercurial.node import bin, nullid from mercurial.i18n import _ @@ -67,6 +71,7 @@ templateutil, url as urlmod, util, +vfs as vfsmod, ) from mercurial.utils import ( procutil, @@ -171,16 +176,49 @@ process(b'', params) return util.urlreq.urlencode(flatparams) +def readarcconfig(repo): +"""Return url, token, callsign read from arcanist config files + +This read and merge content of /etc/arcconfig, ~/.arcrc and .arconfig. +""" +if os.name == 'nt': +paths = [ +vfsmod.vfs(encoding.encoding['ProgramData']).join( + 'Phabricator', 'Arcanist', 'config'), +vfsmod.vfs(encoding.environ['AppData']).join('.arcrc') +] +else: +paths = [ +vfsmod.vfs('/etc').join('.arconfig'), +os.path.expanduser('~/.arcrc'), +] +paths.append(vfsmod.vfs(repo.root).join('.arcconfig')) +config = {} +for path in paths: +if os.path.exists(path): +with open(path, 'rb') as f: +config.update(json.load(f)) +callsign = config.get('repository.callsign') +conduit_uri = config.get('conduit_uri', config.get('config', {}).get('default')) +if conduit_uri is not None: +token = config.get('hosts', {}).get(conduit_uri, {}).get('token') +url = conduit_uri.rstrip('/api/') +return url, token, callsign + def readurltoken(repo): """return conduit url, token and make sure they exist -Currently read from [auth] config section. In the future, it might -make sense to read from .arcconfig and .arcrc as well. +Currently read from [auth] config section and fallback to reading arc +config files. """ url = repo.ui.config(b'phabricator', b'url') if not url: -raise error.Abort(_(b'config %s.%s is required') - % (b'phabricator', b'url')) +url, token, __ = readarcconfig(repo) +if not url or not token: +raise error.Abort(_(b'unable to read phabricator conduit url and ' +b'token from config %s.%s or from arc config ' +b'files') % (b'phabricator', b'url')) +return url, token res = httpconnectionmod.readauthforuri(repo.ui, url, util.url(url).user) token = None @@ -246,7 +284,9 @@ return repophid callsign = repo.ui.config(b'phabricator', b'callsign') if not callsign: -return None +__, __, callsign = readarcconfig(repo) +if not callsign: +return callsign query = callconduit(repo, b'diffusion.repository.search', {b'constraints': {b'callsigns': [callsign]}}) if len(query[r'data']) == 0: To: philpep, #hg-reviewers Cc: mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5299: phabricator: fallback reading arcanist config files
philpep added inline comments. INLINE COMMENTS > phabricator.py:175 > +def readarcconfig(repo): > +"""Return url, token, callsign read from arcanist config files > + I'd be nice to cache the result of `readarcconfig` but I don't known how to implement this, any suggestion ? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5299 To: philpep, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5299: phabricator: fallback reading arcanist config files
philpep added inline comments. INLINE COMMENTS > phabricator.py:201 > +if conduit_uri is not None: > +token = config.get('hosts', {}).get(conduit_uri, {}).get('token') > +url = conduit_uri.rstrip('/api/') HINT: This doesn't work for current mercurial config because "arc install-certificates" add a trailing "/" to conduit_uri and our .arcconfig doesn't have this trailing slash. Any idea how to handle this properly ? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5299 To: philpep, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5299: phabricator: fallback reading arcanist config files
philpep created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY This allow the phabricator extension to read arc config files to auto-configure url, token and callsign. We use it as a fallback when phabricator.url or phabricator.callsign aren't defined. This allow to configure conduit_uri and repository.callsign in a tracked .arcconfig json file in the root of the repository, so users having lot of small repositories in phabricator doesn't need to configure .hg/hgrc after a fresh clone. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5299 AFFECTED FILES hgext/phabricator.py CHANGE DETAILS diff --git a/hgext/phabricator.py b/hgext/phabricator.py --- a/hgext/phabricator.py +++ b/hgext/phabricator.py @@ -37,14 +37,18 @@ # API token. Get it from https://$HOST/conduit/login/ example.phabtoken = cli- + +As a fallback, read config from arc config files (.arcconfig, ~/.arcrc and +/etc/arcconfig) """ from __future__ import absolute_import import itertools import json import operator import re +import os from mercurial.node import bin, nullid from mercurial.i18n import _ @@ -167,16 +171,51 @@ process(b'', params) return util.urlreq.urlencode(flatparams) +def readarcconfig(repo): +"""Return url, token, callsign read from arcanist config files + +This read and merge content of /etc/arcconfig, ~/.arcrc and .arconfig. +""" +if os.name == 'nt': +paths = [ +os.path.join(os.environ['ProgramData'], + 'Phabricator', + 'Arcanist', + 'config'), +os.path.join(os.environ['AppData'], '.arcrc'), +] +else: +paths = [ +os.path.join('/etc', 'arcconfig'), +os.path.join(os.path.expanduser('~'), '.arcrc'), +os.path.join(repo.root, '.arcconfig'), +] +config = {} +for path in paths: +if os.path.exists(path): +with open(path, 'rb') as f: +config.update(json.load(f)) +callsign = config.get('repository.callsign') +conduit_uri = config.get('conduit_uri', config.get('config', {}).get('default')) +if conduit_uri is not None: +token = config.get('hosts', {}).get(conduit_uri, {}).get('token') +url = conduit_uri.rstrip('/api/') +return url, token, callsign + def readurltoken(repo): """return conduit url, token and make sure they exist -Currently read from [auth] config section. In the future, it might -make sense to read from .arcconfig and .arcrc as well. +Currently read from [auth] config section and fallback to reading arc +config files. """ url = repo.ui.config(b'phabricator', b'url') if not url: -raise error.Abort(_(b'config %s.%s is required') - % (b'phabricator', b'url')) +url, token, __ = readarcconfig(repo) +if not url or not token: +raise error.Abort(_(b'unable to read phabricator conduit url and ' +b'token from config %s.%s or from arc config ' +b'files') % (b'phabricator', b'url')) +return url, token res = httpconnectionmod.readauthforuri(repo.ui, url, util.url(url).user) token = None @@ -241,7 +280,9 @@ return repophid callsign = repo.ui.config(b'phabricator', b'callsign') if not callsign: -return None +__, __, callsign = readarcconfig(repo) +if not callsign: +return callsign query = callconduit(repo, b'diffusion.repository.search', {b'constraints': {b'callsigns': [callsign]}}) if len(query[r'data']) == 0: To: philpep, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel