inject environment for verbs

[plata]

It is currently not possible to get resources for a verb (e.g for the registry, see #511).

Decide if it makes sense to inject environment to solve this.

The basic issue is the following: A script/verb is not aware of its location in the repository.

[plata]

@madoar let's continue here.

[plata]

1. Injecting when a script is executed: This is already done for TYPE_ID etc. I don't see how it could be used for a verb.
2. Injecting in include call: Could be possible but how should the variables look like, i.e. how to avoid that several includes overwrite each other?

[madoar]

I believe the question is, how do we define a verb. Is it an independent environment, or is it a part of a script and should therefore also be able to access variables of the script environment.

[plata]

It is part of the script.

[madoar]

1) Aren't the verbs loaded in the same nashorn engine as the script itself? If this is the case they should share the same global variables. 2) The IncludeInjector currently prevents multiple includes from overwriting each other:

            if (includedScripts.add(Arrays.asList(arguments))) {
                nashornEngine.eval("//# sourceURL=" + Arrays.asList(arguments).toString() + "\n" + script,
                        this::throwException);
            }

`includedScripts.add(Arrays.asList(arguments))` returns false, when the script, defined by the arguments, is already included.

[plata]

1. yes
2. yes but that's not the point: How should the environment variables look like? If they are global and have the same name, they will overwrite each other.

[madoar]

Is this really a problem? I mean if we take variables like TYPE_ID or CATEGORY_ID as global variables, aren't they constants and don't change independent of how many times you load them for a single script?

[plata]

Sure but that doesn't help at all. If we want to use resources for verbs we need something like APPLICATION_ID for the verb (and for every included verb).

[madoar]

In addition I think we should move the location, where we set TYPE_ID, CATEGORY_ID, APPLICATION_ID and SCRIPT_ID from inside the AppsController class to a new EngineInjector class, to be more consistent, with the other injected functionality (i.e. include, Bean, tr, ...).

[madoar]

So you mean we would need to add a kind of VERB_ID variable? I'm not sure why we would need such a variable?

[plata]

