Support indexing mulitple release assets on GitHub

[DasSkelett]

Motivation

Kopernicus is a complex mod usually breaking between KSP releases. It used to only support one "primary" KSP version and some older ones in a separate backports repository.

Now that @R-T-B has taken over development, he introduced a new build system which allows him to easily support multiple KSP versions at the same time in the same repo.
Preferably he would want to release all assets/zips for every supported KSP version in a single release tag, like he does for his bleeding edge repo, where the metadata is currently maintained by hand.

However, NetKAN currently doesn't support this. With /ckan/github $krefs, it always takes the module version from the release tag. Even multiple .netkan files with different asset_match wouldn't work, since they'd all end up with the same version.

Concept

Now there is a new optional version_from_asset extension to the GitHub $kref.
It is in the format /version_from_asset/:version_regexp, where the version_regexp is matched against all release assets. All matching release assets will be inflated individually and result in a new metadata module.
The version_regexp must provide a named capturing group called version, which will be used to retrieve the module version for each asset.

asset_match and version_from_asset are mutually exclusive. version_from_asset also provides the filtering aspect of asset_match if needed, because it just skips unmatched assets.

To allow filling in the release tag into the final version string, x_netkan_version_edit now supports a special ${tag} placeholder, which will be replaced by the release tag.

Changes

There is a new GithubReleaseAsset class which holds a few asset-specific properties. GithubRelease has a new field, IEnumerable<GithubReleaseAsset> Assets, and lost asset-specific ones.
GithubApi creates the GithubRelease with the list of all GithubReleaseAsset now.

GithubRef's regex is extended to allow the version_from_asset directive.

GithubTransformer.Transform() decides based on whether a version_from_asset regex is given or not how to inflate the metadata. If we have a regex, we try to get the version out of it, and pass it to TransformOne() together with the asset.

GithubTransformer.TransformOne() also takes a GithubReleaseAsset and String version as argument, from which it takes parts of the needed metadata.
Furthermore it adds a entry for the release tag to the metadata under x_netkan_version_pieces.

VersionEditTransformer reads the dict from x_netkan_version_pieces to replace any special vars.

The spec is also updated. Suggestions welcome if you can put it in better words.

I'm also not really found fond of calling it version_from_asset. It misses the fact that it can result in multiple .ckans.
multi_asset_match misses the fact that it gets the version from the file name.
multi_asset_match_with_version_from_asset is a bit long...

Testing

Take Kopernicus.netkan, switch the kref to

"$kref"         : "#/ckan/github/R-T-B/Kopernicus/version_from_asset/^KopernicusBE_(?<version>.+)\\.zip",

optionally add

"x_netkan_version_edit": {
        "find": "(?<ksp_version>.+)_Release(?<build>[0-9]+)",
        "replace": "release-${ksp_version}-${build}",
        "strict": true
    },
