SSHStore in bfile

[Greg Ward]

[me]
> To clarify: I am willing to put up with duplicate code in bfiles.py in
> order to make it work with Mercurial 1.3.  But I would like to
> refactor Mercurial 1.x so that the duplication is no longer necessary.
>  Once bfiles requires Mercurial 1.x or later, the duplicate code can
> be removed.  Hopefully 1.x will be 1.4, but time is tight for that.

[David]
> Oh, good news, I did't know bfiles was candidate for hg's inclusion. Knowing
> that, your previous remarks about sending patches against hg's code make much
> more sense to me.

No, I didn't mean that bfiles is a candidate for inclusion in
Mercurial.  (Maybe someday it will be, but that's a long way off.)  I
was referring to the fact that every extension has to say "requires
Mercurial version X or later".  In the case of bfiles, if X <= "1.3",
then we have to put up with ugly duplication of ssh protocol-related
code.  But if we get some refactoring patches into Mercurial in time
for 1.4, and *then* make bfiles "require Mercurial 1.4 or later", we
can drop the ugly duplication.

>> Finally, now that I have tried to use your patch, I see that it
>> expects the central store to be a Mercurial repository.  That doesn't
>> make a lot of sense, since the whole point of bfiles is to get large
>> binary files out of the repository.
>
> Yes, this is a drawback I'm quite unhappy with, but I did not see any other
> way of doing it with hg's current code. In my implementation, the creation of
> the repo in the store is managed by the __init__ of the SSHStore class. If
> the "validate_repo" fails, a repo is created on the remote machine.

Hmmm.  Definitely ugly, but I understand it now.

> The only way I see to fix this actually useless machinery is to fix hg's
> sshserve code to be able to be executed without a distant repo. We may thus
> provide a new server command like 'do_connect' to specify a new repo
> location. I'll ivestigate this a bit further however.

What about adding a new server-side command, like bfserve?  Then
instead of running

  ssh <user>@<host> 'hg -R <storedir> serve --stdout'

and requiring the 'bfstore' capability to use commands 'bfget' and
'bfput', we could instead run

  ssh <user>@<host> 'hg bfserve <storedir>'

and use commands 'bfget' and 'bfput'.  (I suppose it makes sense to
use the capabilities mechanism anyways, but it is not strictly
necessary.)

> For now, it is required to have a repo, even if it is not used for the store
> beside the ability to run a 'hg serve' command from there.

Tolerable for now.  I say we get this committed and passing its tests.
 Then we clean it up.

Next topic: testing.

> In fact, current tests can (well,
> should, since 'verify' is not yet implemented in SSHStore) run fine against
> the SSHstore simply by changing the "store" parameter of the hgrc.
> In my repo, I have modified a bit tests/common like:

Not very useful IMHO: the current tests spend a lot of time testing
bfadd, bfrefresh, bfstatus, etc... and all those commands have nothing
to do with the store.  The only commands affected by a new store
implementation are bfupdate and bfput.  So test-sshstore really only
needs to use those commands (and bfstatus to make sure something
happened).

[re: the createrc() function in tests/common]
> [bfiles]
> store = ${STOREPROTOCOL}$STOREDIR

That's a good idea.  Although I would just pass the store URL as a
parameter to createrc.

[me again]
> More importantly, I have refactored the store interface so that get()
> takes a list of (filename, hash) pairs.

[David notices the obvious]
> Shouldn't the put() method be refactored the same way? Looks strange to me to
> different APIs for these 2 methods.

Yes, definitely.  Refactoring store.get() was surprisingly tricky
(hey, I had a cold and was not at my best), so I decided to do one
thing at a time.  I'll do put() this morning, unless I see a new patch
set from you first.

[one last thing]
> Another thing, do you mind if I also write, in common:
>
> createrc() {
>    cat > $HGRCPATH << __EOF__
> $USERHGRC
>
[...]
> This USERHGRC is useful for me when running sshstore tests to be able to
> configure ui.remotecmd (which I need on my station, default hg being my
> disctribution's one) using this shell variable.

That should be unnecessary: why don't you just set $PATH so yours
comes before /usr/bin/hg?

Greg