The purpose of all this is to access resources which are located relative to a verb. To to this, we need a VERB_ID (doesn't help if we know where the application is located).

[plata]

In the end this whole approach is a bit pointless anyways because you can simply use the same strings which are used in the include to get the resource.

[madoar]

Yes. I'm not sure if location is something, which should be injected from outside. I mean when writing a verb or script, I know where it's located (in the include syntax).

[plata]

Exactly. However, it is important to notice that this works only because verbs do not have special characters in their names.

[plata]

I saw earlier on that Lutris does not only have a name for a script but also a slug (what we would call "id"). If we would do something similar, we could just include by using that id. This would also make the fixed 4 level namespace unnecessary.

[madoar]

Yes, but I think we can assume this.

[madoar]

The question here is, how to define this id? Normally a verb is just a single function, which is added to the wine prototype. To define an id we would need some kind of metadata.

[plata]

Could be just a variable inside the verb or in a verb.json.

[madoar]

I don't like the approach of defining the id inside a separate file. The problem with a variable defined inside the verb is again the overwriting issue, which occurs when you're loading multiple verbs.

[plata]

I could imagine something like:

{
    "verbName": ".NET 4.0",
    "id": "wine/verbs/dotnet40"
    "description": "installs .NET 4.0"
}

It would be used like:

include("wine/verbs/dotnet40")

We could do the exact same thing for apps. This would also have the advantage that the ID doesn't magically pop up from nowhere. In addition, it would solve PhoenicisOrg/POL-POM-5#1179.

[madoar]

For applications, how would this change the folder structure? You would add the json object to a new file, right?

[plata]

No, I would just add the id to script.json. Actually we could just use the script.json for both apps and verbs.

[plata]

Thinking about it, I really start liking the idea. It would (almost) make verbs first class citizens as well and unify the structure a bit more.

[madoar]

Yes, this sounds like an approach we should explore.

[plata]

I would really like to hear @qparis opinion about this. However, he seems to be quite busy lately if you look at the other issues pending on his input.

[plata]

How would we handle #534 with this approach? I.e. how to get all verbs for a given engine? Just like getApplication? Or is the concept of type/category/application superfluous if we can retrieve a list of scripts for any given slug (e.g. "/application/games" or "/engines/wine/verbs" or "/engines/wine/verbs/dotnet40")?

[madoar]

If we solve this issue by adding a script.json file for every verb (which I like more and more), the script.json file needs to contain some kind of reference to which engine type it belongs. I'm not sure if this reference should be contained inside the id/slug of the verb or if we should add an additional field (i.e. engineType) for it.

On another note, a reference to the required engine type would be useful for applications too, especially if we want to make the engines changeable later on.

[plata]

If the engine is not part of the id, how would you imagine the id to look like? Only a name e.g. "dotnet40"? I think it is dangerous not to scope it.

(Don't get me wrong: I'm not criticizing, I just try to get a feeling how it could work.)

[madoar]

I could live with a unique name for the id/slug, that doesn't require anything else than being unique. Scoping the id in my opinion has two problems:

• first, we would need to check/validate the ids. This can be hard. Because we would need to define a scheme, which the ids need to implement.
• second, we would add an additional "rule" (the scheme) a script developer needs to know and to follow to write a new script.

I'm not sure how important crucial both points are.

[madoar]

Another feature we could implement with a script.json file is a required minimum version of the engine, which is needed to use the verb (this has been suggested by @ImperatorS79 in #511)

[plata]

In fact, we have the scoping already because the include requires tpye, category and application. Therefore, I don't see a big impact going from ["Applications", "Games", "AoEII", "Steam"] to "/applications/games/aoeII/steam". With unique (un-scoped) names, I see the problem of duplication. For example, we currently have already an app and a verb for Uplay. Of course, this is no problem if you do include(["Applications", "Games", "Uplay", "Online"]) and include(["Engines", "Wine", "Verbs", "Uplay"]). If we want to do include("applications/games/uplay/online") to avoid the fixed 4 level namespace (not sure if it makes sense), it would be better to have the whole slug as id directly.

[madoar]

If we think about improving the whole naming part of scripts, maybe we should also change the way scripts are fetched via include.

As an alternative approach we could build a map from script id to script url during startup. For this we would need to iterate through the whole script repository/folder once. Afterwards whenever a script needs to be fetched, we can just do a simple lookup. This would allow us to circumvent the problem.

[plata]

I don't think script URL would work. We must use the script repository (in the Java application).

Also, I'm still unsure if it makes sense to do this. On the one hand, it has some advantages e.g. that all information for include is available in every script. On the other hand, it encodes the hierarchy in the id instead of a class hierarchy.

[madoar]

URL was the wrong word, I meant URI. I believe we need a reference pointing to the file containing the script, independent of where it is located.

[plata]

I can't quite imagine how this would work. Could you describe what include would do with your proposal step by step?

[madoar]

Basically the include method would then look like this:

    @Override
    public void injectInto(NashornEngine nashornEngine) {
        // a map referencing the slugs to their corresponding script file
        final Map<String, URI> knownScripts = loadAllScripts();

        nashornEngine.put("include", (Consumer<ScriptObjectMirror>) args -> {
            final String slug = args.to(String.class);

            URI scriptUri = knownScripts.get(slug);

            // I guess we would need to add the next method to scriptFetcher
            final String script = scriptFetcher.getScript(scriptUri);
            if (script == null) {
                throwException(new ScriptException(slug + " is not found"));
            } else {
                nashornEngine.eval("//# sourceURL=" + slug + "\n" + script,
                        this::throwException);
            }
        }, this::throwException);
    }

The idea behind the approach is, to statically load all scripts once, i.e. at Phoenicis startup. Afterwards only a lookup in a map is required. In addition this approach would automatically prevent us from loading scripts multiple times.

[plata]

Shouldn't this be in #543?

[plata]

Actually I think that this issue can be closed because we have the IDs available now.

[madoar]

You're right about closing this. Should I copy the code to #543?

[plata]

Yes, please.