[PATCH] largefiles: makes verify to work on local content by default (issue4242) (BC)

[Matt Harbison]

On Thu, 17 Mar 2016 11:56:07 -0400, Piotr Listkiewicz  
<piotr.listkiewicz at gmail.com> wrote:

>>
>> I re-read the code, and it seems you get the point. Perhaps "local" has  
>> two
>> different meanings:
>>  a) a repository alongside the current working directory
>>  b) repositories that are stored in local filesystem (includes (a))
>> New "--remote" options switches (a), and "--lfc" seems to work only for
>> (b).
>> So "--lfc" (without --remote) should always work. On the other hand,
>> "--lfc --remote" won't work if the peer repository is stored remotely.
>> Should we raise error in that case? I don't know...
>
>
> I checked this once again, and before this patch --lfc works even weirder
> than i thought.  So how --lfc works depends on how value
> default/default-push in hgrc is specified. The cases are:
>
> 1) there is no default or is sth wrong with default
> --lfc works as expected, checks locally existence and content
> in code this is localrepo with remote set to repo we invoke the command
>
> 2) default is pointing at repository on local file system (default starts
> with file:")
>
> --lfc checks file existence and content but on the repository pointed by
> default(not on the repository the command was invoked)
>
> in code this is localrepo with remote set to repo pointed by default
>
> 3) default is pointing by http on some place on web
>
> --lfc checks file existence but not content.
>
> in code this is wirestore with remote set to repo pointed by default
>
> What is more, --large and --lfa also choose repository to check in this
> strange way. Option --lfc by design wasn't prepared to work on remote
> repository, because authors thought that it was too expensive to enable  
> it.
> For now there are several different paths to resolve it:
>
> 1) makes --large, --lfa, --lfc do checks locally by default, with  
> --remote
> checks on remote and we accept that --lfc is expensive operation on  
> remote
> repo. In my view that would be what user expects
>
> 2) makes --large, --lfa do checks by default, with --remote checks on
> remote(with remote we mean outside of repository, it can be on the same
> machine). Option --lfc always would be doing local checks, with --remote  
> it
> would abort msg  "cannot --lfc and --remote at the same time".

I guess I would choose this option.  I agree #1 is what the user might  
expect, but presumably the way it would be done is to transfer the file  
 from the remote to the local machine?  Then the user will wonder why not  
just cache the files after going through that expense.  Since there are  
valid reasons for not doing it (e.g. taking up a lot of space), that would  
need yet another flag.  #2 doesn't prevent changing to #1 in the future  
AFAICT.

> However from the point of current implementation it cant be handled so
> easily because wirestore doesn't do any content check, localstore is  
> doing
> no matter if remote is the same repository or other repository on the  
> same
> machine.

I don't understand this comment, if you abort with --lfc and --remote  
together (presumably in the command.py override).  The clone override does  
similar checks for incompatible flags.

> 3) makes --large, --lfa, --lfc do checks only locally

I found the ability to check the parent repo occasionally useful.  It's  
been ages since I used it however, so I don't recall exactly what happened  
to the upstream repo where this was useful.  (If I had to guess, it would  
be that when moving something I did locally to the server, I simply logged  
into the server and cloned from a local `hg serve`, instead of `hg init`  
on the server and then `hg push` from back on the local machine.)

> In my view current behaviour is so weird that choosing any alternative
> would be step in good direction. What do you suggest?
>
> 2016-03-14 14:15 GMT+01:00 Yuya Nishihara <yuya at tcha.org>:
>
>> On Fri, 11 Mar 2016 13:56:56 +0100, Piotr Listkiewicz wrote:
>> > > Hmm it looks for me like this is the other bug in largefile, this  
>> would
>> > > mean that so far --lfc for remote repos didnt verify file contents  
>> but
>> only
>> > > existence.
>> > > After applying this patch --lfc will use localstore.localstore which
>> > > verifies contents correctly (not just existence) but for remote  
>> repos
>> it
>> > > will preserve current( for me it looks like bad) behaviour.
>> >
>> > I checked this once again and I was totally wrong about remotestore  
>> and
>> > "--lfc" and "--lfc --remote", Im sorry for that.
>> >
>> > First of all i made mistake of not changing help, for verify --lfc  
>> should
>> > be:
>> > "verify largefile contents, not just existence" , without remote it  
>> will
>> > check locally , with --remote it will check on remote server.
>> >
>> > After this patch 'hg verify --lfc' (without --remote) will be using
>> > localstore class (with self.remote object pointing to the same local
>> > repository as self.repo). This means that all checking will be done
>> locally
>> > and checking file content will be working correctly(--large and --lfa
>> also
>> > works only locally by default).
>> >
>> > As far as we concern local operations, everything is fine. Problem  
>> starts
>> > with --remote - that is the old implementation of largefiles verify. I
>> have
>> > no idea how but it uses remotestore/wirestore object and connect to  
>> the
>> > other server with them but still verifies things locally.
>> > I have no idea how its working, maybe anybody have any suggestion?
>>
>> I re-read the code, and it seems you get the point. Perhaps "local" has  
>> two
>> different meanings:
>>
>>  a) a repository alongside the current working directory
>>  b) repositories that are stored in local filesystem (includes (a))
>>
>> New "--remote" options switches (a), and "--lfc" seems to work only for
>> (b).
>>
>> So "--lfc" (without --remote) should always work. On the other hand,
>> "--lfc --remote" won't work if the peer repository is stored remotely.
>> Should we raise error in that case? I don't know...