This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8633

share: introduce config option to store requires in .hg/store
ClosedPublic

Authored by pulkit on Jun 15 2020, 11:03 AM.

Details

Reviewers
durin42
marmoute
indygreg
Group Reviewers
hg-reviewers
Commits
rHGd252f51ab032: share: introduce config option to store requires in .hg/store
Summary

This introduces a config option which enabled stores the requirements on a
repository in store instead.

When enabled, .hg/requires will contain the share-safe requirement which
marks that the requirements are present in the store.
This is done so that repository requirements can be shared with shares made
using hg share command.

After this patch, hg share checks whether the source repository has
share-safe requirement, if yes, it does not copy the requirements.

Test for the new functionality is added and a test case in exitsing share tests
is also added.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Jun 15 2020, 11:03 AM
pulkit edited the summary of this revision. (Show Details)Jun 25 2020, 4:52 AM
pulkit updated this revision to Diff 21702.
pulkit updated this revision to Diff 21740.Jul 1 2020, 6:00 AM
marmoute requested changes to this revision.Jul 15 2020, 1:58 PM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
mercurial/localrepo.py
434

Until the semantic is fully implemented, this should be exp-sharesafe otherwise a non complying client could accessing without respecting the semantic.

569–586

This looks like a lot of duplicated code, am I missing something ? Could we do less duplication?

3520–3531

It looks like share-safe should requires store, can we make them dependent of each other.

This revision now requires changes to proceed.Jul 15 2020, 1:58 PM
pulkit marked 2 inline comments as done.Jul 17 2020, 10:24 AM
pulkit added inline comments.
mercurial/localrepo.py
434

Done.

569–586

Refactored in next patch.

3520–3531

Does not share extension implies that store is present?

marmoute added inline comments.Jul 20 2020, 1:05 PM
mercurial/localrepo.py
569–586

I see this is done in D8655 thanks !

3520–3531

Maybe, can you check ?

In all cases, I would rather make the safe-share requirement consistent from the start. Can we have some code either setup store automatically, or reject with some error (whichever solution you prefer).

pulkit updated this revision to Diff 22068.Jul 23 2020, 10:52 AM
pulkit updated this revision to Diff 22171.Aug 1 2020, 10:36 AM
indygreg added a subscriber: indygreg.Aug 5 2020, 9:08 PM

I need to think about the implications of this change a bit more: the requires file plays an important part in how clients open repositories and there are significant backwards and forwards compatibility concerns.

One immediate request is to document semantics in mercurial/helptext/internals/requirements.txt as part of this change.

FWIW I thought I had typed up an internals document on how repositories are opened. But maybe I never submitted it for review or maybe it never landed.

indygreg added a comment.Aug 6 2020, 9:43 PM

I left a handful of comments.

Changing repo opening semantics and requirements handling is a bit scary.

I'm generally in favor of separating working directory requirements from non-working directory requirements so I think this patch is a step in the right direction. There are a few alternative implementations worth considering. (Although I'm unsure if they are superior.)

  1. Write the new requirements file in the .hg directory as .hg/store-requires (or similar). Conceptually, we're talking about working directory and backend requirements. The "store" is just today's layout for storing backend data [which can be shared between working directories].
  2. Write a new requirements file for working directory specific requirements.
mercurial/localrepo.py
577

I'm reasonably certain this logic is duplicated from somewhere. And opening shared repos is quirky enough that I'd feel better if there was a helper function for resolving the store vfs given a vfs for a .hg directory.

585

The swalloing of ENOENT doesn't sit well with me.

The repo opening semantics are such that .hg/requires is read first and all subsequent repo opening is dictated by what is found inside that file.

I think that a .hg/store/requires file presence should be directly indicated via an entry in .hg/requires - say store-requires - and if that entry is present, the file must be present: a missing file would be an error.

So I think the request here would be to rename the requirement to [exp-]store-requires instead of [exp-]share-safe. If we wanted to add additional functionality governing how shared repos are opened, we should consider a separate requirement for that, independent of the one that indicates if a .hg/store/requires exists.

Put another way, we want repo opening semantics to be strictly defined with little room for interpretation. And we want requires entries to be narrowly scoped so there isn't ambiguity about what they mean.

3351

It's really weird that this requirement has share in its name but it isn't dependent on being a shared repo.

3531

This whole block needs more comments because what it is trying to accomplish is important and nuanced.

I would also consider rewriting the code so that we populate variables indicating the set of requirements to populate in .hg/requires and .hg/store/requires. That way we have at most 2 calls to writerequires(), which I think will improve readability.

3531

What this is doing is blindly moving all requirements except SHARESAFE_REQUIREMENT to .hg/store/requires. The movement of most requirements there makes sense because they govern the "store." However, there are requirements like exp-sparse which I believe only concern the working directory and should not be in the "store."

If we want to go the approach of moving store-specific requirements into .hg/store/requires, I think we need to annotate whether requirements belong to the store or the non-store and write/move requirements accordingly.

mercurial/scmutil.py
1498

The presence of this conditional here feels a bit weird to me: I feel like it might be better if we explicitly passed in 2 sets of requirements to read and this function were "dumber."

1499

Writing of the requirements file should be extremely rare. I don't think we need this lock here.

mercurial/store.py
375–376

Bonus points if you refactor this to be a proper data structure as part of this series. A space delimited list/set makes no sense to me.

458

Wait - why was requires here before?

marmoute added a comment.Aug 7 2020, 4:24 AM
In D8633#133117, @indygreg wrote:

I left a handful of comments.
Changing repo opening semantics and requirements handling is a bit scary.
I'm generally in favor of separating working directory requirements from non-working directory requirements so I think this patch is a step in the right direction. There are a few alternative implementations worth considering. (Although I'm unsure if they are superior.)

  1. Write the new requirements file in the .hg directory as .hg/store-requires (or similar). Conceptually, we're talking about working directory and backend requirements. The "store" is just today's layout for storing backend data [which can be shared between working directories].
  2. Write a new requirements file for working directory specific requirements.

I would advocate for having a file for working-directory specific requirements. It is safer to assume all requirements are store specific unless explicitly specified otherwise.

marmoute added inline comments.Aug 7 2020, 4:40 AM
mercurial/localrepo.py
577

I had the same comment, and it turns out @pulkit is cleaning the duplication in later patches.

585

The swalloing of ENOENT doesn't sit well with me.

The repo opening semantics are such that .hg/requires is read first and all subsequent repo opening is dictated by what is found inside that file.

I think that a .hg/store/requires file presence should be directly indicated via an entry in .hg/requires - say store-requires - and if that entry is present, the file must be present: a missing file would be an error.

