In D4312#77107, @idlsoft wrote:

I'm actually kinda looking forward to see if lfs works better with all the new changes.

In D5130#76737, @indygreg wrote:

! In D5130#76736, @mharbison72 wrote:
Also, I need to see if this can be made to work with cloning from a local repo.

I'm really not a fan of local / hardlink clones. They are very brittle. When you start throwing extra repo features like LFS into the mix, it becomes even more dangerous. Especially when we start talking about adding modifying .hg/requires automatically. The fundamental problem is the hardlink clone is strictly separate but it "inherits" some state from its parent. If we start varying behavior of the two repos, interaction between them can be wonky. In general, for advanced features, we should go through the "hg clone --pull" interface to create a clean break and minimize potential for wonkiness.

One thing I forgot to mention in the commit comment is that hg clone --rev REV, where REV is some commit prior to committing any lfs blobs, will cause the requirement to be added to the clone. That didn't happen previously with the transaction hook. But that's probably enough of a corner case to not really care.

In D5130#76727, @indygreg wrote:

As a follow-up, it would be nice to have the LFS wire protocol capabilities documented in mercurial/help/internals/wireprotocol.txt, especially since it isn't obvious how the now 2 capabilities are different.

LGTM, thanks.

In D5025#76443, @mharbison72 wrote:

In D5025#75556, @yuja wrote:

def perfstartup(ui, repo, **opts):
    opts = _byteskwargs(opts)
    timer, fm = gettimer(ui, opts)

• cmd = sys.argv[0] + cmd = fsencode(sys.argv[0])

Applying fsencode() on sys.argv is probably wrong on Windows, but it's perf.py,
I don't care.

Sadly, it doesn't work on Windows. But since os.system() is called with an r-string, bytes wouldn't be right anyway. When I simply formatted in sys.argv[0], I got this:

--- c:/Users/Matt/projects/hg_py3/tests/test-contrib-perf.t
+++ c:/Users/Matt/projects/hg_py3/tests/test-contrib-perf.t.err
@@ -184,6 +184,7 @@
   $ hg perfrevrange
   $ hg perfrevset 'all()'
   $ hg perfstartup
+  (null): can't open file '<unprintable file name>': [Errno 2] $ENOENT$
   $ hg perfstatus
   $ hg perftags
   $ hg perftemplating

This will be good to figure out because there are ~400 other instances of this error, mostly around launching hooks.

In D5025#75556, @yuja wrote:

def perfstartup(ui, repo, **opts):
    opts = _byteskwargs(opts)
    timer, fm = gettimer(ui, opts)

• cmd = sys.argv[0] + cmd = fsencode(sys.argv[0])

Applying fsencode() on sys.argv is probably wrong on Windows, but it's perf.py,
I don't care.

Apparently thg uses this to avoid loading the data into memory just to figure out the length like fctx.rawsize(). Any suggestions?

Windows doesn't like this test, and gets a connection refused. Under the glob is the machine name instead of $LOCALIP that is used above it. Obviously this isn't the right fix, but this makes the test run on Windows to illustrate it's not something else:

In D4713#73260, @yuja wrote:

IIUC, we would have to enable the largefiles extension once to clone the
repo, but that's no longer needed. So you can enable the extension without
hearing its name at all.

Perhaps, a warning can be displayed when the non-core repository requirement
is added?

In D4713#73077, @yuja wrote:

> Can we add some warnings here? The largefiles is IMHO one of the most buggy
>  extensions, and loading it has a side effect (e.g. fsmonitor is disabled.)
>  It shouldn't be silently loaded into the process.


I get what you’re thinking, but maybe a debug message is better than a warning, even though it’s less visible. I’m concerned about every invocation being unnecessarily noisy now that .hg/hgrc isn’t updated,

I don't think a debug message would work because ordinary users will never
see it. Is that common to work on largefiles repo without enabling the
extension globally?

In D4713#72604, @yuja wrote:

1. Map of requirements to list of extensions to load automatically when
2. requirement is present. autoextensions = { + b'largefiles': [b'largefiles'], b'lfs': [b'lfs'], }

Can we add some warnings here? The largefiles is IMHO one of the most buggy
extensions, and loading it has a side effect (e.g. fsmonitor is disabled.)
It shouldn't be silently loaded into the process.

In D4711#71787, @indygreg wrote:

I actually wrote the patch to add the lfs requirement on clone. However, the way lfs is advertised on the server is a bit buggy. See https://hg.mozilla.org/users/gszorc_mozilla.com/hg/rev/d39bf70535a5.

I could solve this problem if I wanted to. But I have other aspects of the code I'd prefer to work on.