Patchwork [2,of,4] dispatch: load shared source repository config in share-safe mode

login
register
mail settings
Submitter Pulkit Goyal
Date Oct. 16, 2020, 7:56 a.m.
Message ID <03bb891ad7fffc564b5a.1602834992@workspace>
Download mbox | patch
Permalink /patch/47483/
State New
Headers show

Comments

Pulkit Goyal - Oct. 16, 2020, 7:56 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1600435358 -19800
#      Fri Sep 18 18:52:38 2020 +0530
# Node ID 03bb891ad7fffc564b5af73f8887c41f4eb0a7f3
# Parent  90e0e681dc5b94b1636417b7f5da75762a88572f
# EXP-Topic share-safe
dispatch: load shared source repository config in share-safe mode

It seems to me now that there are two steps when config is loaded:

1) on dispatch
2) repository object creation

Recent patches added functionality that there can be shares in share-safe mode
where config of the source repository is shared with the the shares. However we
missed adding logic to read the source config on dispatch. This leads to
extensions not being loaded on dispatch and hence extensions command not being
recognized.

This patch fixes it by reading the shared source config on dispatch.

Differential Revision: https://phab.mercurial-scm.org/D9047
Yuya Nishihara - Oct. 16, 2020, 10:48 a.m.
On Fri, 16 Oct 2020 13:26:32 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1600435358 -19800
> #      Fri Sep 18 18:52:38 2020 +0530
> # Node ID 03bb891ad7fffc564b5af73f8887c41f4eb0a7f3
> # Parent  90e0e681dc5b94b1636417b7f5da75762a88572f
> # EXP-Topic share-safe
> dispatch: load shared source repository config in share-safe mode

> +def _readsharedsourceconfig(ui, path):
> +    """if the current repository is shared one, this tries to read
> +    .hg/hgrc of shared source if we are in share-safe mode
> +
> +    Config read is loaded into the ui object passed
> +
> +    This should be called before reading .hg/hgrc or the main repo
> +    as that overrides config set in shared source"""
> +    try:
> +        with open(os.path.join(path, b".hg", b"requires"), "rb") as fp:
> +            requirements = set(fp.read().splitlines())
> +            if not (
> +                requirementsmod.SHARESAFE_REQUIREMENT in requirements
> +                and requirementsmod.SHARED_REQUIREMENT in requirements
> +            ):
> +                return
> +            hgvfs = vfs.vfs(os.path.join(path, b".hg"))
> +            sharedvfs = localrepo._getsharedvfs(hgvfs, requirements)
> +            ui.readconfig(sharedvfs.join(b"hgrc"), path)

Maybe relative paths need to be resolved against shared wdir path, not the
path of the current repo.
Pulkit Goyal - Oct. 16, 2020, 12:38 p.m.
On Fri, Oct 16, 2020 at 4:27 PM Yuya Nishihara <yuya@tcha.org> wrote:
>
> On Fri, 16 Oct 2020 13:26:32 +0530, Pulkit Goyal wrote:
> > # HG changeset patch
> > # User Pulkit Goyal <7895pulkit@gmail.com>
> > # Date 1600435358 -19800
> > #      Fri Sep 18 18:52:38 2020 +0530
> > # Node ID 03bb891ad7fffc564b5af73f8887c41f4eb0a7f3
> > # Parent  90e0e681dc5b94b1636417b7f5da75762a88572f
> > # EXP-Topic share-safe
> > dispatch: load shared source repository config in share-safe mode
>
> > +def _readsharedsourceconfig(ui, path):
> > +    """if the current repository is shared one, this tries to read
> > +    .hg/hgrc of shared source if we are in share-safe mode
> > +
> > +    Config read is loaded into the ui object passed
> > +
> > +    This should be called before reading .hg/hgrc or the main repo
> > +    as that overrides config set in shared source"""
> > +    try:
> > +        with open(os.path.join(path, b".hg", b"requires"), "rb") as fp:
> > +            requirements = set(fp.read().splitlines())
> > +            if not (
> > +                requirementsmod.SHARESAFE_REQUIREMENT in requirements
> > +                and requirementsmod.SHARED_REQUIREMENT in requirements
> > +            ):
> > +                return
> > +            hgvfs = vfs.vfs(os.path.join(path, b".hg"))
> > +            sharedvfs = localrepo._getsharedvfs(hgvfs, requirements)
> > +            ui.readconfig(sharedvfs.join(b"hgrc"), path)
>
> Maybe relative paths need to be resolved against shared wdir path, not the
> path of the current repo.