+1 for having some explicit error. If we go the route of using a wc specific files, we should be strict on having it too.

I don't have a strong opinion on having the requires file in the .hg/store/ directory yet. It seems cleaner/clearer to do so.

585

So I think the request here would be to rename the requirement to [exp-]store-requires instead of [exp-]share-safe. If we wanted to add additional functionality governing how shared repos are opened, we should consider a separate requirement for that, independent of the one that indicates if a .hg/store/requires exists.

Having a galaxy on interdependent requirements can become a nightmare, because we either have to start dealing with all the corner case of some being enabled without the other or to add extra logic to make sure they are not being used without the other (and error out on bad configuration). So having simple requirement adding a consistent improvement seems like a better idea.

The share-safe requirement comes with other semantic beside the store one, for example regarding the reading of the configuration. So store-requires would be too narrow.

3351

The requirements is named "share-safe" because after this change, the repository is safe to be shared. A repository do not need to be shared to "share-safe" the same way a watch does not need to be in the submerged to be water-proof.

3531

Defaulting to "requirements is a store requirement" and explicitly flagging requirement that are working copy specific seems the safest.

mercurial/scmutil.py
1499

I agreed that the requirements should be written at repository creation time (or upgrade time) and that is all. Maybe we should update the acquiring of the lock with some code checking we are actually holding a lock while doing this?

The upgrade usecase is a good argument in favor of moving the requirement file inside the store directory.

mercurial/store.py
375–376

I can add an extra bonus point to the stake :-)

pulkit marked 3 inline comments as done.Aug 7 2020, 8:59 AM
pulkit planned changes to this revision.
In D8633#133117, @indygreg wrote:

I left a handful of comments.
Changing repo opening semantics and requirements handling is a bit scary.
I'm generally in favor of separating working directory requirements from non-working directory requirements so I think this patch is a step in the right direction. There are a few alternative implementations worth considering. (Although I'm unsure if they are superior.)

  1. Write the new requirements file in the .hg directory as .hg/store-requires (or similar). Conceptually, we're talking about working directory and backend requirements. The "store" is just today's layout for storing backend data [which can be shared between working directories].
  2. Write a new requirements file for working directory specific requirements.

Hi, thanks a lot for the detailed review. I think we can have .hg/requires as working directory specific and .hg/store/requires as store requirements.

I have not yet implemented all the fixes you mentioned. Hence marking this as planned changes. However I have added some parents patches which should be good to review.

pulkit updated this revision to Diff 22381.Aug 10 2020, 11:26 AM
pulkit updated this revision to Diff 22397.Aug 11 2020, 5:06 AM
pulkit marked 2 inline comments as done.Aug 11 2020, 5:25 AM
In D8633#133201, @pulkit wrote:
In D8633#133117, @indygreg wrote:

I left a handful of comments.
Changing repo opening semantics and requirements handling is a bit scary.
I'm generally in favor of separating working directory requirements from non-working directory requirements so I think this patch is a step in the right direction. There are a few alternative implementations worth considering. (Although I'm unsure if they are superior.)

  1. Write the new requirements file in the .hg directory as .hg/store-requires (or similar). Conceptually, we're talking about working directory and backend requirements. The "store" is just today's layout for storing backend data [which can be shared between working directories].
  2. Write a new requirements file for working directory specific requirements.

Hi, thanks a lot for the detailed review. I think we can have .hg/requires as working directory specific and .hg/store/requires as store requirements.
I have not yet implemented all the fixes you mentioned. Hence marking this as planned changes. However I have added some parents patches which should be good to review.

@indygreg I think I have taken care of all fixes you suggested except one (the finegrained store-requires one). This patch should be good for an another look now.

mercurial/localrepo.py
585

Fixed the swallowing of ENOENT.

About a fine grained store-requires requirement, I am not sure what's the best way out.

Right now exp-share-safe implies two things, requirements in store and source .hgrc be shared. We definitely can have dedicated store-requires but that introduces a spider of inter-dependent requirements.

Does exp-share-safe implies store-requires or need it? etc.

For now, I have not changed anything on this front in this patch.

3351

What @marmoute said, the requirement is named share-safe because after this change, the repository is safe to be shared.

3531

The whole code block is now much simpler, thanks to filterrequirements introduced in previous patches.

mercurial/scmutil.py
1498

This code is much cleaner now after introducing filterrequirements in previous patches.

mercurial/store.py
458

A line of code dating back to 2008. Removing it does not make any tests fail. Will send a patch for it separately.

pulkit marked 2 inline comments as done.Aug 11 2020, 5:27 AM
In D8633#133021, @indygreg wrote:

I need to think about the implications of this change a bit more: the requires file plays an important part in how clients open repositories and there are significant backwards and forwards compatibility concerns.
One immediate request is to document semantics in mercurial/helptext/internals/requirements.txt as part of this change.

Since this patch does not implement all the things which are covered by this requirement, I appended a patch (D8914) at end of series for this.

FWIW I thought I had typed up an internals document on how repositories are opened. But maybe I never submitted it for review or maybe it never landed.

pulkit updated this revision to Diff 22444.Aug 25 2020, 7:39 AM
indygreg accepted this revision.Aug 25 2020, 9:33 PM

I'm approving this as an experimental quality feature. I think the technical direction is sound and the splitting of requirements corrects some long-standing soundness issues and will lead to a better end-user experience for shared repositories.

Before the experimental label is ripped off, I think we should consider:

  • Whether store requirement belongs in .hg/requires or .hg/store/requires. It's weird for it to be in .hg/store since that requirement essentially says files are in store/.
  • The name of the requirement having share in it still doesn't sit well with me. The thing the requirement is saying is that additional requirements exist in store/requires. A side-effect of that is that repository store sharing is more robust. I'd rather the .hg/requires file contain storerequires. If .hg/requires is using a shared store, it can have sharedstorerequires to indicate that special variant. But we could also use storerequires in this case as well: it all depends how specific we want to be with requires entries! Another way of looking at this is that it is weird an unshared repository has a requirement with share in it. That's a good indicator the naming is off!
mercurial/localrepo.py
557

As written, this code will accept the share safe requirement without the store requirement. The store requirement should be mandatory and the share safe requirement without store is nonsensical.

It would be nice if there were code somewhere that rejected this invalid combination of requirements. But the chances of this happening should be small and it is probably OK to live without it in the repo opening code. However, we should look for it when writing the requirement, since it would be possible to coerce config options to produce such a repository.

562

I'm not sure cacheaudited provides any benefit for a single file read. But it should be harmless since we throw away the vfs instance after a single use.

3394

I'm on the fence about whether we should drop or abort here. Either way is probably fine.

3528

sharedrepo wants to be converted to a constant.

