This is very similar to what we just did for LFS but for largefiles.
See recent commit messages for the rationale here.
Details
- Reviewers
- None
- Group Reviewers
hg-reviewers - Commits
- rHG823a580448d7: largefiles: automatically load largefiles extension when required (BC)
Diff Detail
- Repository
- rHG Mercurial
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
- Map of requirements to list of extensions to load automatically when
- 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.
I didn't realize largefiles is riddled with bugs. I'd be happy to drop this changeset if there are concerns about its behavior.
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, and noise on stderr triggers that thgw.exe dialog when UAC is enabled. OTOH, updating the hgrc with this would make for a quick and dirty one shot warning.
> 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?
and noise on stderr triggers that thgw.exe dialog when UAC is enabled.
Well, that's the bug of TortoiseHg if it doesn't redirect ui.ferr.
FWIW, I like the idea of enabling storage extensions automatically.
The only data point I have is what I did, which was to only enable it on the command line when cloning. The clone command modifying the local hgrc if necessary handled the rest. I only started to do that because I got annoyed at how simply having it loaded could alter certain commands. IDK if command line enabling was ever documented as the preferred alternative, but that's how I described it in e1dbe0b215ae.
At this point, I can't recall specific issues with globally enabling, and they might be mostly fixed. They definitely aren't all fixed. That said, this implicit loading on demand seems roughly equivalent to the previous behavior, so I'm not sure that this would be any more surprising. Repos on the command line would be isolated, but not with commandserver because of the global function wrapping. But I'm guessing most people don't understand that about commandserver.
The only data point I have is what I did, which was to only enable it on the command line when cloning. The clone command modifying the local hgrc if necessary handled the rest. I only started to do that because I got annoyed at how simply having it loaded could alter certain commands. IDK if command line enabling was ever documented as the preferred alternative, but that's how I described it in https://phab.mercurial-scm.org/rHGe1dbe0b215ae137eec53ceb12440536d570a83d2. At this point, I can't recall specific issues with globally enabling, and they might be mostly fixed. They definitely aren't all fixed. That said, this implicit loading on demand seems roughly equivalent to the previous behavior, so I'm not sure that this would be any more surprising. Repos on the command line would be isolated, but not with commandserver because of the global function wrapping. But I'm guessing most people don't understand that about commandserver.
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?
I'll take a look and see what I can figure out, but it likely won't be until next week. I think it should maybe be limited to a clone/unbundle adding a requirement with an implicitly loaded extension.