movingblocks / gestalt Goto Github PK

• Is it worth keeping the existing VersionRange class at all, or should we convert entirely?
• Instead of building an expression out of minVersion, should we allow module metadata to define its own expression?
  • We would need to use the Maven-style expressions in that case in order for compile-time dependency resolution to still work.

Follow-up from #101.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Now that IntelliJ checks dependencies against a vulnerability database, we see gestalt pulling in some dependencies with known CVEs. Either directly or transitively through things like Reflections.

We should update the dependencies or add constraints to the transitive dependencies, as appropriate.

Related to MovingBlocks/Terasology#5021

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Gradle 7 has some features Terasology is interested in (e.g. version catalogs). Since the MovingBlocks/✳ libraries are often used in composite builds with Terasology, it seems like a good idea to keep their gradle versions from drifting too far apart.

I propose upgrading all the libraries to the latest in the 6.x line (currently 6.9.0) and fixing whatever deprecation warnings crop up there. That should give us a smooth path to then adopting Gradle 7.

• Create Entity
• Add Component
• Update Component
• Remove Component
• Retrieve Component
• Stored component distinct from retrieved component

• In-memory object storage
• Orientdb
• Other
• Benchmark

• AssetType#reload calls loadAsset within a AccessController.doPrivilege block
• AssetType#createInstance does not call loadAsset within a AccessController.doPrivileged block

org.terasology.gestalt.assets.module.autoreload.ModuleEnvironmentWatcherTest.test()

It sometimes passes, and sometimes on CI it does not

org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:35)
	at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:162)
	at org.terasology.gestalt.assets.module.autoreload.ModuleEnvironmentWatcherTest.test(ModuleEnvironmentWatcherTest.java:91)

Currently, only module names can be used to resolve modules. When multiple versions of a module are available, it might be useful to resolve a particular version (or a range?) of a module.
For example, someone might want to play v0.9 even if v1.0 is already available.

The master-server uses a workaround based on a fake module to resolve a specified version of a module:

ModuleMetadata meta = new ModuleMetadata();
Name fakeName = new Name("fake");
meta.setId(fakeName);
meta.setVersion(new Version(1, 0, 0));
DependencyInfo di = new DependencyInfo();
di.setId(name);
di.setMinVersion(version);
di.setMaxVersion(version.getNextPatchVersion());
meta.getDependencies().add(di);
DummyModule fakeMod = new DummyModule(meta);

moduleRegistry.add(fakeMod);
ResolutionResult result = dependencyResolver.resolve(fakeName);
moduleRegistry.remove(fakeMod);

I would find it useful if this was supported by DependencyResolver.

Apparently ModuleMetadata.getDependencies can return a list with a null element when module.txt contains an extra trailing comma, as in the example below.

Could catch the null here, but probably better to fix it so that list can't contain null elements at all, even if the JSON is invalid.

{
  "id": "BlockDetector",
  "version": "1.4.0-SNAPSHOT",
  "author": "rzats",
  "displayName": "BlockDetector",
  "description": "Adds a detector item that indicates the location of arbitrary blocks.",
  "dependencies": [
    {
      "id": "CoreAssets",
      "minVersion": "2.0.0"
    },
    {
      "id": "Inventory",
      "minVersion": "1.1.0"
    },
  ],
  "serverSideOnly": false,
  "isAsset": true,
  "isLibrary": true
}

As far as I understand it, the minimum version of Android that gestalt supports is API 24 (Android 7.0). This is because gestalt uses certain Java 8 APIs, such as Optional, which are not available under any earlier versions.

java.lang.NoSuchMethodError: No static method withInitial(Ljava/util/function/Supplier;)Ljava/lang/ThreadLocal; in class Ljava/lang/ThreadLocal; or its super classes (declaration of 'java.lang.ThreadLocal' appears in /system/framework/core-oj.jar)
    at org.terasology.gestalt.assets.management.ContextManager.<clinit>(ContextManager.java:36)
    at org.terasology.gestalt.assets.management.ContextManager.getCurrentContext(ContextManager.java:45)
    at org.terasology.gestalt.assets.management.AssetManager.resolve(AssetManager.java:151)

The only use of this method that I could find was in ContextManager.java:

The Java implementation of this (from the OpenJDK source) behaves equivalently:
https://github.com/openjdk/jdk/blob/f613d43f269edd6f213629f45f643d0ae620b8ee/src/java.base/share/classes/java/lang/ThreadLocal.java#L142-L144
https://github.com/openjdk/jdk/blob/f613d43f269edd6f213629f45f643d0ae620b8ee/src/java.base/share/classes/java/lang/ThreadLocal.java#L291-L307

Would it be possible to change the usage in ContextManager to the form used in PrefabJsonFormat?

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

With a working Android agent in Jenkins that's successfully building DestSol we should be able to apply the same approach to Gestalt to finally have its Android component built in CI and published to Artifactory. So far that's been done via local publish by a dev with a working Android Studio and a credential for Artifactory.

BenjaminAmos/DestinationSol#1 held the initial effort for Android in DS, since then merged into the mainline repo, the Android facade, and our infra repo

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Currently gestalt depends on a custom snapshot of what was org.reflections:0.9.12, re-named to org.terasology:reflections:0.9.12-MB. I do not know the exact reasons for the fork but MovingBlocks/reflections only contains a single unique commit with minor code changes. The README claims that the project is unsupported but things appear to have changed since then, with several upstream org.reflections releases being made, up to the most recent org.reflections:0.10.2. It might be worth upgrading to the upstream source to benefit from any bugfixes that might have occurred since then.

This is mostly an obsolete issue with gestalt 8 (develop branch), since it doesn't rely on reflections anymore. Terasology and Destination Sol could still be relying on gestalt 7 for some time to come though.

When a module has a dependency on another module with minVersion 0.x.y, and the maxVersion is not specified, it should default to 0.x+1.0. This reflects that the API is not considered stable until version 1.0.0 is reached, so each minor version is likely a breaking change.

• Begin Transaction
• Commit Transaction
• Component caching
• Optimistic Concurrency

We've decided to change version comparison to be more consistent with Semantic Versioning.

Currently a dependency declared as 2.0.0 may be satisfied by a module implementing 2.0.0-SNAPSHOT. This should not be true under semantic versioning.

Requires a change to the VersionRange class and tests, as well as documentation. (Check both Terasology and gestalt wikis.)

Currently, the ResolutionResult class returns false when the resolutions fails. I would find it helpful to get more information on why that happened.

This could include the modules (version ranges) in the DAG that were missing or the dependency links (as modeled by the Constraint class).

Investigate the possibility of triggering disposal of assets not referenced outside of the asset system.

The class PrefabDeltaFormat in terasology is annotated with this annotation and is currently using a static way to get data (CoreRegistry). There should be a way to get data to RegisterAssetDeltaFileFormat classes.

The tricky part about adding dependency injection will be the documentation with javadoc: If the annotation is defined in gestal core it can't obviously explain how the dependency injection is done in terasology.

Instance assets should be disposed when no longer referenced, as there is no way to reobtain them (as opposed to non-instance assets which can always be reobtained by urn).