indygreg added a comment.Aug 25 2020, 9:48 PM

I was going to push this but there's a test failure:

--- /home/gps/src/hg-committed/tests/test-remotefilelog-share.t
+++ /home/gps/src/hg-committed/tests/test-remotefilelog-share.t#safe.err
@@ -28,6 +28,7 @@


   $ hgcloneshallow ssh://user@dummy/master source --noupdate -q
+  devel-warn: write with no lock: "requires" at: /home/gps/src/hg-committed/mercurial/scmutil.py:1505 (writerequires)
   $ hg share source dest
   updating working directory
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
marmoute accepted this revision.Aug 26 2020, 3:16 AM
In D8633#133933, @indygreg wrote:

I'm approving this as an experimental quality feature. I think the technical direction is sound and the splitting of requirements corrects some long-standing soundness issues and will lead to a better end-user experience for shared repositories.
Before the experimental label is ripped off, I think we should consider:

  • Whether store requirement belongs in .hg/requires or .hg/store/requires. It's weird for it to be in .hg/store since that requirement essentially says files are in store/.

the store requirements is clearly something that needs to be shared between the source repo and the potential shares, so this argue for having it in .hg/store/requires, however as you point out if we are reading .hg/store/requirements we obviously already aware that we have a store. Maybe sharesafe should imply store? Or maybe that special case is not special enough and we can live with the funny inconsistency?

  • The name of the requirement having share in it still doesn't sit well with me. The thing the requirement is saying is that additional requirements exist in store/requires.

The requirements also change the configuration reading logic so that the configuration from the source repository is also read by the shares. So the sharesafe requirement is really about fixing various issue that were making repository unsafe to share. And use multiple behavior/layout change to achieve this.

Another way of looking at this is that it is weird an unshared repository has a requirement with share in it. That's a good indicator the naming is off!

I really don't understand your reasoning here. as I pointed before they are countless example of such naming in real life. For example, waterproof item does not imply they are (or will) submerged, fireproof item does not imply they are (or will) be on fire and military grade hardware does not requires you to be in the military to use them. So having a repository to be "share safe" does not requires it to be shared to me. Can you elaborate on why you find that naming suspicious ?

This revision is now accepted and ready to land.Aug 26 2020, 3:16 AM
pulkit marked 2 inline comments as done.Aug 26 2020, 4:44 AM
pulkit added inline comments.
mercurial/localrepo.py
557

The SHARESAFE_REQUIREMENT implies that store is present. There are two ways SHARESAFE_REQUIREMENT can be introduced:

  1. Creating new repos with config option enabled
  2. Using upgrade

For 1, we check that store is present. Refer checkrequirementscompat() a bit down in this file.

2 is not yet implemented and we will add the check there too when implemented.

So until the user is not manually editing the requires file, we can be sure that if SHARESAFE_REQUIREMENT is present, store will be present.

However, we should look for it when writing the requirement, since it would be possible to
coerce config options to produce such a repository.

Definitely, this patch has a test case for that in test-share.t.

562

Seems like copy-paste leftover, removed it.

3394

I am bit more inclined on aborting here but realized that this whole needs a separate discussion as unfortunately we didn't have such handling yet.

3528

This is bit messy here. sharedrepo here is createopts key rather than a requirement name.

pulkit marked 2 inline comments as done.Aug 26 2020, 6:05 AM
pulkit updated this revision to Diff 22450.
pulkit added a comment.Aug 26 2020, 6:06 AM
In D8633#133939, @indygreg wrote:

I was going to push this but there's a test failure:

--- /home/gps/src/hg-committed/tests/test-remotefilelog-share.t
+++ /home/gps/src/hg-committed/tests/test-remotefilelog-share.t#safe.err
@@ -28,6 +28,7 @@
   $ hgcloneshallow ssh://user@dummy/master source --noupdate -q
+  devel-warn: write with no lock: "requires" at: /home/gps/src/hg-committed/mercurial/scmutil.py:1505 (writerequires)
   $ hg share source dest
   updating working directory
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved

Oops, turns out shallow-cloning in remotefilelog does not acquire a lock. I added D8952 in start of series to acquire lock while writing requires in shallow-cloning in remotefilelog.

pulkit updated this revision to Diff 22654.Sep 16 2020, 7:56 AM
indygreg accepted this revision.Sep 17 2020, 9:13 PM

Revision Contents
Changeset List

Diff 22691

mercurial/configitems.py

b'exp-use-copies-side-data-changeset', b'exp-use-copies-side-data-changeset',
default=False, default=False,
experimental=True, experimental=True,
) )
coreconfigitem( coreconfigitem(
b'format', b'exp-use-side-data', default=False, experimental=True, b'format', b'exp-use-side-data', default=False, experimental=True,
) )
coreconfigitem( coreconfigitem(
b'format', b'exp-share-safe', default=False, experimental=True,
)
coreconfigitem(
b'format', b'internal-phase', default=False, experimental=True, b'format', b'internal-phase', default=False, experimental=True,
) )
coreconfigitem( coreconfigitem(
b'fsmonitor', b'warn_when_unused', default=True, b'fsmonitor', b'warn_when_unused', default=True,
) )
coreconfigitem( coreconfigitem(
b'fsmonitor', b'warn_update_file_count', default=50000, b'fsmonitor', b'warn_update_file_count', default=50000,
) )

mercurial/localrepo.py

return changegroup.makechangegroup(self._repo, outgoing, b'01', source) return changegroup.makechangegroup(self._repo, outgoing, b'01', source)
# End of baselegacywirecommands interface. # End of baselegacywirecommands interface.
# Functions receiving (ui, features) that extensions can register to impact # Functions receiving (ui, features) that extensions can register to impact
# the ability to load repositories with custom requirements. Only # the ability to load repositories with custom requirements. Only
# functions defined in loaded extensions are called. # functions defined in loaded extensions are called.
# #
marmouteUnsubmitted
Done

Until the semantic is fully implemented, this should be exp-sharesafe otherwise a non complying client could accessing without respecting the semantic.

marmoute: Until the semantic is fully implemented, this should be `exp-sharesafe` otherwise a non…
pulkitAuthorUnsubmitted
Done

Done.

pulkit: Done.
# The function receives a set of requirement strings that the repository # The function receives a set of requirement strings that the repository
# is capable of opening. Functions will typically add elements to the # is capable of opening. Functions will typically add elements to the
# set to reflect that the extension knows how to handle that requirements. # set to reflect that the extension knows how to handle that requirements.
featuresetupfuncs = set() featuresetupfuncs = set()
def _getsharedvfs(hgvfs, requirements): def _getsharedvfs(hgvfs, requirements):
""" returns the vfs object pointing to root of shared source """ returns the vfs object pointing to root of shared source
# Can be raised on Python 3.8 when path is invalid. # Can be raised on Python 3.8 when path is invalid.
raise error.Abort( raise error.Abort(
_(b'invalid path %s: %s') % (path, pycompat.bytestr(e)) _(b'invalid path %s: %s') % (path, pycompat.bytestr(e))
) )
raise error.RepoError(_(b'repository %s not found') % path) raise error.RepoError(_(b'repository %s not found') % path)
requirements = _readrequires(hgvfs, True) requirements = _readrequires(hgvfs, True)
shared = (
requirementsmod.SHARED_REQUIREMENT in requirements
or requirementsmod.RELATIVE_SHARED_REQUIREMENT in requirements
)
if shared:
sharedvfs = _getsharedvfs(hgvfs, requirements)
# if .hg/requires contains the sharesafe requirement, it means
# there exists a `.hg/store/requires` too and we should read it
# NOTE: presence of SHARESAFE_REQUIREMENT imply that store requirement
indygregUnsubmitted
Done

As written, this code will accept the share safe requirement without the store requirement. The store requirement should be mandatory and the share safe requirement without store is nonsensical.

It would be nice if there were code somewhere that rejected this invalid combination of requirements. But the chances of this happening should be small and it is probably OK to live without it in the repo opening code. However, we should look for it when writing the requirement, since it would be possible to coerce config options to produce such a repository.

indygreg: As written, this code will accept the share safe requirement without the `store` requirement.
pulkitAuthorUnsubmitted
Done

The SHARESAFE_REQUIREMENT implies that store is present. There are two ways SHARESAFE_REQUIREMENT can be introduced:

  1. Creating new repos with config option enabled
  2. Using upgrade

For 1, we check that store is present. Refer checkrequirementscompat() a bit down in this file.

2 is not yet implemented and we will add the check there too when implemented.

So until the user is not manually editing the requires file, we can be sure that if SHARESAFE_REQUIREMENT is present, store will be present.

However, we should look for it when writing the requirement, since it would be possible to
coerce config options to produce such a repository.

Definitely, this patch has a test case for that in test-share.t.

pulkit: The `SHARESAFE_REQUIREMENT` implies that `store` is present. There are two ways…
# is present. We never write SHARESAFE_REQUIREMENT for a repo if store
# is not present, refer checkrequirementscompat() for that
if requirementsmod.SHARESAFE_REQUIREMENT in requirements:
if shared:
# This is a shared repo
indygregUnsubmitted
Done

I'm not sure cacheaudited provides any benefit for a single file read. But it should be harmless since we throw away the vfs instance after a single use.

indygreg: I'm not sure `cacheaudited` provides any benefit for a single file read. But it should be…
pulkitAuthorUnsubmitted
Done

Seems like copy-paste leftover, removed it.

pulkit: Seems like copy-paste leftover, removed it.
storevfs = vfsmod.vfs(sharedvfs.join(b'store'))
else:
storevfs = vfsmod.vfs(hgvfs.join(b'store'))
requirements |= _readrequires(storevfs, False)
# The .hg/hgrc file may load extensions or contain config options # The .hg/hgrc file may load extensions or contain config options
# that influence repository construction. Attempt to load it and # that influence repository construction. Attempt to load it and
# process any new extensions that it may have pulled in. # process any new extensions that it may have pulled in.
if loadhgrc(ui, wdirvfs, hgvfs, requirements): if loadhgrc(ui, wdirvfs, hgvfs, requirements):
afterhgrcload(ui, wdirvfs, hgvfs, requirements) afterhgrcload(ui, wdirvfs, hgvfs, requirements)
extensions.loadall(ui) extensions.loadall(ui)
extensions.populateui(ui) extensions.populateui(ui)
# Set of module names of extensions loaded for this repository. # Set of module names of extensions loaded for this repository.
indygregUnsubmitted
Done

I'm reasonably certain this logic is duplicated from somewhere. And opening shared repos is quirky enough that I'd feel better if there was a helper function for resolving the store vfs given a vfs for a .hg directory.

indygreg: I'm reasonably certain this logic is duplicated from somewhere. And opening shared repos is…
marmouteUnsubmitted
Done

I had the same comment, and it turns out @pulkit is cleaning the duplication in later patches.

marmoute: I had the same comment, and it turns out @pulkit is cleaning the duplication in later patches.
extensionmodulenames = {m.__name__ for n, m in extensions.extensions(ui)} extensionmodulenames = {m.__name__ for n, m in extensions.extensions(ui)}
supportedrequirements = gathersupportedrequirements(ui) supportedrequirements = gathersupportedrequirements(ui)
# We first validate the requirements are known. # We first validate the requirements are known.
ensurerequirementsrecognized(requirements, supportedrequirements) ensurerequirementsrecognized(requirements, supportedrequirements)
# Then we validate that the known set is reasonable to use together. # Then we validate that the known set is reasonable to use together.
indygregUnsubmitted
Not Done

The swalloing of ENOENT doesn't sit well with me.

The repo opening semantics are such that .hg/requires is read first and all subsequent repo opening is dictated by what is found inside that file.

I think that a .hg/store/requires file presence should be directly indicated via an entry in .hg/requires - say store-requires - and if that entry is present, the file must be present: a missing file would be an error.

So I think the request here would be to rename the requirement to [exp-]store-requires instead of [exp-]share-safe. If we wanted to add additional functionality governing how shared repos are opened, we should consider a separate requirement for that, independent of the one that indicates if a .hg/store/requires exists.

Put another way, we want repo opening semantics to be strictly defined with little room for interpretation. And we want requires entries to be narrowly scoped so there isn't ambiguity about what they mean.

indygreg: The swalloing of `ENOENT` doesn't sit well with me. The repo opening semantics are such that `.
marmouteUnsubmitted
Not Done

The swalloing of ENOENT doesn't sit well with me.

The repo opening semantics are such that .hg/requires is read first and all subsequent repo opening is dictated by what is found inside that file.

I think that a .hg/store/requires file presence should be directly indicated via an entry in .hg/requires - say store-requires - and if that entry is present, the file must be present: a missing file would be an error.

+1 for having some explicit error. If we go the route of using a wc specific files, we should be strict on having it too.

I don't have a strong opinion on having the requires file in the .hg/store/ directory yet. It seems cleaner/clearer to do so.

marmoute: >The swalloing of ENOENT doesn't sit well with me. > The repo opening semantics are such that .
marmouteUnsubmitted
Not Done

So I think the request here would be to rename the requirement to [exp-]store-requires instead of [exp-]share-safe. If we wanted to add additional functionality governing how shared repos are opened, we should consider a separate requirement for that, independent of the one that indicates if a .hg/store/requires exists.

Having a galaxy on interdependent requirements can become a nightmare, because we either have to start dealing with all the corner case of some being enabled without the other or to add extra logic to make sure they are not being used without the other (and error out on bad configuration). So having simple requirement adding a consistent improvement seems like a better idea.

The share-safe requirement comes with other semantic beside the store one, for example regarding the reading of the configuration. So store-requires would be too narrow.

marmoute: > So I think the request here would be to rename the requirement to `[exp-]store-requires`…
pulkitAuthorUnsubmitted
Done

Fixed the swallowing of ENOENT.

About a fine grained store-requires requirement, I am not sure what's the best way out.

Right now exp-share-safe implies two things, requirements in store and source .hgrc be shared. We definitely can have dedicated store-requires but that introduces a spider of inter-dependent requirements.

Does exp-share-safe implies store-requires or need it? etc.

For now, I have not changed anything on this front in this patch.

pulkit: Fixed the swallowing of ENOENT. About a fine grained `store-requires` requirement, I am not…
ensurerequirementscompatible(ui, requirements) ensurerequirementscompatible(ui, requirements)
marmouteUnsubmitted
Done

This looks like a lot of duplicated code, am I missing something ? Could we do less duplication?

marmoute: This looks like a lot of duplicated code, am I missing something ? Could we do less duplication?
marmouteUnsubmitted
Done

I see this is done in D8655 thanks !

marmoute: I see this is done in D8655 thanks !
pulkitAuthorUnsubmitted
Done

Refactored in next patch.

pulkit: Refactored in next patch.
# TODO there are unhandled edge cases related to opening repositories with # TODO there are unhandled edge cases related to opening repositories with
# shared storage. If storage is shared, we should also test for requirements # shared storage. If storage is shared, we should also test for requirements
# compatibility in the pointed-to repo. This entails loading the .hg/hgrc in # compatibility in the pointed-to repo. This entails loading the .hg/hgrc in
# that repo, as that repo may load extensions needed to open it. This is a # that repo, as that repo may load extensions needed to open it. This is a
# bit complicated because we don't want the other hgrc to overwrite settings # bit complicated because we don't want the other hgrc to overwrite settings
# in this hgrc. # in this hgrc.
# #
# This bug is somewhat mitigated by the fact that we copy the .hg/requires # This bug is somewhat mitigated by the fact that we copy the .hg/requires
# file when sharing repos. But if a requirement is added after the share is # file when sharing repos. But if a requirement is added after the share is
# performed, thereby introducing a new requirement for the opener, we may # performed, thereby introducing a new requirement for the opener, we may
# will not see that and could encounter a run-time error interacting with # will not see that and could encounter a run-time error interacting with
# that shared store since it has an unknown-to-us requirement. # that shared store since it has an unknown-to-us requirement.
# At this point, we know we should be capable of opening the repository. # At this point, we know we should be capable of opening the repository.
# Now get on with doing that. # Now get on with doing that.
features = set() features = set()
# The "store" part of the repository holds versioned data. How it is # The "store" part of the repository holds versioned data. How it is
# accessed is determined by various requirements. If `shared` or # accessed is determined by various requirements. If `shared` or
# `relshared` requirements are present, this indicates current repository # `relshared` requirements are present, this indicates current repository
# is a share and store exists in path mentioned in `.hg/sharedpath` # is a share and store exists in path mentioned in `.hg/sharedpath`
shared = (
requirementsmod.SHARED_REQUIREMENT in requirements
or requirementsmod.RELATIVE_SHARED_REQUIREMENT in requirements
)
if shared: if shared:
sharedvfs = _getsharedvfs(hgvfs, requirements)
storebasepath = sharedvfs.base storebasepath = sharedvfs.base
cachepath = sharedvfs.join(b'cache') cachepath = sharedvfs.join(b'cache')
features.add(repository.REPO_FEATURE_SHARED_STORAGE) features.add(repository.REPO_FEATURE_SHARED_STORAGE)
else: else:
storebasepath = hgvfs.base storebasepath = hgvfs.base
cachepath = hgvfs.join(b'cache') cachepath = hgvfs.join(b'cache')
wcachepath = hgvfs.join(b'wcache') wcachepath = hgvfs.join(b'wcache')
b'generaldelta', b'generaldelta',
requirementsmod.TREEMANIFEST_REQUIREMENT, requirementsmod.TREEMANIFEST_REQUIREMENT,
requirementsmod.COPIESSDC_REQUIREMENT, requirementsmod.COPIESSDC_REQUIREMENT,
requirementsmod.REVLOGV2_REQUIREMENT, requirementsmod.REVLOGV2_REQUIREMENT,
requirementsmod.SIDEDATA_REQUIREMENT, requirementsmod.SIDEDATA_REQUIREMENT,
requirementsmod.SPARSEREVLOG_REQUIREMENT, requirementsmod.SPARSEREVLOG_REQUIREMENT,
requirementsmod.NODEMAP_REQUIREMENT, requirementsmod.NODEMAP_REQUIREMENT,
bookmarks.BOOKMARKS_IN_STORE_REQUIREMENT, bookmarks.BOOKMARKS_IN_STORE_REQUIREMENT,
requirementsmod.SHARESAFE_REQUIREMENT,
} }
_basesupported = supportedformats | { _basesupported = supportedformats | {
b'store', b'store',
b'fncache', b'fncache',
requirementsmod.SHARED_REQUIREMENT, requirementsmod.SHARED_REQUIREMENT,
requirementsmod.RELATIVE_SHARED_REQUIREMENT, requirementsmod.RELATIVE_SHARED_REQUIREMENT,
b'dotencode', b'dotencode',
requirementsmod.SPARSE_REQUIREMENT, requirementsmod.SPARSE_REQUIREMENT,
requirements.add(b'lfs') requirements.add(b'lfs')
if ui.configbool(b'format', b'bookmarks-in-store'): if ui.configbool(b'format', b'bookmarks-in-store'):
requirements.add(bookmarks.BOOKMARKS_IN_STORE_REQUIREMENT) requirements.add(bookmarks.BOOKMARKS_IN_STORE_REQUIREMENT)
if ui.configbool(b'format', b'use-persistent-nodemap'): if ui.configbool(b'format', b'use-persistent-nodemap'):
requirements.add(requirementsmod.NODEMAP_REQUIREMENT) requirements.add(requirementsmod.NODEMAP_REQUIREMENT)
# if share-safe is enabled, let's create the new repository with the new
# requirement
if ui.configbool(b'format', b'exp-share-safe'):
requirements.add(requirementsmod.SHARESAFE_REQUIREMENT)
indygregUnsubmitted
Not Done

It's really weird that this requirement has share in its name but it isn't dependent on being a shared repo.

indygreg: It's really weird that this requirement has `share` in its name but it isn't dependent on being…
marmouteUnsubmitted
Not Done

The requirements is named "share-safe" because after this change, the repository is safe to be shared. A repository do not need to be shared to "share-safe" the same way a watch does not need to be in the submerged to be water-proof.

marmoute: The requirements is named "share-safe" because after this change, the repository is safe to be…
pulkitAuthorUnsubmitted
Done

What @marmoute said, the requirement is named share-safe because after this change, the repository is safe to be shared.

pulkit: What @marmoute said, the requirement is named `share-safe` because after this change, the…
return requirements return requirements
def checkrequirementscompat(ui, requirements): def checkrequirementscompat(ui, requirements):
""" Checks compatibility of repository requirements enabled and disabled. """ Checks compatibility of repository requirements enabled and disabled.
Returns a set of requirements which needs to be dropped because dependend Returns a set of requirements which needs to be dropped because dependend
requirements are not enabled. Also warns users about it """ requirements are not enabled. Also warns users about it """
): ):
raise error.Abort( raise error.Abort(
_( _(
b"cannot create shared repository as source was created" b"cannot create shared repository as source was created"
b" with 'format.usestore' config disabled" b" with 'format.usestore' config disabled"
) )
) )
if requirementsmod.SHARESAFE_REQUIREMENT in requirements:
ui.warn(
_(
b"ignoring enabled 'format.exp-share-safe' config because "
b"it is incompatible with disabled 'format.usestore'"
b" config\n"
)
)
dropped.add(requirementsmod.SHARESAFE_REQUIREMENT)
indygregUnsubmitted
Not Done

