implementing script metadata and DAG sorting mechanism #13944

wfjsw · 2023-11-11T09:51:46Z

Description

NOTE: This PR directly conflicts with #13943

BREAKING: graphlib is a Python 3.9 thing. It may breaks on Python 3.8.

Implemented an extension metadata system with configparser
In the extension metadata file, extensions are allowed to
- Specify a canonical name, which will be used to uniquely define an extension and to prevent an extension from being load twice. This is optional. The default value is the directory name.
- Specify that the extension or a specific file Requires another extension or a file of the same type. The extension/script will be disabled if the requirement does not meet.
- Specify that a specific file wants to load After another extension or a single file of the same type.
- Specify that a specific file wants to load Before another extension or a single file of the same type.
When a circular dependency is detected, it will collapse into filesystem order.

When specifying individual script's Requires, Before and After, use either:

Extension's canonical name, or
<extension_canonical_name>/<script_file_name> (note the directory name is not included)

~~I have not tested this yet. Will do it later.~~

~~This system is built upon the implication that script files has unique name. Please let me know if this is not the case.~~

Example ~~sd_webui_metadata.ini~~ metadata.ini:

[Extension]
Name=adetailer
Requires=sd-webui-controlnet sd-webui-animatediff

[scripts]
Requires=sd-webui-controlnet sd-webui-animatediff
Before=sd-webui-controlnet

[scripts/!adetailer.py]
Requires=sd-webui-controlnet sd-webui-animatediff
Before=sd-webui-controlnet
After=sd-webui-animatediff/animatediff.py

[javascript]
After=ui.js

[javascript/canvas.js]
After=localization.js

Checklist:

I have read contributing wiki page
I have performed a self-review of my own code
My code follows the style guidelines
My code passes tests

…atively

w-e-w · 2023-11-11T10:17:23Z

This system is built upon the implication that script files has unique name. Please let me know if this is not the case.

sadly that is Faaaaaaaar from the case

examples of extensions using generic names

this is not all of them

yes in lots of cases these are files that are not meant to be imported by others extensions
but there's also lots of instances of people using main.py as the name of the script file

wfjsw · 2023-11-11T10:40:54Z

I use this condition because afaik scripts will step on each other anyway if names collide (or this has been fixed already? I dunno). If this is not fixed I'd rather make the error explicit. Or I can add some scope here.

w-e-w · 2023-11-11T11:20:01Z

Tl;DR is that duplicate name will prevent a later scripts from importing the correct script

currently if there is a duplicate script file

every one of those files will be executed (so far no problem)

the problem comes when a script tries to "import" a file that has duplicate names from another file
when importing it will always import the first one thats loaded

example: as I have 2 ext
ext_1-script_1.py and ext_1-script_2.py
script_2 imports script_1

ext_2-script_1.py and ext_2-script_2.py
script_2 imports script_1

execute:
ext_1-script_1.py ok
ext_1-script_2.py ok import script_1.py found ext_1-script_1.py ok

ext_2-script_1.py ok
ext_2-script_2.py ok import script_1.py found ext_1-script_1.py NOT_OK

but if later script does not import the script with duplicate name then all is fine

ext_2-script_1.py and script_2.py
script_2 imports script_1

ext_2-script_1.py and ext_2-script_2.py

execute:

ext_1-script_1.py ok
ext_1-script_2.py ok import script_1.py found ext_1-script_1.py ok

ext_2-script_1.py ok
ext_2-script_2.py ok

w-e-w · 2023-11-11T13:59:19Z

just realized that we already have a good candidate to be used as the unique identifier name for an extension
currently in the index every extension is listed under a unique file-name.json
https://github.com/AUTOMATIC1111/stable-diffusion-webui-extensions/tree/extensions/extensions

wfjsw · 2023-11-11T16:15:33Z

But it is not possible to match a canonical name between an already installed extension and this file.

w-e-w · 2023-11-11T16:52:14Z

But it is not possible to match a canonical name between an already installed extension and this file.

the extension install directly cannot be used as a means of identification

even though by default an extension is installed using the repo name
the name can be modified by the user
and this is not including situations that the extension repo name is just bad see Catppuccin Theme

I've mentioned in past I wish to make the install directory unique possibly by adding a Owner sufix, but the idea hasn't coming to fruition because some extension uses bad method of importing libraries form other extension

currently I do not believe there is a definitive method of identifying an extension
we need an extension to declare its identity

and it also doesn't really matter that pre-existing extension cannot be identified
if extension isn't updated with metadata the benefit of this system is already half gone
as it will be basically guesswork to generate the proper load order