Here, path of the current repo = shared wdir path.

Let's say we have a repo `main` and we share it as `sharedrepo`. This
patch implements reading of `.hg/hgrc` of main repository when hg is
run in `sharedrepo`.
Yuya Nishihara - Oct. 16, 2020, 1:09 p.m.
On Fri, 16 Oct 2020 18:08:04 +0530, Pulkit Goyal wrote:
> On Fri, Oct 16, 2020 at 4:27 PM Yuya Nishihara <yuya@tcha.org> wrote:
> >
> > On Fri, 16 Oct 2020 13:26:32 +0530, Pulkit Goyal wrote:
> > > # HG changeset patch
> > > # User Pulkit Goyal <7895pulkit@gmail.com>
> > > # Date 1600435358 -19800
> > > #      Fri Sep 18 18:52:38 2020 +0530
> > > # Node ID 03bb891ad7fffc564b5af73f8887c41f4eb0a7f3
> > > # Parent  90e0e681dc5b94b1636417b7f5da75762a88572f
> > > # EXP-Topic share-safe
> > > dispatch: load shared source repository config in share-safe mode
> >
> > > +def _readsharedsourceconfig(ui, path):
> > > +    """if the current repository is shared one, this tries to read
> > > +    .hg/hgrc of shared source if we are in share-safe mode
> > > +
> > > +    Config read is loaded into the ui object passed
> > > +
> > > +    This should be called before reading .hg/hgrc or the main repo
> > > +    as that overrides config set in shared source"""
> > > +    try:
> > > +        with open(os.path.join(path, b".hg", b"requires"), "rb") as fp:
> > > +            requirements = set(fp.read().splitlines())
> > > +            if not (
> > > +                requirementsmod.SHARESAFE_REQUIREMENT in requirements
> > > +                and requirementsmod.SHARED_REQUIREMENT in requirements
> > > +            ):
> > > +                return
> > > +            hgvfs = vfs.vfs(os.path.join(path, b".hg"))
> > > +            sharedvfs = localrepo._getsharedvfs(hgvfs, requirements)
> > > +            ui.readconfig(sharedvfs.join(b"hgrc"), path)
> >
> > Maybe relative paths need to be resolved against shared wdir path, not the
> > path of the current repo.
> 
> Here, path of the current repo = shared wdir path.
> 
> Let's say we have a repo `main` and we share it as `sharedrepo`. This
> patch implements reading of `.hg/hgrc` of main repository when hg is
> run in `sharedrepo`.

I meant paths in main_repo_root/.hg/hgrc should be resolved relative to
main_repo_root, not the current repo root.
Pulkit Goyal - Oct. 16, 2020, 1:24 p.m.
On Fri, Oct 16, 2020 at 6:48 PM Yuya Nishihara <yuya@tcha.org> wrote:
>
> On Fri, 16 Oct 2020 18:08:04 +0530, Pulkit Goyal wrote:
> > On Fri, Oct 16, 2020 at 4:27 PM Yuya Nishihara <yuya@tcha.org> wrote:
> > >
> > > On Fri, 16 Oct 2020 13:26:32 +0530, Pulkit Goyal wrote:
> > > > # HG changeset patch
> > > > # User Pulkit Goyal <7895pulkit@gmail.com>
> > > > # Date 1600435358 -19800
> > > > #      Fri Sep 18 18:52:38 2020 +0530
> > > > # Node ID 03bb891ad7fffc564b5af73f8887c41f4eb0a7f3
> > > > # Parent  90e0e681dc5b94b1636417b7f5da75762a88572f
> > > > # EXP-Topic share-safe
> > > > dispatch: load shared source repository config in share-safe mode
> > >
> > > > +def _readsharedsourceconfig(ui, path):
> > > > +    """if the current repository is shared one, this tries to read
> > > > +    .hg/hgrc of shared source if we are in share-safe mode
> > > > +
> > > > +    Config read is loaded into the ui object passed
> > > > +
> > > > +    This should be called before reading .hg/hgrc or the main repo
> > > > +    as that overrides config set in shared source"""
> > > > +    try:
> > > > +        with open(os.path.join(path, b".hg", b"requires"), "rb") as fp:
> > > > +            requirements = set(fp.read().splitlines())
> > > > +            if not (
> > > > +                requirementsmod.SHARESAFE_REQUIREMENT in requirements
> > > > +                and requirementsmod.SHARED_REQUIREMENT in requirements
> > > > +            ):
> > > > +                return
> > > > +            hgvfs = vfs.vfs(os.path.join(path, b".hg"))
> > > > +            sharedvfs = localrepo._getsharedvfs(hgvfs, requirements)
> > > > +            ui.readconfig(sharedvfs.join(b"hgrc"), path)
> > >
> > > Maybe relative paths need to be resolved against shared wdir path, not the
> > > path of the current repo.
> >
> > Here, path of the current repo = shared wdir path.
> >
> > Let's say we have a repo `main` and we share it as `sharedrepo`. This
> > patch implements reading of `.hg/hgrc` of main repository when hg is
> > run in `sharedrepo`.
>
> I meant paths in main_repo_root/.hg/hgrc should be resolved relative to
> main_repo_root, not the current repo root.

Isn't we are doing that by first getting the sharedvfs (which returns
main_repo_root/.hg) and join with that. Or you mean
`localrepo._getsharedvfs()` is not doing the correct thing?
Yuya Nishihara - Oct. 17, 2020, 3:44 a.m.
On Fri, 16 Oct 2020 18:54:38 +0530, Pulkit Goyal wrote:
> On Fri, Oct 16, 2020 at 6:48 PM Yuya Nishihara <yuya@tcha.org> wrote:
> >
> > On Fri, 16 Oct 2020 18:08:04 +0530, Pulkit Goyal wrote:
> > > On Fri, Oct 16, 2020 at 4:27 PM Yuya Nishihara <yuya@tcha.org> wrote:
> > > >
> > > > On Fri, 16 Oct 2020 13:26:32 +0530, Pulkit Goyal wrote:
> > > > > # HG changeset patch
> > > > > # User Pulkit Goyal <7895pulkit@gmail.com>
> > > > > # Date 1600435358 -19800
> > > > > #      Fri Sep 18 18:52:38 2020 +0530
> > > > > # Node ID 03bb891ad7fffc564b5af73f8887c41f4eb0a7f3
> > > > > # Parent  90e0e681dc5b94b1636417b7f5da75762a88572f
> > > > > # EXP-Topic share-safe
> > > > > dispatch: load shared source repository config in share-safe mode
> > > >
> > > > > +def _readsharedsourceconfig(ui, path):
> > > > > +    """if the current repository is shared one, this tries to read
> > > > > +    .hg/hgrc of shared source if we are in share-safe mode
> > > > > +
> > > > > +    Config read is loaded into the ui object passed
> > > > > +
> > > > > +    This should be called before reading .hg/hgrc or the main repo
> > > > > +    as that overrides config set in shared source"""
> > > > > +    try:
> > > > > +        with open(os.path.join(path, b".hg", b"requires"), "rb") as fp:
> > > > > +            requirements = set(fp.read().splitlines())
> > > > > +            if not (
> > > > > +                requirementsmod.SHARESAFE_REQUIREMENT in requirements
> > > > > +                and requirementsmod.SHARED_REQUIREMENT in requirements
> > > > > +            ):
> > > > > +                return
> > > > > +            hgvfs = vfs.vfs(os.path.join(path, b".hg"))
> > > > > +            sharedvfs = localrepo._getsharedvfs(hgvfs, requirements)
> > > > > +            ui.readconfig(sharedvfs.join(b"hgrc"), path)
> > > >
> > > > Maybe relative paths need to be resolved against shared wdir path, not the
> > > > path of the current repo.
> > >
> > > Here, path of the current repo = shared wdir path.
> > >
> > > Let's say we have a repo `main` and we share it as `sharedrepo`. This
> > > patch implements reading of `.hg/hgrc` of main repository when hg is
> > > run in `sharedrepo`.
> >
> > I meant paths in main_repo_root/.hg/hgrc should be resolved relative to
> > main_repo_root, not the current repo root.
> 
> Isn't we are doing that by first getting the sharedvfs (which returns
> main_repo_root/.hg) and join with that. Or you mean
> `localrepo._getsharedvfs()` is not doing the correct thing?