I'm on the fence about whether we should drop or abort here. Either way is probably fine.

indygreg: I'm on the fence about whether we should drop or abort here. Either way is probably fine.
pulkitAuthorUnsubmitted
Done

I am bit more inclined on aborting here but realized that this whole needs a separate discussion as unfortunately we didn't have such handling yet.

pulkit: I am bit more inclined on aborting here but realized that this whole needs a separate…
return dropped return dropped
def filterknowncreateopts(ui, createopts): def filterknowncreateopts(ui, createopts):
"""Filters a dict of repo creation options against options that are known. """Filters a dict of repo creation options against options that are known.
Receives a dict of repo creation options and returns a dict of those Receives a dict of repo creation options and returns a dict of those
options that we don't know how to handle. options that we don't know how to handle.
# The revlog header has version 2, which won't be recognized by # The revlog header has version 2, which won't be recognized by
# such old clients. # such old clients.
hgvfs.append( hgvfs.append(
b'00changelog.i', b'00changelog.i',
b'\0\0\0\2 dummy changelog to prevent using the old repo ' b'\0\0\0\2 dummy changelog to prevent using the old repo '
b'layout', b'layout',
) )
scmutil.writerequires(hgvfs, requirements) # Filter the requirements into working copy and store ones
wcreq, storereq = scmutil.filterrequirements(requirements)
# write working copy ones
scmutil.writerequires(hgvfs, wcreq)
# If there are store requirements and the current repository
# is not a shared one, write stored requirements
# For new shared repository, we don't need to write the store
# requirements as they are already present in store requires
if storereq and b'sharedrepo' not in createopts:
indygregUnsubmitted
Not Done

sharedrepo wants to be converted to a constant.

indygreg: `sharedrepo` wants to be converted to a constant.
pulkitAuthorUnsubmitted
Done

This is bit messy here. sharedrepo here is createopts key rather than a requirement name.

pulkit: This is bit messy here. `sharedrepo` here is `createopts` key rather than a requirement name.
scmutil.writerequires(hgvfs, wcreq)
storevfs = vfsmod.vfs(hgvfs.join(b'store'), cacheaudited=True)
scmutil.writerequires(storevfs, storereq)
marmouteUnsubmitted
Done

It looks like share-safe should requires store, can we make them dependent of each other.

marmoute: It looks like `share-safe` should requires `store`, can we make them dependent of each other.
pulkitAuthorUnsubmitted
Done

Does not share extension implies that store is present?

pulkit: Does not `share` extension implies that `store` is present?
marmouteUnsubmitted
Done

Maybe, can you check ?

In all cases, I would rather make the safe-share requirement consistent from the start. Can we have some code either setup store automatically, or reject with some error (whichever solution you prefer).

marmoute: Maybe, can you check ? In all cases, I would rather make the safe-share requirement consistent…
indygregUnsubmitted
Not Done

This whole block needs more comments because what it is trying to accomplish is important and nuanced.

I would also consider rewriting the code so that we populate variables indicating the set of requirements to populate in .hg/requires and .hg/store/requires. That way we have at most 2 calls to writerequires(), which I think will improve readability.

indygreg: This whole block needs more comments because what it is trying to accomplish is important and…
indygregUnsubmitted
Not Done

What this is doing is blindly moving all requirements except SHARESAFE_REQUIREMENT to .hg/store/requires. The movement of most requirements there makes sense because they govern the "store." However, there are requirements like exp-sparse which I believe only concern the working directory and should not be in the "store."

If we want to go the approach of moving store-specific requirements into .hg/store/requires, I think we need to annotate whether requirements belong to the store or the non-store and write/move requirements accordingly.