eventually extension will be updated with the metadata, so it shouldn't be an issue after a while

if it is really necessary, it is possible for us to implement a system that will automatically assign a identifier by matching the git url

after the load order system is fully setup extension will start updateing and providing the metadata

I still prefer json over ini
json allows the storage of more complex data types where as ini is only strings and need manual conversion

you mentioned fearing that people might json file, the user should not edit this file this file should be only edited by the developer
if any configuration needs to be done it should be done via tha UI an information is stored in settings
if a corruption does happen the error will be immediately apparent, as a message can be shown in the UI and can be fix by the developer before publishing

in my opinion json is more versatile as it supports different data types
it is also easy to read what indentation is used
I do have plans using metadat for other use not just for load order

wfjsw · 2023-11-11T18:00:00Z

Some observation:

Topological sort would put any file that has a dependency relation to the bottom of the list.

[Extension]
Name=adetailer

[scripts]
After=sd-webui-controlnet/controlnet.py
Before=sd-webui-controlnet/external_code.py

AUTOMATIC1111 · 2023-11-19T06:25:30Z

Do we have any users with 3.8 (probably some google colab-like services)?

The syntax for requirements as it's written seems to make it impossible to use spaces in names.

I would prefer a more simple name for the metadata file, like metadata.ini. Is ini even a good choice for a file like that?

Is it possible to just move files that have dep requirement in the order of loaded scripts to their correct location, without putting them all at the end?

Also I'd rather emit warnings and still attempt to load extensions with missing dependencies rather than forbid it outright.

wfjsw · 2023-11-19T06:35:23Z

Colab upgraded to Python 3.10 already. but we could always opt to try and catch a ImportError and do fallback here.
Why'd we want spaces in name? This is an internal identifier rather than a display name and currently it's already hard to use spaces given the git repo name restriction. If display name is needed there could be a new field.
I'm fine with that. Given ini is capable of handling strings, boolean and numbers I don't know anything else would happened to be needed here.
It is possible to imply a default After relation based on the directory names but I don't know if it will accidentally create loops. Also it would cause ambiguities.
I guess the extension would crash anyway if they decided to assume the required extension is there and import it... but I'm fine with this.

w-e-w · 2023-11-19T06:46:14Z

I would prefer a more simple name for the metadata file, like metadata.ini

I would argue against using a common filename like metadata
as there's a chance of the filename name is already used for other purposes

Is ini even a good choice for a file like that?

ini is capable of handling strings

I also don't prefer ini, even it's support handles the data types we need, ini it's not really convenient to have structures
such as list or dict
there's the possibility of extending the metadata for other use in the future, so having the format more structure is also beneficial

Also I'd rather emit warnings and still attempt to load extensions with missing dependencies rather than forbid it outright.

I guess the extension would crash anyway if they decided to assume the required extension is there and import it... but I'm fine with this.

I agree on this point
in some cases certain extensions would have extra functions when other extensions is installed, but still would work without them
is like a "soft" requirement, naturally for these extra function to work this extension would have to be loaded after their dependencies, but they also have error handling when those extensions do not exist

I'm not sure maybe this already works but just not specify require only after

wfjsw · 2023-11-19T06:47:33Z

I'm not sure maybe this already works but just not specify require only after

Requires and After are independent and none of it implies the other.

w-e-w · 2023-11-19T06:51:03Z

I'm not sure maybe this already works but just not specify require only after

Requires and After are independent and none of it implies the other.

nice

wfjsw · 2023-11-19T06:57:06Z

(Some of the design actually comes from systemd.service and that's the main reason I choose ini.)

w-e-w · 2023-11-19T07:25:08Z

I would prefer a more simple name for the metadata file, like metadata.ini

I would argue against using a common filename like metadata
as there's a chance of the filename name is already used for other purposes

example of how it might look using json

fill path

{
    "extension": {
        "name": "adetailer",
        "requires": ["sd-webui-controlnet", "sd-webui-animatediff"]
    },
    "scripts": {
        "requires": ["sd-webui-controlnet", "sd-webui-animatediff"],
        "before": ["sd-webui-controlnet"]
    },
    "scripts/!adetailer.py": {
        "requires": ["sd-webui-controlnet", "sd-webui-animatediff"],
        "before": ["sd-webui-controlnet"],
        "after": ["sd-webui-animatediff/animatediff.py"]
    },
    "javascript": {
        "after": ["ui.js"]
    },
    "javascript/canvas.js": {
        "after": "localization.js"
    }
}

nested

{
    "name": "adetailer",
    "requires": ["sd-webui-controlnet", "sd-webui-animatediff"],
    "scripts": {
        "!adetailer.py" : {
            "requires": ["sd-webui-controlnet", "sd-webui-animatediff"],
            "before": ["sd-webui-controlnet"],
            "after": ["sd-webui-animatediff/animatediff.py"]
        }
    },
    "javascript": {
        "canvas.js": {
            "after": "localization.js"   <---when there's only one then list [] is not required
        }
    }
}

AUTOMATIC1111 · 2023-11-20T11:52:50Z

I changed a lot in implementation to use classes with named fields instead of dictionaries, use a custom sorter that doesn't change the order as much and ignores cyclic reference errors (which I see as a preferable solution: if one extension adds some messed up order, that shouldn't break nice ordering of all other extensions), and eliminated some duplicate code.