The path of the config file is correct, but its data are resolved against
the "path" of the current repo by ui.readconfig(..., root=path). Since the
config file is placed inside the main repo, relative paths in that config
file should be resolved as if the file were read in the main repo.
Pulkit Goyal - Nov. 20, 2020, 8:47 a.m.
On Sat, Oct 17, 2020 at 9:15 AM Yuya Nishihara <yuya@tcha.org> wrote:
>
> On Fri, 16 Oct 2020 18:54:38 +0530, Pulkit Goyal wrote:
> > On Fri, Oct 16, 2020 at 6:48 PM Yuya Nishihara <yuya@tcha.org> wrote:
> > >
> > > On Fri, 16 Oct 2020 18:08:04 +0530, Pulkit Goyal wrote:
> > > > On Fri, Oct 16, 2020 at 4:27 PM Yuya Nishihara <yuya@tcha.org> wrote:
> > > > >
> > > > > On Fri, 16 Oct 2020 13:26:32 +0530, Pulkit Goyal wrote:
> > > > > > # HG changeset patch
> > > > > > # User Pulkit Goyal <7895pulkit@gmail.com>
> > > > > > # Date 1600435358 -19800
> > > > > > #      Fri Sep 18 18:52:38 2020 +0530
> > > > > > # Node ID 03bb891ad7fffc564b5af73f8887c41f4eb0a7f3
> > > > > > # Parent  90e0e681dc5b94b1636417b7f5da75762a88572f
> > > > > > # EXP-Topic share-safe
> > > > > > dispatch: load shared source repository config in share-safe mode
> > > > >
> > > > > > +def _readsharedsourceconfig(ui, path):
> > > > > > +    """if the current repository is shared one, this tries to read
> > > > > > +    .hg/hgrc of shared source if we are in share-safe mode
> > > > > > +
> > > > > > +    Config read is loaded into the ui object passed
> > > > > > +
> > > > > > +    This should be called before reading .hg/hgrc or the main repo
> > > > > > +    as that overrides config set in shared source"""
> > > > > > +    try:
> > > > > > +        with open(os.path.join(path, b".hg", b"requires"), "rb") as fp:
> > > > > > +            requirements = set(fp.read().splitlines())
> > > > > > +            if not (
> > > > > > +                requirementsmod.SHARESAFE_REQUIREMENT in requirements
> > > > > > +                and requirementsmod.SHARED_REQUIREMENT in requirements
> > > > > > +            ):
> > > > > > +                return
> > > > > > +            hgvfs = vfs.vfs(os.path.join(path, b".hg"))
> > > > > > +            sharedvfs = localrepo._getsharedvfs(hgvfs, requirements)
> > > > > > +            ui.readconfig(sharedvfs.join(b"hgrc"), path)
> > > > >
> > > > > Maybe relative paths need to be resolved against shared wdir path, not the
> > > > > path of the current repo.
> > > >
> > > > Here, path of the current repo = shared wdir path.
> > > >
> > > > Let's say we have a repo `main` and we share it as `sharedrepo`. This
> > > > patch implements reading of `.hg/hgrc` of main repository when hg is
> > > > run in `sharedrepo`.
> > >
> > > I meant paths in main_repo_root/.hg/hgrc should be resolved relative to
> > > main_repo_root, not the current repo root.
> >
> > Isn't we are doing that by first getting the sharedvfs (which returns
> > main_repo_root/.hg) and join with that. Or you mean
> > `localrepo._getsharedvfs()` is not doing the correct thing?
>
> The path of the config file is correct, but its data are resolved against
> the "path" of the current repo by ui.readconfig(..., root=path). Since the
> config file is placed inside the main repo, relative paths in that config
> file should be resolved as if the file were read in the main repo.