indygreg: What this is doing is blindly moving all requirements except `SHARESAFE_REQUIREMENT` to `.
marmouteUnsubmitted
Not Done

Defaulting to "requirements is a store requirement" and explicitly flagging requirement that are working copy specific seems the safest.

marmoute: Defaulting to "requirements is a store requirement" and explicitly flagging requirement that…
pulkitAuthorUnsubmitted
Done

The whole code block is now much simpler, thanks to filterrequirements introduced in previous patches.

pulkit: The whole code block is now much simpler, thanks to `filterrequirements` introduced in previous…
# Write out file telling readers where to find the shared store. # Write out file telling readers where to find the shared store.
if b'sharedrepo' in createopts: if b'sharedrepo' in createopts:
hgvfs.write(b'sharedpath', sharedpath) hgvfs.write(b'sharedpath', sharedpath)
if createopts.get(b'shareditems'): if createopts.get(b'shareditems'):
shared = b'\n'.join(sorted(createopts[b'shareditems'])) + b'\n' shared = b'\n'.join(sorted(createopts[b'shareditems'])) + b'\n'
hgvfs.write(b'shared', shared) hgvfs.write(b'shared', shared)

mercurial/requirements.py

# Denotes that the current repository is a share # Denotes that the current repository is a share
SHARED_REQUIREMENT = b'shared' SHARED_REQUIREMENT = b'shared'
# Denotes that current repository is a share and the shared source path is # Denotes that current repository is a share and the shared source path is
# relative to the current repository root path # relative to the current repository root path
RELATIVE_SHARED_REQUIREMENT = b'relshared' RELATIVE_SHARED_REQUIREMENT = b'relshared'
# A repository with share implemented safely. The repository has different
# store and working copy requirements i.e. both `.hg/requires` and
# `.hg/store/requires` are present.
SHARESAFE_REQUIREMENT = b'exp-sharesafe'
# List of requirements which are working directory specific # List of requirements which are working directory specific
# These requirements cannot be shared between repositories if they # These requirements cannot be shared between repositories if they
# share the same store # share the same store
# * sparse is a working directory specific functionality and hence working # * sparse is a working directory specific functionality and hence working
# directory specific requirement # directory specific requirement
# * SHARED_REQUIREMENT and RELATIVE_SHARED_REQUIREMENT are requirements which # * SHARED_REQUIREMENT and RELATIVE_SHARED_REQUIREMENT are requirements which
# represents that the current working copy/repository shares store of another # represents that the current working copy/repository shares store of another
# repo. Hence both of them should be stored in working copy # repo. Hence both of them should be stored in working copy
# * SHARESAFE_REQUIREMENT needs to be stored in working dir to mark that rest of
# the requirements are stored in store's requires
WORKING_DIR_REQUIREMENTS = { WORKING_DIR_REQUIREMENTS = {
SPARSE_REQUIREMENT, SPARSE_REQUIREMENT,
SHARED_REQUIREMENT, SHARED_REQUIREMENT,
RELATIVE_SHARED_REQUIREMENT, RELATIVE_SHARED_REQUIREMENT,
SHARESAFE_REQUIREMENT,
} }

mercurial/scmutil.py

def filterrequirements(requirements): def filterrequirements(requirements):
""" filters the requirements into two sets: """ filters the requirements into two sets:
wcreq: requirements which should be written in .hg/requires wcreq: requirements which should be written in .hg/requires
storereq: which should be written in .hg/store/requires storereq: which should be written in .hg/store/requires
Returns (wcreq, storereq) Returns (wcreq, storereq)
""" """
if False: if requirementsmod.SHARESAFE_REQUIREMENT in requirements:
wc, store = set(), set() wc, store = set(), set()
for r in requirements: for r in requirements:
if r in requirementsmod.WORKING_DIR_REQUIREMENTS: if r in requirementsmod.WORKING_DIR_REQUIREMENTS:
wc.add(r) wc.add(r)
else: else:
store.add(r) store.add(r)
return wc, store return wc, store
return requirements, None return requirements, None
def writereporequirements(repo, requirements=None): def writereporequirements(repo, requirements=None):
""" writes requirements for the repo to .hg/requires """ """ writes requirements for the repo to .hg/requires """
if requirements: if requirements:
repo.requirements = requirements repo.requirements = requirements
wcreq, storereq = filterrequirements(repo.requirements) wcreq, storereq = filterrequirements(repo.requirements)
if wcreq is not None: if wcreq is not None:
indygregUnsubmitted
Done

The presence of this conditional here feels a bit weird to me: I feel like it might be better if we explicitly passed in 2 sets of requirements to read and this function were "dumber."

indygreg: The presence of this conditional here feels a bit weird to me: I feel like it might be better…
pulkitAuthorUnsubmitted
Done

This code is much cleaner now after introducing filterrequirements in previous patches.

pulkit: This code is much cleaner now after introducing `filterrequirements` in previous patches.
writerequires(repo.vfs, wcreq) writerequires(repo.vfs, wcreq)
indygregUnsubmitted
Not Done

Writing of the requirements file should be extremely rare. I don't think we need this lock here.

indygreg: Writing of the requirements file should be extremely rare. I don't think we need this lock here.
marmouteUnsubmitted
Not Done

I agreed that the requirements should be written at repository creation time (or upgrade time) and that is all. Maybe we should update the acquiring of the lock with some code checking we are actually holding a lock while doing this?

The upgrade usecase is a good argument in favor of moving the requirement file inside the store directory.

marmoute: I agreed that the requirements should be written at repository creation time (or upgrade time)…
if storereq is not None: if storereq is not None:
writerequires(repo.svfs, storereq) writerequires(repo.svfs, storereq)
def writerequires(opener, requirements): def writerequires(opener, requirements):
with opener(b'requires', b'w', atomictemp=True) as fp: with opener(b'requires', b'w', atomictemp=True) as fp:
for r in sorted(requirements): for r in sorted(requirements):
fp.write(b"%s\n" % r) fp.write(b"%s\n" % r)

mercurial/store.py

mode = vfs.stat().st_mode mode = vfs.stat().st_mode
# avoid some useless chmods # avoid some useless chmods
if (0o777 & ~util.umask) == (0o777 & mode): if (0o777 & ~util.umask) == (0o777 & mode):
mode = None mode = None
except OSError: except OSError:
mode = None mode = None
return mode return mode
_data = [ _data = [
indygregUnsubmitted
Done

Bonus points if you refactor this to be a proper data structure as part of this series. A space delimited list/set makes no sense to me.

indygreg: Bonus points if you refactor this to be a proper data structure as part of this series. A space…
marmouteUnsubmitted
Done

I can add an extra bonus point to the stake :-)

marmoute: I can add an extra bonus point to the stake :-)
b'bookmarks', b'bookmarks',
b'narrowspec', b'narrowspec',
b'data', b'data',
b'meta', b'meta',
b'00manifest.d', b'00manifest.d',
b'00manifest.i', b'00manifest.i',
b'00changelog.d', b'00changelog.d',
b'00changelog.i', b'00changelog.i',
b'phaseroots', b'phaseroots',
b'obsstore', b'obsstore',
b'requires',
] ]
def isrevlog(f, kind, st): def isrevlog(f, kind, st):
return kind == stat.S_IFREG and f[-2:] in (b'.i', b'.d') return kind == stat.S_IFREG and f[-2:] in (b'.i', b'.d')
class basicstore(object): class basicstore(object):
''' '''
# yield data files first # yield data files first
for x in self.datafiles(matcher): for x in self.datafiles(matcher):
yield x yield x
for x in self.topfiles(): for x in self.topfiles():
yield x yield x
def copylist(self): def copylist(self):
return [b'requires'] + _data return _data
indygregUnsubmitted
Done

Wait - why was requires here before?

indygreg: Wait - why was `requires` here before?
pulkitAuthorUnsubmitted
Done

A line of code dating back to 2008. Removing it does not make any tests fail. Will send a patch for it separately.

pulkit: A line of code dating back to 2008. Removing it does not make any tests fail. Will send a patch…
def write(self, tr): def write(self, tr):
pass pass
def invalidatecaches(self): def invalidatecaches(self):
pass pass
def markremoved(self, fn): def markremoved(self, fn):
b'dh', b'dh',
b'fncache', b'fncache',
b'phaseroots', b'phaseroots',
b'obsstore', b'obsstore',
b'00manifest.d', b'00manifest.d',
b'00manifest.i', b'00manifest.i',
b'00changelog.d', b'00changelog.d',
b'00changelog.i', b'00changelog.i',
b'requires',
) )
return [b'requires', b'00changelog.i'] + [b'store/' + f for f in d] return [b'requires', b'00changelog.i'] + [b'store/' + f for f in d]
def write(self, tr): def write(self, tr):
self.fncache.write(tr) self.fncache.write(tr)
def invalidatecaches(self): def invalidatecaches(self):
self.fncache.entries = None self.fncache.entries = None

tests/test-journal-share.t

#testcases safe normal
#if safe
$ echo "[format]" >> $HGRCPATH
$ echo "exp-share-safe = True" >> $HGRCPATH
#endif
Journal extension test: tests the share extension support Journal extension test: tests the share extension support
$ cat >> testmocks.py << EOF $ cat >> testmocks.py << EOF
> # mock out procutil.getuser() and util.makedate() to supply testable values > # mock out procutil.getuser() and util.makedate() to supply testable values
> import os > import os
> from mercurial import util > from mercurial import util
> from mercurial.utils import procutil > from mercurial.utils import procutil
> def mockgetuser(): > def mockgetuser():

tests/test-narrow-share.t

#testcases flat tree #testcases flat tree
#testcases safe normal
#if safe
$ echo "[format]" >> $HGRCPATH
$ echo "exp-share-safe = True" >> $HGRCPATH
#endif
$ . "$TESTDIR/narrow-library.sh" $ . "$TESTDIR/narrow-library.sh"
#if tree #if tree
$ cat << EOF >> $HGRCPATH $ cat << EOF >> $HGRCPATH
> [experimental] > [experimental]
> treemanifest = 1 > treemanifest = 1
> EOF > EOF

tests/test-remotefilelog-share.t

#require no-windows #require no-windows
#testcases safe normal
#if safe
$ echo "[format]" >> $HGRCPATH
$ echo "exp-share-safe = True" >> $HGRCPATH
#endif
$ . "$TESTDIR/remotefilelog-library.sh" $ . "$TESTDIR/remotefilelog-library.sh"
$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [extensions] > [extensions]
> remotefilelog= > remotefilelog=
> share= > share=
> EOF > EOF

tests/test-share-bookmarks.t

#testcases vfs svfs #testcases vfs svfs
#testcases safe normal
#if safe
$ echo "[format]" >> $HGRCPATH
$ echo "exp-share-safe = True" >> $HGRCPATH
#endif
$ echo "[extensions]" >> $HGRCPATH $ echo "[extensions]" >> $HGRCPATH
$ echo "share = " >> $HGRCPATH $ echo "share = " >> $HGRCPATH
#if svfs #if svfs
$ echo "[format]" >> $HGRCPATH $ echo "[format]" >> $HGRCPATH
$ echo "bookmarks-in-store = yes " >> $HGRCPATH $ echo "bookmarks-in-store = yes " >> $HGRCPATH
#endif #endif
bm3 4:62f4ded848e4 bm3 4:62f4ded848e4
bm4 5:92793bfc8cad bm4 5:92793bfc8cad
$ cd .. $ cd ..
Test that if store is disabled, we drop the bookmarksinstore requirement Test that if store is disabled, we drop the bookmarksinstore requirement
$ hg init brokenrepo --config format.bookmarks-in-store=True --config format.usestore=false $ hg init brokenrepo --config format.bookmarks-in-store=True --config format.usestore=false
ignoring enabled 'format.bookmarks-in-store' config beacuse it is incompatible with disabled 'format.usestore' config ignoring enabled 'format.bookmarks-in-store' config beacuse it is incompatible with disabled 'format.usestore' config
ignoring enabled 'format.exp-share-safe' config because it is incompatible with disabled 'format.usestore' config (safe !)

tests/test-share-safe.t

  • This file was added.
setup
$ cat >> $HGRCPATH <<EOF
> [extensions]
> share =
> [format]
> exp-share-safe = True
> EOF
prepare source repo
$ hg init source
$ cd source
$ cat .hg/requires
exp-sharesafe
$ cat .hg/store/requires
dotencode
fncache
generaldelta
revlogv1
sparserevlog
store
$ hg debugrequirements
dotencode
exp-sharesafe
fncache
generaldelta
revlogv1
sparserevlog
store
$ echo a > a
$ hg ci -Aqm "added a"
$ echo b > b
$ hg ci -Aqm "added b"
$ cd ..
Create a shared repo and check the requirements are shared and read correctly
$ hg share source shared1
updating working directory
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cd shared1
$ cat .hg/requires
exp-sharesafe
shared
$ hg debugrequirements -R ../source
dotencode
exp-sharesafe
fncache
generaldelta
revlogv1
sparserevlog
store
$ hg debugrequirements
dotencode
exp-sharesafe
fncache
generaldelta
revlogv1
shared
sparserevlog
store
$ echo c > c
$ hg ci -Aqm "added c"
$ hg unshare

tests/test-share.t

#testcases safe normal
#if safe
$ echo "[format]" >> $HGRCPATH
$ echo "exp-share-safe = True" >> $HGRCPATH
#endif
$ echo "[extensions]" >> $HGRCPATH $ echo "[extensions]" >> $HGRCPATH
$ echo "share = " >> $HGRCPATH $ echo "share = " >> $HGRCPATH
prepare repo1 prepare repo1
$ hg init repo1 $ hg init repo1
$ cd repo1 $ cd repo1
$ echo a > a $ echo a > a
Explicitly kill daemons to let the test exit on Windows Explicitly kill daemons to let the test exit on Windows
$ killdaemons.py $ killdaemons.py
Test sharing a repository which was created with store requirement disable Test sharing a repository which was created with store requirement disable
$ hg init nostore --config format.usestore=false $ hg init nostore --config format.usestore=false
ignoring enabled 'format.exp-share-safe' config because it is incompatible with disabled 'format.usestore' config (safe !)
$ hg share nostore sharednostore $ hg share nostore sharednostore
abort: cannot create shared repository as source was created with 'format.usestore' config disabled abort: cannot create shared repository as source was created with 'format.usestore' config disabled
[255] [255]