wfjsw requested a review from AUTOMATIC1111 as a code owner November 11, 2023 09:51

implementing script metadata and DAG sorting mechanism

0fc7dc1

wfjsw force-pushed the dag branch from db48f6d to 0fc7dc1 Compare November 11, 2023 10:01

wfjsw added 2 commits November 11, 2023 04:03

populate loaded_extensions from extension list instead

0d1924c

reverse the extension load order so builtin extensions load earlier n…

bc1a450

…atively

w-e-w mentioned this pull request Nov 11, 2023

Extension / Script load and execution order system #13943

Closed

4 tasks

remove the assumption of same name

7af576e

allow comma and whitespace as separator

520e52f

wfjsw marked this pull request as draft November 11, 2023 17:13

wfjsw added 2 commits November 11, 2023 11:17

fix

48d6102

bug fix

3bb32be

wfjsw marked this pull request as ready for review November 11, 2023 17:58

use metadata.ini for meta filename

bde439e

AUTOMATIC1111 approved these changes Nov 20, 2023

View reviewed changes

AUTOMATIC1111 merged commit 73a0b4b into AUTOMATIC1111:dev Nov 20, 2023
3 checks passed

w-e-w mentioned this pull request Dec 4, 2023

1.7.0-RC #14196

Closed

w-e-w mentioned this pull request Dec 16, 2023

1.7.0 #14323

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implementing script metadata and DAG sorting mechanism #13944

implementing script metadata and DAG sorting mechanism #13944

wfjsw commented Nov 11, 2023 •

edited

Loading

w-e-w commented Nov 11, 2023 •

edited

Loading

wfjsw commented Nov 11, 2023 •

edited

Loading

w-e-w commented Nov 11, 2023 •

edited

Loading

w-e-w commented Nov 11, 2023

wfjsw commented Nov 11, 2023

w-e-w commented Nov 11, 2023

wfjsw commented Nov 11, 2023

AUTOMATIC1111 commented Nov 19, 2023 •

edited

Loading

wfjsw commented Nov 19, 2023 •

edited

Loading

w-e-w commented Nov 19, 2023 •

edited

Loading

wfjsw commented Nov 19, 2023

w-e-w commented Nov 19, 2023

wfjsw commented Nov 19, 2023

w-e-w commented Nov 19, 2023 •

edited

Loading

AUTOMATIC1111 commented Nov 20, 2023

implementing script metadata and DAG sorting mechanism #13944

implementing script metadata and DAG sorting mechanism #13944

Conversation

wfjsw commented Nov 11, 2023 • edited Loading

Description

Checklist:

w-e-w commented Nov 11, 2023 • edited Loading

wfjsw commented Nov 11, 2023 • edited Loading

w-e-w commented Nov 11, 2023 • edited Loading

w-e-w commented Nov 11, 2023

wfjsw commented Nov 11, 2023

w-e-w commented Nov 11, 2023

wfjsw commented Nov 11, 2023

AUTOMATIC1111 commented Nov 19, 2023 • edited Loading

wfjsw commented Nov 19, 2023 • edited Loading

w-e-w commented Nov 19, 2023 • edited Loading

wfjsw commented Nov 19, 2023

w-e-w commented Nov 19, 2023

wfjsw commented Nov 19, 2023

w-e-w commented Nov 19, 2023 • edited Loading

AUTOMATIC1111 commented Nov 20, 2023

wfjsw commented Nov 11, 2023 •

edited

Loading

w-e-w commented Nov 11, 2023 •

edited

Loading

wfjsw commented Nov 11, 2023 •

edited

Loading

w-e-w commented Nov 11, 2023 •

edited

Loading

AUTOMATIC1111 commented Nov 19, 2023 •

edited

Loading

wfjsw commented Nov 19, 2023 •

edited

Loading

w-e-w commented Nov 19, 2023 •

edited

Loading

w-e-w commented Nov 19, 2023 •

edited

Loading