Understood now, thank you! Will send a followup.

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -36,13 +36,16 @@  from . import (
     help,
     hg,
     hook,
+    localrepo,
     profiling,
     pycompat,
     rcutil,
     registrar,
+    requirements as requirementsmod,
     scmutil,
     ui as uimod,
     util,
+    vfs,
 )
 
 from .utils import (
@@ -941,6 +944,29 @@  def runcommand(lui, repo, cmd, fullargs,
     return ret
 
 
+def _readsharedsourceconfig(ui, path):
+    """if the current repository is shared one, this tries to read
+    .hg/hgrc of shared source if we are in share-safe mode
+
+    Config read is loaded into the ui object passed
+
+    This should be called before reading .hg/hgrc or the main repo
+    as that overrides config set in shared source"""
+    try:
+        with open(os.path.join(path, b".hg", b"requires"), "rb") as fp:
+            requirements = set(fp.read().splitlines())
+            if not (
+                requirementsmod.SHARESAFE_REQUIREMENT in requirements
+                and requirementsmod.SHARED_REQUIREMENT in requirements
+            ):
+                return
+            hgvfs = vfs.vfs(os.path.join(path, b".hg"))
+            sharedvfs = localrepo._getsharedvfs(hgvfs, requirements)
+            ui.readconfig(sharedvfs.join(b"hgrc"), path)
+    except IOError:
+        pass
+
+
 def _getlocal(ui, rpath, wd=None):
     """Return (path, local ui object) for the given target path.
 
@@ -961,12 +987,14 @@  def _getlocal(ui, rpath, wd=None):
     else:
         lui = ui.copy()
         if rcutil.use_repo_hgrc():
+            _readsharedsourceconfig(lui, path)
             lui.readconfig(os.path.join(path, b".hg", b"hgrc"), path)
 
     if rpath:
         path = lui.expandpath(rpath)
         lui = ui.copy()
         if rcutil.use_repo_hgrc():
+            _readsharedsourceconfig(lui, path)
             lui.readconfig(os.path.join(path, b".hg", b"hgrc"), path)
 
     return path, lui
diff --git a/tests/test-share-safe.t b/tests/test-share-safe.t
--- a/tests/test-share-safe.t
+++ b/tests/test-share-safe.t
@@ -102,21 +102,14 @@  Test that extensions of source repositor
   share
   $ hg extdiff -R ../source -p echo
 
-BROKEN: the command below does not work but debugextensions says that extension
+BROKEN: the command below will not work if config of shared source is not loaded
+on dispatch but debugextensions says that extension
 is loaded
   $ hg debugextensions
   extdiff
   share
 
-BROKEN: extdiff command should work here
   $ hg extdiff -p echo
-  hg: unknown command 'extdiff'
-  'extdiff' is provided by the following extension:
-  
-      extdiff       command to allow external programs to compare revisions
-  
-  (use 'hg help extensions' for information on enabling extensions)
-  [255]
 
 However, local .hg/hgrc should override the config set by share source