Make compiler aware of libraries that are dependencies of dependencies

[cboulanger]

This is related to #344

Dependencies of contribs are downloaded, but not recognized by the compiler. If we don't want the dependencies of contribs (and their dependencies etc.) pollute contrib.json, we need to make the compiler aware of those.

[hkollmann]

The compiler is aware of libraries that are dependencies of dependencies. See Compile.js, __checkDependencies

[cboulanger]

We need a good test case. I'll create two test contribs that we can use for this

[cboulanger]

Ok, maybe it is a good idea to proceed in small steps and agree on what behaviour we want to have, @hkollmann & @johnspackman ?

I have created repos qxl.test1 (with a single library qxl.test1) and qxl.test2 (with 4 libraries qxl.test2A, ...B, ...C, ...D and a qooxdoo.json). As per Manifest.json/requires, qxl.test1 has a dependency on qxl.test2/test2C, and qxl.test2A depends on ...B, which depends on ...C, which depends on ...D.

The libraries have not yet been published to the contrib registry.

No contrib.json has been created yet. Since this file is created when you do qx contrib install, this is an artificial situation, but one that we should deal with.

First step:

If I check out the repo qxl.test1 and to qx compile, I get

Error: TypeError: Cannot read property 'qooxdoo/qxl.test2C' of undefined
    at libs.find.entry (/Users/cboulanger/Code/qooxdoo-compiler/lib/qx/tool/cli/commands/Compile.js:637:94)
    at Array.find (native)
    at libs.forEach.lib (/Users/cboulanger/Code/qooxdoo-compiler/lib/qx/tool/cli/commands/Compile.js:637:30)
    at Array.forEach (<anonymous>)
    at defaultConstructor.__checkDependencies (/Users/cboulanger/Code/qooxdoo-compiler/lib/qx/tool/cli/commands/Compile.js:603:12)
    at <anonymous>

The error is completely predictable (since the setup is wrong), but I think we should catch it better.

[cboulanger]

Ok, this is taken care of by https://github.com/qooxdoo/qooxdoo-compiler/commit/253a41d05ff841dff778fd93c5c3f61f60041a37. Next: I added source code dependencies between qxl.test1 -> qxl.test2C and qxl.test2A -> qxl.test2B -> qxl.test2C -> qxl.test2D

in qxl.test1:

qx clean && qx list && && qx compile
qxl.test1.Application: [71,10] Unresolved use of symbol qxl.test2C.Application

What should be the behavior:

1. leave as is
2. better error message ("Please install...")
3. automatically install from Manifest.json/requires data

[oetiker]

we have this duality between Manifest.json/requires and contrib.json ... AND also implicit requirements from the source itself ... especially having things in Manifest.json and contrib.json seem a tad over-defined ...

having stuff automatically installed by 'compile' sounds convenient ... but also a bit 'quick' I would rather have a separate command for that or at least an option in compile. In this way odd behaviour for situations without network access would be prevented

[cboulanger]

I see it this way:

• The source code only contains the well-known name of the dependency, without version information (corresponds to require("..."); in npm land)
• Manifest.json corresponds to package.json: author-defined minimal version -> will create a contrib.json with these versions where one doesn't exist
• contrib.json corresponds to package-lock.json: constraints-enforced actual version (might be higher than what Manifest.json requires). It should be generated on demand and be optional. It can (but need not) be committed to VCS.

You're right about network access. We could either

• use a CLI option such as --download to automatically install dependencies or
• do it interactively (unless --noninteractive is passed) "Do you want to download ...."

(edited for syntax)

[hkollmann]

The current version of the compiler tries to install missing libs if found in contrib.json.

if (!data.libraries.every(libData => fs.existsSync(libData + "/Manifest.json"))) {
        console.log("One or more libraries not found - trying to install them from library repository...");
        await (new qx.tool.cli.commands.contrib.Install({quiet:true})).process();
      }

[oetiker]

ok ... the distinction makes sense ... thanks ... I prefer commandline tools not to be interactive :) so my favorite would be for compile to complain and mention that if called with --download it would try to get the dependencies from the net

if a 3rd party library gets required by a dependency, how is the version determined ? also Manifest.json first and overriden by the dependencies contrib.json (of the dependency if present?)

and big complaint different versions are in contrib.jsons ?

[cboulanger]

@hkollmann Yes, that's true, but I would prefer if we kept contrib.json somewhat secondary and opaque (so that we can always change the syntax (or the name, see #279) of it should the need arise). I think the only user-facing data structures should be Manifest.json and compile.json. contrib.json should be generated on demand and contain reproduceable data. This would correspond to the package.json and package-lock.json distinction, which is already known to people.

[hkollmann]

You are right. The question is should contrib.json contain all depended libs, included the dependecies of dependencies. This would be much faster and easier to parse during version checks of the compiler.

BTW package-lock.json contains the dependend libs,

[cboulanger]

Maybe it would make sense to include a schema-version entry in our config files, so that changes to the schema can be easily noticed. Or we use the compiler version as schema version and then keep track of at which compiler version the schema changes.

