Re: [PATCH 3 of 6] lfs: infer the blob store URL from paths.default

2018-04-09 Thread Yuya Nishihara
On Mon, 9 Apr 2018 10:53:43 -0400, Matt Harbison wrote:
> 
> > On Apr 9, 2018, at 9:26 AM, Yuya Nishihara  wrote:
> > 
> >> On Mon, 09 Apr 2018 00:26:43 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison 
> >> # Date 1523154140 14400
> >> #  Sat Apr 07 22:22:20 2018 -0400
> >> # Node ID 3d5a3d9692c0e9f3424a3eac148bef674580885f
> >> # Parent  f4381233ecb960307d39459ea961a0af03df442b
> >> lfs: infer the blob store URL from paths.default
> > 
> >> def remote(repo):
> >> -"""remotestore factory. return a store in _storemap depending on 
> >> config"""
> >> +"""remotestore factory. return a store in _storemap depending on 
> >> config
> >> +
> >> +If ``lfs.url`` is specified, use that remote endpoint.  Otherwise, 
> >> try to
> >> +infer the endpoint, based on the remote repository using the same path
> >> +adjustments as git.  As an extension, 'http' is supported as well so 
> >> that
> >> +``hg serve`` works out of the box.
> >> +
> >> +
> >> https://github.com/git-lfs/git-lfs/blob/master/docs/api/server-discovery.md
> >> +"""
> >> url = util.url(repo.ui.config('lfs', 'url') or '')
> >> +if url.scheme is None:
> >> +# TODO: investigate 'paths.remote:lfsurl' style path 
> >> customization,
> >> +# and fall back to inferring from 'paths.remote' if unspecified.
> >> +defaulturl = util.url(repo.ui.config('paths', 'default') or b'')
> >> +
> >> +# TODO: support local paths as well.
> >> +if defaulturl.scheme and defaulturl.scheme != b'file':
> >> +if defaulturl.scheme in (b'http', b'https', b'ssh'):
> >> +defaulturl.path = defaulturl.path or b'' + 
> >> b'.git/info/lfs'
> >> +if defaulturl.scheme == b'ssh':
> >> +defaulturl.scheme = b'https'
> > 
> > Maybe we shouldn't rewrite ssh to https because there's no clue that the SSH
> > path matches the HTTP path. Also, eventually we might add an SSH-based LFS
> > protocol.
> 
> That seems to be what the git spec in the function doc here is doing.  But I 
> don’t have a problem dropping it, because it’s hard to describe in 
> documentation, and it does seem to be assuming a lot.

Ugh. Can you add a TODO comment for that? I think it's better to not follow
the git way here since we're making new SSH protocol which will look more
like HTTP.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 6] lfs: infer the blob store URL from paths.default

2018-04-09 Thread Matt Harbison

> On Apr 9, 2018, at 9:26 AM, Yuya Nishihara  wrote:
> 
>> On Mon, 09 Apr 2018 00:26:43 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison 
>> # Date 1523154140 14400
>> #  Sat Apr 07 22:22:20 2018 -0400
>> # Node ID 3d5a3d9692c0e9f3424a3eac148bef674580885f
>> # Parent  f4381233ecb960307d39459ea961a0af03df442b
>> lfs: infer the blob store URL from paths.default
> 
>> def remote(repo):
>> -"""remotestore factory. return a store in _storemap depending on 
>> config"""
>> +"""remotestore factory. return a store in _storemap depending on config
>> +
>> +If ``lfs.url`` is specified, use that remote endpoint.  Otherwise, try 
>> to
>> +infer the endpoint, based on the remote repository using the same path
>> +adjustments as git.  As an extension, 'http' is supported as well so 
>> that
>> +``hg serve`` works out of the box.
>> +
>> +
>> https://github.com/git-lfs/git-lfs/blob/master/docs/api/server-discovery.md
>> +"""
>> url = util.url(repo.ui.config('lfs', 'url') or '')
>> +if url.scheme is None:
>> +# TODO: investigate 'paths.remote:lfsurl' style path customization,
>> +# and fall back to inferring from 'paths.remote' if unspecified.
>> +defaulturl = util.url(repo.ui.config('paths', 'default') or b'')
>> +
>> +# TODO: support local paths as well.
>> +if defaulturl.scheme and defaulturl.scheme != b'file':
>> +if defaulturl.scheme in (b'http', b'https', b'ssh'):
>> +defaulturl.path = defaulturl.path or b'' + b'.git/info/lfs'
>> +if defaulturl.scheme == b'ssh':
>> +defaulturl.scheme = b'https'
> 
> Maybe we shouldn't rewrite ssh to https because there's no clue that the SSH
> path matches the HTTP path. Also, eventually we might add an SSH-based LFS
> protocol.

That seems to be what the git spec in the function doc here is doing.  But I 
don’t have a problem dropping it, because it’s hard to describe in 
documentation, and it does seem to be assuming a lot.

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 6] lfs: infer the blob store URL from paths.default

2018-04-09 Thread Yuya Nishihara
On Mon, 09 Apr 2018 00:26:43 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison 
> # Date 1523154140 14400
> #  Sat Apr 07 22:22:20 2018 -0400
> # Node ID 3d5a3d9692c0e9f3424a3eac148bef674580885f
> # Parent  f4381233ecb960307d39459ea961a0af03df442b
> lfs: infer the blob store URL from paths.default

>  def remote(repo):
> -"""remotestore factory. return a store in _storemap depending on 
> config"""
> +"""remotestore factory. return a store in _storemap depending on config
> +
> +If ``lfs.url`` is specified, use that remote endpoint.  Otherwise, try to
> +infer the endpoint, based on the remote repository using the same path
> +adjustments as git.  As an extension, 'http' is supported as well so that
> +``hg serve`` works out of the box.
> +
> +
> https://github.com/git-lfs/git-lfs/blob/master/docs/api/server-discovery.md
> +"""
>  url = util.url(repo.ui.config('lfs', 'url') or '')
> +if url.scheme is None:
> +# TODO: investigate 'paths.remote:lfsurl' style path customization,
> +# and fall back to inferring from 'paths.remote' if unspecified.
> +defaulturl = util.url(repo.ui.config('paths', 'default') or b'')
> +
> +# TODO: support local paths as well.
> +if defaulturl.scheme and defaulturl.scheme != b'file':
> +if defaulturl.scheme in (b'http', b'https', b'ssh'):
> +defaulturl.path = defaulturl.path or b'' + b'.git/info/lfs'
> +if defaulturl.scheme == b'ssh':
> +defaulturl.scheme = b'https'

Maybe we shouldn't rewrite ssh to https because there's no clue that the SSH
path matches the HTTP path. Also, eventually we might add an SSH-based LFS
protocol.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel