Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omnipresent VersionedOverrideTransformers #1684

Merged
merged 3 commits into from
Apr 28, 2016
Merged

Conversation

dbent
Copy link
Member

@dbent dbent commented Apr 26, 2016

@pjf @Starstrider42 @Dazpoet

Prompted by #1674. The issue was that the x_netkan_override was overriding the install stanza. However, the AvcTransformer uses the install stanza to determine which files in an archive will be installed so that it can pick the correct .version file to use. The problem was that the AvcTransformer executes before the VersionedOverrideTransformer so the AvcTransformer didn't think any files would be installed because the appropriate override hadn't occurred yet.

What this PR does is basically allow you specify when an override should execute, through the use of before and after properties in the override.

To accomplish this there are three main changes:

  • Each ITransformer must have a name which identifies it uniquely.
  • VersionedOverrideTransformer takes a list of names of ITransformer implementations that it executes directly before and executes directly after.
  • The top-level NetkanTransformer has been modified to inject an instance of VersionedOverrideTransformer before and after every other ITransformer.

So now the transformation pipeline looks like:

VersionedOverrideTransformer: before: first
FirstTransformer
VersionedOverrideTransformer: before: second, after: first
SecondTransformer
VersionedOverrideTransformer: before third, after: second
ThirdTransformer
VersionedOverrideTransformer: after: third

There are also two "psuedo-transformers", $none and $all (should be self-explanatory) so the pipeline really looks like:

VersionedOverrideTransformer: before: first, $all, after: $none
FirstTransformer
VersionedOverrideTransformer: before: second, after: first
SecondTransformer
VersionedOverrideTransformer: before third, after: second
ThirdTransformer
VersionedOverrideTransformer: after: third, $all, before: $none

How does this fix #1674?