```json
or a different `replace` to test the tag replacement

    "replace": "${tag}-${ksp_version}",

to make the version format match the old ones.

Inflating `Kopernicus.netkan` should result in 4 .ckan files.

[HebaruSan]

This looks great! My only real concern is that other mod authors will be upset we didn't do it earlier. 😬

Thinking about how it might be used and abused, I can imagine a mod author wanting to combine the tag and a substring from the asset. I.e.:

Tag: 1.2.3 (purely mod-version string unrelated to game version)
Assets:
  - Name: ForKSP-1.7.3.zip (just game version)
  - Name: ForKSP-1.10.x.zip (just game version)

Intended resulting versions:

- 1.2.3-1.7.3
- 1.2.3-1.10.x

Is that a possibility here? If not, could we achieve it?

[DasSkelett]

Thinking about how it might be used and abused, I can imagine a mod author wanting to combine the tag and a substring from the asset.

Yeah, also thought of that and that it could be useful.
Currently it's not possible, we only take what's inside the version capture group and that's it.

We'd need to declare "special" variables (like tag) that we would fill in into the version string.
E.g. if the version string contains ${tag} we would replace it with the tag.
Let me have a think how exactly we could achieve this, maybe with some regex magic.

[HebaruSan]

  --releases           (Default: 1) Number of releases to inflate, or 'all'

  --skip-releases      (Default: 0) Number of releases to skip / index of 
                       release to inflate.

Should the numbers passed to these options relate to the number of tags we process, or the number of modules we ultimately inflate?

[DasSkelett]

  --releases           (Default: 1) Number of releases to inflate, or 'all'

  --skip-releases      (Default: 0) Number of releases to skip / index of 
                       release to inflate.

Should the numbers passed to these options relate to the number of tags we process, or the number of modules we ultimately inflate?

Currently they dictate the number of processed respectively skipped tags. Given the original use case for those flags, that is backfilling missing/skipped releases, I'd say they should keep that behaviour.
If we want an option to limit the total inflated modules, we should introduce a new option for that. Not sure where we'd need that currently though, since it is undefined which asset(s) you'd get.

We could clarify the description of those options though, maybe by replacing "releases" with "release tags"?

[HebaruSan]

Given the original use case for those flags, that is backfilling missing/skipped releases, I'd say they should keep that behaviour.

OK, I'm convinced. The descriptions are probably fine as they are.

[DasSkelett]

So regarding using the tag, there's the problem that the current .netkan-provided regex is only used as a "find" regex, i.e. one to get information with. To bring in the tag, we need another place where we can specify how to format the new version and where to fill in the tag (and possibly other data in the future) inside the string, like a "replace" regex.

Yup, this sounds a lot like our x_netkan_version_edit.

Some approaches that came to my mind:

1. Use the x_netkan_version_edit replace string by allowing to add a ${tag} (or ${x_tag} to avoid conflicts with user-provided capture groups), which we replace then.
   However there's a caveat: VersionEditTransformer has no idea of any GitHub tags or anything else, given that it happens way later in the inflation process and is source independent. It's basically "stateless" in relation to other transformers.
   We would need to introduce a way to submit a dict of "additional replacements" the VersionEditTransformer should execute, tied to each module. Temporarily encode it in the metadata variable?

2. Still use x_netkan_version_edit replace string, but read and edit it in-place in the GithubTransformer (maybe via a static helper function in ModuleService to make it available to other transformers as well). When VersionEditTransformer is executed, the ${tag} / ${x_tag} is already replaced with the actual tag.

3. Change /version_from_asset/:version_regexp to /version_from_asset/:version_regexp[/:replace] which allows a replacement format/string to be specified directly in the kref, basically duplicating the a bunch of logic from VersionEditTransformer. Least favourite solution.

4. Any additional ideas you come up with.

[HebaruSan]

1. VersionEditTransformer has no idea of any GitHub tags or anything else, given that it happens way later in the inflation process and is source independent. It's basically "stateless" in relation to other transformers.
   We would need to introduce a way to submit a dict of "additional replacements" the VersionEditTransformer should execute, tied to each module. Temporarily encode it in the metadata variable?

Sounds like a possible job for TransformOptions (defined in ITransformer.cs, available to GithubTransformer.Transform in its opts parameter)? Maybe it could have a Dictionary<string, string> VersionPieces which transformers could populate with keys and values, and then VersionEditTransformer could pull those in?

Though I'm not sure whether that will be duplicated into separate objects when we return multiple modules, might have to put some further effort into that (the loop is in NetkanTransformer.Transform)...

... yeah, TransformOptions currently will be one shared object per initial netkan. I'll look for a good way to differentiate it...

[HebaruSan]

OK, it's possible to use TransformOptions, but it doesn't turn out very well. The signature of ITransformer.Transform needs to change to return Tuple<Metadata, TransformOptions> or similar, which becomes a mess for typical code and downright complicated for your case. Instead, how about:

1. Temporarily encode it in the metadata variable?

I think that's excusable because Metadata is intended to hold the state of the inflation as it proceeds, and we already have a pattern of creating new instances when anything changes.

[DasSkelett]

That works like a charm!

1931 [1] WARN CKAN.NetKAN.Transformers.VersionEditTransformer (null) - Replacing ${tag} with UBE-release-71 in ${tag}-1.10.1
1933 [1] WARN CKAN.NetKAN.Transformers.VersionEditTransformer (null) - Replacing ${tag} with UBE-release-71 in ${tag}-1.11.1
1933 [1] WARN CKAN.NetKAN.Transformers.VersionEditTransformer (null) - Replacing ${tag} with UBE-release-71 in ${tag}-1.8.1
1933 [1] WARN CKAN.NetKAN.Transformers.VersionEditTransformer (null) - Replacing ${tag} with UBE-release-71 in ${tag}-1.9.1
1933 [1] WARN CKAN.NetKAN.Transformers.VersionEditTransformer (null) - Replacing ${tag} with UBE-release-70 in ${tag}-1.10.1
1933 [1] WARN CKAN.NetKAN.Transformers.VersionEditTransformer (null) - Replacing ${tag} with UBE-release-70 in ${tag}-1.11.1
1933 [1] WARN CKAN.NetKAN.Transformers.VersionEditTransformer (null) - Replacing ${tag} with UBE-release-70 in ${tag}-1.8.1
1933 [1] WARN CKAN.NetKAN.Transformers.VersionEditTransformer (null) - Replacing ${tag} with UBE-release-70 in ${tag}-1.9.1

$ ll -n Kopernicus*.ckan
-rw-rw-r-- 1 1000 1000 1,8K Jan 30 05:13 Kopernicus-2-UBE-release-70-1.10.1.ckan
-rw-rw-r-- 1 1000 1000 1,8K Jan 30 05:13 Kopernicus-2-UBE-release-70-1.11.1.ckan
-rw-rw-r-- 1 1000 1000 1,8K Jan 30 05:13 Kopernicus-2-UBE-release-70-1.8.1.ckan
-rw-rw-r-- 1 1000 1000 1,8K Jan 30 05:13 Kopernicus-2-UBE-release-70-1.9.1.ckan
-rw-rw-r-- 1 1000 1000 1,8K Jan 30 05:13 Kopernicus-2-UBE-release-71-1.10.1.ckan
-rw-rw-r-- 1 1000 1000 1,8K Jan 30 05:13 Kopernicus-2-UBE-release-71-1.11.1.ckan
-rw-rw-r-- 1 1000 1000 1,8K Jan 30 05:13 Kopernicus-2-UBE-release-71-1.8.1.ckan
-rw-rw-r-- 1 1000 1000 1,8K Jan 30 05:13 Kopernicus-2-UBE-release-71-1.9.1.ckan

with

    "$kref"         : "#/ckan/github/R-T-B/Kopernicus/version_from_asset/^KopernicusBE_(?<version>.+)\\.zip",
    "x_netkan_version_edit": {
        "find": "(?<ksp_version>.+)_Release(?<build>[0-9]+)",
        "replace": "${tag}-${ksp_version}",
        "strict": true
    },

Hang on, I should probably edit the spec to mention that feature , too...

[HebaruSan]

I'm pretty happy. Now we can think about how to announce this to mod authors that might benefit. 😬