D5299: phabricator: fallback reading arcanist config files

[philpep (Philippe Pepiot)]

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

[durin42 (Augie Fackler)]

durin42 requested changes to this revision.
durin42 added a comment.
This revision now requires changes to proceed.


  I see at least one outstanding comment that points out a typo. I'm generally 
in favor of this though, so please let us know when it's time to take another 
look.

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

[Kwan (Ian Moody)]

Kwan added inline comments.

INLINE COMMENTS

> philpep wrote in phabricator.py:175
> I'd be nice to cache the result of `readarcconfig` but I don't known how to 
> implement this, any suggestion ?

Looks like annotating it with `@util.cachefunc` is enough?

> Kwan wrote in phabricator.py:217-222
> Something I realised with some testing is that this requires both .arcconfig 
> and .arcrc, one can't rely on the repo's .arcconfig for the repo settings 
> while using .hg/hgrc for the auth settings (or use hgrc for phab url with 
> .arcrc for auth).  Maybe it could instead do the following:
> 
>   hgrcurl = repo.ui.config(b'phabricator', b'url')
>   arcurl, arctoken, __ = readarcconfig(repo)
>   url = hgrcurl or arcurl
>   if not url:
>   [etc...]
>   
>   res = [etc...]
>   hgrctoken = None
>   
>   if res:
>   [etc...]
>   hgrctoken = auth.get(b'token')
>   token = hgrctoken or arctoken
>   if not token:
>   [etc...]
> 
> That way any mix of settings in hgrc or .arcconfig/.arcrc would work, with 
> hgrc having precedence.

Although, this wouldn't work with changing phab instances in hgrc, and using 
.arcrc for the token, since `readarcconfig` would return the  token for the 
original URL.  An alternative would be having `readarcconfig` return a dict 
mapping hosts to tokens (with '/api/' stripped from the hosts), and then 
`readurltoken` does `token = hgrctoken or arctokens.get(url)`.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5299

To: philpep, #hg-reviewers
Cc: 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

[Kwan (Ian Moody)]

Kwan added inline comments.

INLINE COMMENTS

> Kwan wrote in phabricator.py:201
> I think you could do
> 
>   url = util.url(conduit_uri)
>   url.path = None
>   url = b'%s' % url
> 
> Also I think you need to check for `phabricator.uri` first, then fall back to 
> `conduit_uri`, see change to phabricator's .arcconfig 
> 
>  and mozilla-central's current one 
>  (and 
> perhaps rename conduit_uri variable appropriately)

Oh, and the `config.get('hosts', {}).get(uri)` would have to use a uri with 
`.path = b'api/'`

> phabricator.py:193
> +paths = [
> +vfsmod.vfs('/etc').join('.arconfig'),
> +os.path.expanduser('~/.arcrc'),

Typo: should be `arcconfig` (two 'c's).
Also no dot for the "/etc/" one according to the docs

> phabricator.py:217-222
> +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

Something I realised with some testing is that this requires both .arcconfig 
and .arcrc, one can't rely on the repo's .arcconfig for the repo settings while 
using .hg/hgrc for the auth settings (or use hgrc for phab url with .arcrc for 
auth).  Maybe it could instead do the following:

  hgrcurl = repo.ui.config(b'phabricator', b'url')
  arcurl, arctoken, __ = readarcconfig(repo)
  url = hgrcurl or arcurl
  if not url:
  [etc...]
  
  res = [etc...]
  hgrctoken = None
  
  if res:
  [etc...]
  hgrctoken = auth.get(b'token')
  token = hgrctoken or arctoken
  if not token:
  [etc...]

That way any mix of settings in hgrc or .arcconfig/.arcrc would work, with hgrc 
having precedence.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5299

To: philpep, #hg-reviewers
Cc: 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

[Kwan (Ian Moody)]

Kwan added inline comments.

INLINE COMMENTS

> philpep wrote in phabricator.py:201
> 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 ?

I think you could do

  url = util.url(conduit_uri)
  url.path = None
  url = b'%s' % url

Also I think you need to check for `phabricator.uri` first, then fall back to 
`conduit_uri`, see change to phabricator's .arcconfig 

 and mozilla-central's current one 
 (and 
perhaps rename conduit_uri variable appropriately)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5299

To: philpep, #hg-reviewers
Cc: 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 (Philippe Pepiot)]

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 (Philippe Pepiot)]

philpep updated this revision to Diff 14253.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5299?vs=14175=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

[mharbison72 (Matt Harbison)]

mharbison72 added inline comments.

INLINE COMMENTS

> phabricator.py:187
> +paths = [
> +vfsmod.vfs(encoding.encoding['ProgramData']).join(
> + 'Phabricator', 'Arcanist', 'config'),

s/.encoding/.environ/ ?

> phabricator.py:200
> +if vfsmod.vfs(path).exists():
> +with open(path, 'rb') as f:
> +config.update(json.load(f))

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.

> phabricator.py:290
> +if not callsign:
> +return callsign
>  query = callconduit(repo, b'diffusion.repository.search',

It might be clearer to return None, since the function is to fetch the repoid.

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 (Philippe Pepiot)]

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 (Philippe Pepiot)]

philpep updated this revision to Diff 14175.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5299?vs=14174=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 (Philippe Pepiot)]

philpep updated this revision to Diff 14174.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5299?vs=12590=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

[mharbison72 (Matt Harbison)]

mharbison72 added inline comments.

INLINE COMMENTS

> phabricator.py:179
> +"""
> +if os.name == 'nt':
> +paths = [

pycompat.iswindows

> phabricator.py:181
> +paths = [
> +os.path.join(os.environ['ProgramData'],
> + 'Phabricator',

I think there’s something for environment variables in encoding (or maybe 
procutil- I don’t have the code in front of me).

Also, is there a way to use vfs here?  I think we’re trying to avoid direct 
os.path usage.

https://www.mercurial-scm.org/wiki/WindowsUTF8Plan

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 (Philippe Pepiot)]

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 (Philippe Pepiot)]

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 (Philippe Pepiot)]

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