Well given this .netkan (@Starstrider42 changed the archive to workaround the issue in NetKAN#3691 so to make it reproducable I modified from the one given in #1674 by inverting the install stanzas and override condition).

{
    "spec_version": 1,
    "identifier": "CustomAsteroids-Pops-Stock-Inner",
    "$kref": "#/ckan/spacedock/210",
    "$vref" : "#/ckan/ksp-avc",
    "install": [
        {
            "file": "GameData/CustomAsteroids",
            "install_to": "GameData",
            "filter_regexp": "(?<!Basic Asteroids\\.cfg)$"
        }
    ],
    "x_netkan_override" : [
        {
            "version": ">=v1.3.0",
            "override" : {
                "install": [
                    {
                        "file": "Standard Setup",
                        "install_to": "GameData",
                        "filter_regexp": "(?<!Basic Asteroids\\.cfg)$"
                    }
                ]
            }
        }
    ],
}

NetKAN compalains:

1076 [1] FATAL CKAN.NetKAN.Program (null) - No files found in GameData/CustomAsteroids to install!

However using this .netkan (notice the addition of "before": "avc")

{
    "spec_version": 1,
    "identifier": "CustomAsteroids-Pops-Stock-Inner",
    "$kref": "#/ckan/spacedock/210",
    "$vref" : "#/ckan/ksp-avc",
    "install": [
        {
            "file": "GameData/CustomAsteroids",
            "install_to": "GameData",
            "filter_regexp": "(?<!Basic Asteroids\\.cfg)$"
        }
    ],
    "x_netkan_override" : [
        {
            "version": ">=v1.3.0",
            "before": "avc",
            "override" : {
                "install": [
                    {
                        "file": "Standard Setup",
                        "install_to": "GameData",
                        "filter_regexp": "(?<!Basic Asteroids\\.cfg)$"
                    }
                ]
            }
        }
    ],
}

NetKAN successfully produces the following .ckan:

{
    "spec_version": 1,
    "identifier": "CustomAsteroids-Pops-Stock-Inner",
    "name": "Custom Asteroids",
    "abstract": "Lets users control where asteroids appear",
    "author": "Starstrider42",
    "license": "MIT",
    "resources": {
        "homepage": "http://forum.kerbalspaceprogram.com/index.php?/topic/72785-/",
        "spacedock": "https://spacedock.info/mod/210/Custom%20Asteroids",
        "repository": "https://github.com/Starstrider42/Custom-Asteroids",
        "x_screenshot": "https://spacedock.info/content/Starstrider42_1134/Custom_Asteroids/Custom_Asteroids-1461528691.6712377.png"
    },
    "version": "v1.3.0",
    "ksp_version": "1.1.0",
    "install": [
        {
            "file": "Standard Setup",
            "install_to": "GameData",
            "filter_regexp": "(?<!Basic Asteroids\\.cfg)$"
        }
    ],
    "download": "https://spacedock.info/mod/210/Custom%20Asteroids/download/v1.3.0",
    "download_size": 77254,
    "x_generated_by": "netkan"
}

To remain backwards compatible with existing overrides, there is a hardcoded VersionedOverrideTransformer that occurs before and after null.

The following is the list of all possible values of before and after. It exposes some of the internal architecture of NetKAN, but I think this is the best possible compromise to solve this particular issue.

$none
$all
avc
download_size
epoch
forced_v
generated_by
github
http
internal_ckan
jenkins
metanetkan
optimus_prime
property_sort
spacedock
strip_netkan_metadata
version_edit
versioned_override

Puts VersionedOverrideTransformers between every transformer and adds the
ability to specify which VersionedOverrideTransformer should execute an
override by the use of "before" or "after" properties.
@Starstrider42
Copy link
Contributor

From your opening explanation I take it you guys need to support multiple .version files in the same archive... 😲

I'm afraid the whole *Transformers/pipeline business is a bit over my head... so in the example you gave, the >=v1.3.0 override is applied before KSP-AVC is invoked. Is that chosen based on the version number from SpaceDock? Should I (as a user, not a CKAN insider) think of "before": "avc" as "pretend, for this override only, that there was no KSP-AVC $vref"?

If you're concerned about exposing implementation details, perhaps you can trim the list of allowed values a bit. I certainly can't guess what all of them mean off the top of my head... (but I'll bite: is there actually an optimus_prime in NetKAN)?

@Dazpoet
Copy link
Member

Dazpoet commented Apr 27, 2016

Yes, there is an easter egg which triggers an Optimus Prime quote ;)

@dbent
Copy link
Member Author

dbent commented Apr 27, 2016

From your opening explanation I take it you guys need to support multiple .version files in the same archive... 😲

It's necessary because a lot of mods distribute their dependencies in their own archive (bleh), meaning we need some mechanism to decide which of multiple possible .version files actually correspond to the mod we're installing. The default mechanism is to only look at files that will actually be installed, hence why the value of the install stanzas are important to the AvcTransformer. (It supports selecting a .version file via a regex expression if necessary, but that's rarely used).

I'm afraid the whole *Transformers/pipeline business is a bit over my head... so in the example you gave, the >=v1.3.0 override is applied before KSP-AVC is invoked. Is that chosen based on the version number from SpaceDock? Should I (as a user, not a CKAN insider) think of "before": "avc" as "pretend, for this override only, that there was no KSP-AVC $vref"?

NetKAN works by applying a series of transformations on its input metadata. This series of transformations is the "pipeline". The metadata at the start of the pipeline should be a valid .netkan file and the metadata at the end of the pipeline should be a valid .ckan file.

One of these transformations is applied by the AvcTransformer which will read a .version file and set the version property (if not already set) and update the ksp_version properties. At the point in the pipeline when the AvcTransformer is invoked the only version information that exists is what we retrieved from SpaceDock (by way of the SpacedockTransformer).

So what before and after do is tell the VersionedOverrideTransformer (which processes the override stanzas) when to apply the override. In this case it tells it to apply the override before the AvcTransformer, so that when the AvcTransformer is invokved, the install stanzas have been overridden and it knows the correct set of files to look for the .version file in.

If you're concerned about exposing implementation details, perhaps you can trim the list of allowed values a bit. I certainly can't guess what all of them mean off the top of my head...

I'm not too concerned, like I said in the documentation update in this PR, the vast vast majority of overrides should never have to use a before or after property, just for special cases like this.

(but I'll bite: is there actually an optimus_prime in NetKAN)?

Isn't there a little bit of Optimus Prime everywhere?

@techman83
Copy link
Member

I've had a good read over this and it looks really good. I've also tested across all our NetKANs and tested with the example NetKAN. The indexer will pickup the new version on the next run.

Autobots roll out!

@techman83 techman83 merged commit 4a7e149 into master Apr 28, 2016
@Dazpoet Dazpoet deleted the feature/omni_overrides branch April 28, 2016 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NetKAN doesn't override install field if $vref defined
5 participants