Potential BC break on largefiles/lfs

[Martin von Zweigbergk]

On Wed, Sep 19, 2018, 18:43 Gregory Szorc <gregory.szorc at gmail.com> wrote:

> Currently, the largefiles and lfs extensions monkeypatch various functions
> so that when they encounter repository data indicative of largefiles or
> lfs, they automagically add a repository requirement for largefiles/lfs.
>

Do I remember correctly that largefiles can also remove the requirement
(I've forgotten the circumstance)?

I also think that we add one for tree manifests when they're applied from a
bundle, but I think we only do that if the repo is empty. I don't remember
if there's a good reason for that. I think we simply don't know before the
cloning starts if the remote is flat or tree. I'm sure we can come up with
a way of letting the client know that earlier.


> This behavior is arguably user-friendly and therefore good.
>
> At the same time, it is also a bit surprising. Repository requirements
> (the .hg/requires file) is ideally read-only after repository creation.
> This ensures that we are always able to open/read a repository with the
> Mercurial client that created it. Silently adding requirements when
> interacting with remote servers or performing a local commit that
> introduces a well-named file undermines that guarantee.
>
> Furthermore, I'm attempting to refactor how repository objects are
> constructed. Essentially I want to make the repository type dynamically
> derived from .hg/requires. i.e. instead of monkeypatching repo objects post
> construction, we'll build a type that is custom tailored for that exact
> repository. Unfortunately, largefiles and lfs dynamically changing the
> requirements - and therefore behavior of a repository object -
> mid-operation (like an `hg pull`) undermines my "repo type is static for
> its lifetime" goals.
>
> I wanted to take a quick temperature check to see if we're comfortable
> incurring a BC break around this auto-add-requirements behavior.
>
> I'm proposing that instead of auto-adding requirements when data is
> encountered, we hard fail when incoming data references largefiles or lfs.
> The error message would say something like "largefiles/lfs not enabled on
> this repository; enable largefiles/lfs via `hg debugenablelfs` and retry
> this operation. See <URL> for more info." This error would manifest:
>
> * During `hg pull`, immediately after retrieving the remote's capabilities
> (before any data has been transferred).
> * During `hg unbundle`, when the first data referencing largefiles/lfs is
> encountered.
> * During `hg commit` if the committed data involves LFS.
>
> The most user hostile is `hg unbundle`, as we won't see an error until
> some data has been transferred. So you could potentially transfer megabytes
> of data *then* get the error. The other errors should be ~immediate.
>
> When cloning from a repo with largefiles/lfs enabled, we would continue to
> automagically set up the repo for largefiles/lfs from its onset. So this
> proposed change would only impact people who have existing clones then
> attempt to pull/unbundle/commit data that introduces largefiles/lfs. In
> other words, introducing largefiles/lfs into an existing repository would
> require all existing clones to take manual action to enable it.
>
> If we can't incur this BC break, it makes my alternate storage backend
> work a little harder. But I think I can work around it.
>
> An alternative to the BC break would be supporting largefiles/lfs by
> default. i.e. the .hg/requires entry wouldn't be necessary to enable the
> feature. But this would entail moving basic functionality to core and/or
> raising an error when attempting to resolve largefiles/lfs data without the
> extension loaded.
>
> Thoughts?
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20180919/02f96ebe/attachment.html>