Bug 5902 - lfs crashes in TortoiseHg when enabled only at the repo level
Summary: lfs crashes in TortoiseHg when enabled only at the repo level
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: lfs (show other bugs)
Version: 4.6
Hardware: All All
: normal bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-29 22:31 UTC by Matt Harbison
Modified: 2018-06-12 00:00 UTC (History)
2 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Harbison 2018-05-29 22:31 UTC
I'm not sure if this is a thg bug for not isolating the extension, or an extension bug, because it operates on a repo that didn't get reposetup() called on it.  I suspect the latter.  I had a repo open in one tab with lfs enabled locally, and the Mercurial tab visible without lfs.  I clicked something in the Mercurial repo, and got this:

    #!python
    ** Mercurial version (4.6).  TortoiseHg version (4.6)
    ** Command: --nofork workbench
    ** CWD: C:\Users\mharbison\Desktop
    ** Encoding: cp1252
    ** Extensions loaded: eol, extdiff, mercurial_keyring, purge, rebase, schemes, share, shelve, strip, reviewboard, evolve,
tortoisehg.util.configitems, lfs
    ** Python version: 2.7.13 (v2.7.13:a06454b1afa1, Dec 17 2016, 20:53:40) [MSC v.1500 64 bit (AMD64)]
    ** Windows version: sys.getwindowsversion(major=6, minor=2, build=9200, platform=2, service_pack='')
    ** Processor architecture: x64
    ** Qt-5.9.2 PyQt-5.9.1 QScintilla-2.10.2.dev1711012104
    Traceback (most recent call last):
      File "tortoisehg\hgqt\revdetails.pyo", line 392, in _onFileSelected
      File "tortoisehg\hgqt\fileview.pyo", line 492, in display
      File "tortoisehg\hgqt\filedata.pyo", line 464, in load
      File "mercurial\patch.pyo", line 2356, in diff
      File "mercurial\patch.pyo", line 2471, in diffhunks
      File "mercurial\scmutil.pyo", line 1371, in prefetchfiles
      File "mercurial\util.pyo", line 2966, in __call__
      File "hgext\lfs\wrapper.pyo", line 252, in _prefetchfiles
    AttributeError: '_fncachevfs' object has no attribute 'lfslocalblobstore'

It should be simple enough to check for the attribute in most wrapper functions taking a repo or ctx, but I don't remember playing those games with largefiles.
Comment 1 Yuya Nishihara 2018-05-30 07:50 UTC
I think this can be reproduced in command-server session, so it's a bug of
the extension.
Comment 2 Matt Harbison 2018-05-30 10:23 UTC
OK, this should be easy enough to handle.

What about extension supplied revsets/filesets/templates?  I think those are registered globally like extsetup() wrappers, so it would seem like they would be available to other repos.  (I don't think the lfs ones are harmful like the missing attribute, but they wouldn't error out as unknown.)
Comment 3 Yuya Nishihara 2018-05-31 07:30 UTC
> What about extension supplied revsets/filesets/templates?

The same rule applies. We need to check the repo type or capability if
revsets/filesets/templates depend on the interface extended by reposetup().
But I think there's no need to error out as long as the output makes sense.
Comment 4 HG Bot 2018-06-04 18:40 UTC
Fixed by https://mercurial-scm.org/repo/hg/rev/3790efb388ca
Matt Harbison <matt_harbison@yahoo.com>
lfs: bypass wrapped functions when reposetup() hasn't been called (issue5902)

There are only a handful of methods that access repo attributes that are applied
in reposetup().  The `diff` test covers all of the commands that call
scmutil.prefetchfiles().  Along the way, I saw that adding files and upgrading
the repo format were also problems (also tested here).

I don't think running `hg serve` through the commandserver is sane, but I
conditionalized both the capabilities and the wsgirequest handler because it's
trivially correct.  It doesn't look like there has ever been a caller of
candownload(), so there's no test for that path.

The upload case isn't testable, because uploadblobs() bails if there are no
pointers.  The requirement should be added any time pointers are introduced, and
that would force the extension to be loaded specifically for the repo.  This
covers `debuglfsupload`, the pre-push hook (which isn't set until the repo is
promoted to LFS), and uploadblobsfromrevs(), which can be called by other
extensions.

I think readfromstore() and writetostore() are only reachable as a flag
processor for revlog.REVIDX_EXTSTORED, and a requirement is added as soon as
that is seen, so I don't think those are a problem.

(please test the fix)
Comment 5 Bugzilla 2018-06-12 00:00 UTC
Bug was set to TESTING for 7 days, resolving