Page MenuHomePhabricator

localrepo: load the share source .hg/hgrc also in share-safe mode (API)
Needs ReviewPublic

Authored by pulkit on Jun 24 2020, 10:31 AM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Summary

The second part of the Share Safe Plan is to share source repo config also.
This patch adds logic to load the source repo .hg/hgrc if we are in share safe
mode. On unshare, we copy and prepend source config to current repo so that
config which was shared is persisted.

A test is added to show that now if we enable a hook on the source repo, that
also runs on the shared repositories.

API change as a new optional argument sharedvfs added to localrepo.loadhgrc()

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Jun 24 2020, 10:31 AM
pulkit updated this revision to Diff 21742.Jul 1 2020, 6:00 AM
pulkit edited the summary of this revision. (Show Details)Jul 2 2020, 7:16 AM
pulkit updated this revision to Diff 21752.

Something in this stack (that I didn't push) breaks the phabricator extension. See phabricator.py line 170:

@eh.wrapfunction(localrepo, "loadhgrc")
def _loadhgrc(orig, ui, wdirvfs, hgvfs, requirements):

So you'll need to do....something there. Maybe look for other wrappers in extensions too while you're there...

marmoute requested changes to this revision.Wed, Jul 15, 2:11 PM
marmoute added a subscriber: marmoute.

I am fine with taking this root (reading source hgrc and adding a nonsharedrc) as I think it is a better experience overall. However, we have to be prepared to drop a large backward compatibility breaking notice when we turn sharesafe on by default, because people relying on the config to not be shared might be up for a surprise.

tests/test-share-safe.t
136

Can you also add test for the trusting logic ? making sure the other hgrc is untrusted as expected when relevant ?

160–177

There is apparently an unshare command. Sicne we read the source config, that source config need to be copied in the repository when unsharing. Can you add code and test for this?

This revision now requires changes to proceed.Wed, Jul 15, 2:11 PM
pulkit retitled this revision from localrepo: load the share source .hg/hgrc also in share-safe mode to localrepo: load the share source .hg/hgrc also in share-safe mode (API).Mon, Jul 20, 8:15 AM
pulkit edited the summary of this revision. (Show Details)
pulkit updated this revision to Diff 21977.
marmoute accepted this revision.EditedMon, Jul 20, 1:18 PM

I would be more confident if we get some explicit test about trust. However unshare is now working.

pulkit added a comment.EditedMon, Jul 20, 1:44 PM

I would be more confident if we get some explicit test about trust. However unshare is not working.

There is an explicit test about trust, kindly check inline comments, mentioned you.

Didn't understand the second part, what's not working?

tests/test-share-safe.t
155

@marmoute test about trust

pulkit updated this revision to Diff 22002.Tue, Jul 21, 9:42 AM
marmoute accepted this revision.Tue, Jul 21, 9:47 AM
marmoute added inline comments.
tests/test-share-safe.t
155

Ha greate, I missed this whil read it :-)

pulkit updated this revision to Diff 22071.Thu, Jul 23, 10:53 AM
pulkit updated this revision to Diff 22223.Mon, Aug 3, 9:12 AM