[oetiker]

"$schema" as per json schema rules would be cool

[cboulanger]

@hkollmann Yes that's an important question. We should never pollute Manifest.json with deps of deps (I don't think we do currently, or do we?), but we could do it with contrib.json.

[hkollmann]

manifest.json should only mention the mandatory needed libs.

[cboulanger]

Just realized: when calculating dependencies (including recursive dependencies), the compiler should probably only look at Manifest.json files, and not (as it currently does) at the contrib.json files. I think contrib.json should only store the result of a dependency calculation for the particular project, i.e. application.

[cboulanger]

That would really clear up the confusion about why we have two files. Manifest.json is per library, contrib.json is per project. Does that make sense to you?

[oetiker]

or contrib.json is like package.lock ... what is the difference between project and library?

[cboulanger]

As I would use the term, the project is the particular app that you are working on, which has dependencies on libraries. For your project/app, it makes sense to keep a contrib.json which records the very particular set of library versions. For a library, it does not, it needs only record the minimal versions of the libraries it needs to function. At least, that is how it makes sense for me.

[hkollmann]

Other idea: For libraries contrib.json contains lists all stuff for demo app or api-viewer. contrib.json is not used in apps. manifest.json declares all needed sub dependencies.

[cboulanger]

@hkollmann That's the opposite of what I've been arguing for 😁 ... I think the reverse makes more sense. Dependencies are calculated at compile time, which is only relevant for projects/apps - because libraries are not compiled, just their demos are or the apps that use the libraries. That's why I would see compile.json and contrib.json to belong together, although compile.json is original data, whereas contrib.json is derived data. You should always be able to discard contrib.json and regenerated it from the Manifest.json data of all libraries used.

[cboulanger]

Of course, applications ARE libraries, but not the reverse ... that's why you can have libraries without compile.json, contrib.json, but not without Manifest.json, whereas an application must have all of them (if it uses contribs).

[cboulanger]

Think of contrib.json as package-lock.json, a cached version of very particular configuration of library versions, which is there to preserve a state (so that different people will get the same state when they check out and compile the application code). This is not possible with Manifest.json since it records only the minimal requirements of a particular library, and in the specific requirements of an application you might get a situation where a higher version of a library is required.

[johnspackman]

@cboulanger :+1:

While contrib.json can be discarded and rebuilt by qx, even compile.json can be disregarded an rebuilt from scratch by the user - in some cases it will be, for example when creating a new project using the same contribs.

compile.json is a kind of transient information, about how the developer wants to compile some apps for a given use case / project, contrib.json is about how qx connects the contribs to the compiler, but it's Manifest.json that's critical to describe libraries and their dependencies and is entirely hand edited by the developer of that library.

[cboulanger]

Thinking aloud here:

• the app requires bar and baz
• bar requires goo@^1.0.0
• baz requries goo@^1.5.0 -> the app requires goo@^1.5.0, this needs to go into contrib.json, but has no relevance for either bar or baz

[cboulanger]

@johnspackman you are right, but compile.json will also contain hand-edited information (for example, targets, includes, etc., environment variables), which cannot be retrieved from anywhere else, are therefore not redundant the same way as the information in contrib.json is.

[hkollmann]

@cboulanger : We are not opposite. I agree fully to your arguments.

libs can be applications so they can contain contrib.json and compile.json.

[oetiker]

why does compile have to be seperate from Manifest ?

[johnspackman]

because compilation has nothing to do with libraries/contribs. An application is just a contrib/library which also has a compile.json, but actually that's just convention. You can have compile.json on it's own, with no javascript in the current directory tree, or in fact anything that looks like a qooxdoo application directory. You add to the libraries:[] in compile.json and the compiler does the rest (I do this when generating applications across multiple projects ready for deployment).

Manifest.json is about the library, what version it is, what it requires, etc

[derrell]

We seem to have confusion about names, and I'm wondering if we need to reinvent the wheel here. Given that two of these names map directly to something related to npm, what if we used the exact same names:

• package.json instead of Manifest.json
• package-lock.json instead of contrib.json

We then only have compile.json which is unique to qooxdoo and describes how to build an application or set of applications.

[derrell]

Oh, we might actually have a conflict in that case... but how about qxpackage.json and qxpackage-lock.json for the two that match npm's usage?

[cboulanger]

@derrell I think Manifest.json is a perfectly good and established name - I don't think it would be wise to change it. As to contrib.json, I think we should change the name eventually to reflect that this is about libraries in general, not only "contributions". But let's do this later. IMHO as long there is good documentation, it does not really matter how we name things.

[cboulanger]

@johnspackman 👍 I also think the separation makes good sense. compile.(js/json) contains compiler instructions, Manifest.json library information. In sum, I think what we have is good, we just need to document it well (and create those darn json-schema files I've been meaning to provide for so long) 😎

[cboulanger]

I think all the major points in this issue have been adressed. Please open new issues for anything that you feel hasn't been solved.