search

vapoursynth / vsrepo

A simple package repository for VapourSynth
MIT License
113 stars 29 forks source link

Crashing with RecursionError #154

Open dixie-flatliner opened 3 years ago

dixie-flatliner commented 3 years ago

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.

theChaosCoder commented 3 years ago

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

myrsloik commented 3 years ago

Is this with the R54 vsrepo or master?

theChaosCoder commented 3 years ago

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

theChaosCoder commented 3 years ago

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

theChaosCoder commented 3 years ago

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

theChaosCoder commented 3 years ago

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

myrsloik commented 3 years ago

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 commented 3 years ago

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.

theChaosCoder commented 3 years ago

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 commented 3 years ago

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

dixie-flatliner commented 3 years ago

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.

myrsloik commented 3 years ago

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.

theChaosCoder commented 3 years ago

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... 😑

myrsloik commented 3 years ago

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