phabricator: fallback reading arcanist config files
Needs ReviewPublic

Authored by philpep on Nov 23 2018, 12:12 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
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.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped
philpep created this revision.Nov 23 2018, 12:12 PM
philpep added inline comments.Nov 23 2018, 12:15 PM
hgext/phabricator.py
207

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 ?

philpep added inline comments.Nov 23 2018, 12:16 PM
hgext/phabricator.py
181

I'd be nice to cache the result of readarcconfig but I don't known how to implement this, any suggestion ?

mharbison72 added inline comments.
hgext/phabricator.py
185

pycompat.iswindows

187

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

philpep updated this revision to Diff 14174.Thu, Feb 21, 1:08 PM
philpep updated this revision to Diff 14175.Thu, Feb 21, 1:16 PM
philpep marked 2 inline comments as done.Thu, Feb 21, 1:19 PM

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!

mharbison72 added inline comments.Fri, Feb 22, 12:59 PM
hgext/phabricator.py
187

s/.encoding/.environ/ ?

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.

290

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

philpep updated this revision to Diff 14253.Wed, Feb 27, 5:48 AM
philpep marked 3 inline comments as done.Wed, Feb 27, 5:49 AM
philpep added inline comments.
hgext/phabricator.py
187

Woops... Thanks for catching this!

200

Yes, done.

290

Indeed, I fixed this.

philpep marked 4 inline comments as done.Mon, Mar 4, 5:56 PM