vsrepo icon indicating copy to clipboard operation
vsrepo copied to clipboard

Crashing with RecursionError

Open dixie-flatliner opened this issue 2 years ago • 14 comments

Fresh install of Vapoursynth, fresh install of Python 3.9, fresh install of Windows.

Lots of packages seem to trigger a Recursion Error with the traceback in crash_a.txt. "upgrade-all" triggers one with the contents of crash_b.txt. There's no clear common denominator (some fail, some don't), and the messages are exactly the same regardless of the script. Have also tested with both the script provided in the official installer, and the latest commit from this repo. Notably the example command (vsrepo.py install havsfunc ffms2 d2v) is triggering the former.

crash_a.txt crash_b.txt

Apologies if this is some form of PEBKAC scenario I've created.

dixie-flatliner avatar Sep 16 '21 21:09 dixie-flatliner

Same happend to me with latest python 3.9 + VS R54 stable in Windows-Sandbox (basically a fresh windows).

theChaosCoder avatar Sep 17 '21 09:09 theChaosCoder

Is this with the R54 vsrepo or master?

myrsloik avatar Sep 17 '21 09:09 myrsloik

vsrepo master. R54 does not work bcs of the agent thingy.

theChaosCoder avatar Sep 17 '21 09:09 theChaosCoder

It seems it only happens with packages where havsfunc is a dependency. I will test later more.

theChaosCoder avatar Sep 17 '21 09:09 theChaosCoder

Removing "nnedi3_resample" from havsfunc as a dependency "fixes" the installation. Still unclear why it happens.

theChaosCoder avatar Sep 17 '21 10:09 theChaosCoder

And removing muvsfunc from nnedi3_resample "fixes" it also... there is some kind of dependency-check-loop created here.

This happens also on my pc with everyhing = latest pre-realese version.

compile first, then don't forget to extract the vspackages3.json (<= this really is annoying, we should either keep the generated json or make a "keep" parameter so it does not get deleted on compile. I always forget it while testing packages...)

use portable installation python vsrupdate.py compile python .\vsrepo.py -p -t win64 install nnedi3_resample -s vscripts

theChaosCoder avatar Sep 17 '21 10:09 theChaosCoder

nnedi3_resample => muvsfunc => havsfunc => nnedi3_resample

Simple rule: smaller wrappers like nnedi3_resample shouldn't depend on the gigantic script collections.

Additional detail: nnedi3_resample probably shouldn't rely on the old nnedi3 since znedi3 is a perfect replacement. Could also apply to other scripts.

myrsloik avatar Sep 17 '21 10:09 myrsloik

And nnedi3_resample only uses muvsfunc.Depth. This could be trivially done with the internal resizer but apparently we need 2 extra dependencies there. Sigh... I really don't feel like implementing a circular dependency detector so I suggest we drop muvsfunc as a nnedi3_resample dependency for now.

myrsloik avatar Sep 17 '21 11:09 myrsloik

Yes lets drop muvsfunc.

I don't really know how dependencies are currently handled in vsrepo, but I would build an install list first (going through each package only once) and then you basically only need to trigger the dl function for each item in the list.

theChaosCoder avatar Sep 17 '21 12:09 theChaosCoder

@dixie-flatliner Please run vsrepo update again and check if everything is ok now.

theChaosCoder avatar Sep 17 '21 12:09 theChaosCoder

Indeed it is. Sorry for making a report here for what wound up being a problem with a script.

@theChaosCoder : that install list idea is sounds really logical.

dixie-flatliner avatar Sep 17 '21 12:09 dixie-flatliner

The list idea is only worth it if we're going to allow circular dependencies. I'm not sure we should. If anything I'd rather add vsrupdate code to verify there are no circular references.

myrsloik avatar Sep 17 '21 13:09 myrsloik

Yeah some kind of check would be good or this will happen again someday.

I just noticed that the dependency is mvsfunc and not muvsfunc... 😑

theChaosCoder avatar Sep 18 '21 21:09 theChaosCoder

These are great module names! I've never confused myself either! Will add some check for this later. Keeping this issue as a reminder.

myrsloik avatar Sep 18 '21 21:09 myrsloik