Update Dependency Schemas

wbond commented 3 years ago

In order to get Python 3.8 support working properly, we need dependency releases to indicate what python versions they support.

A new python_versions key should be added to the repository JSON schema.

The current state of the schema is: https://github.com/wbond/package_control/blob/four-point-oh/example-repository.json#L371-L427.

The code that consumes the JSON is at: https://github.com/wbond/package_control/blob/four-point-oh/package_control/providers/repository_provider.py.

The channel JSON schema also needs to be updated, since it caches the repository schema. The example is at: https://github.com/wbond/package_control/blob/four-point-oh/example-channel.json#L43-L67.

The code to consume it is at: https://github.com/wbond/package_control/blob/four-point-oh/package_control/providers/channel_provider.py.

Ultra-Instinct-05 commented 3 years ago

I can maybe try & help but unsure how to contribute & test since PC doesn't have a contributing guidelines.

wbond commented 3 years ago

Tests are actually run from within Sublime Text.

Step 1: Obtain a GitHub Client ID and Client Secret by creating an OAuth Application at https://github.com/settings/applications/new. You can enter whatever URLs for your app homepage and callback. You won't actually being using the Oauth auth flow.

Step 2: Add your client id and secret to https://github.com/wbond/package_control/blob/four-point-oh/package_control/tests/__init__.py#L12-L13.

Step 3: Set "enable_test": true, in https://github.com/wbond/package_control/blob/four-point-oh/Package%20Control.sublime-settings#L230.

Step 4: Execute the command Package Control: Tests from the command palette.

Tests are all stored in https://github.com/wbond/package_control/tree/four-point-oh/package_control/tests/.

I suppose your first PR could be writing up directions for running the tests! :-)

wbond commented 3 years ago

My thoughts on python_versions is that it should be a list of strings, allowing the same values that are specified in a .python-version file in a package.

For example:

// Python 3.3
"python_versions": ["3.3"]
// Python 3.8
"python_versions": ["3.8"]
// Either
"python_versions": ["3.3", "3.8"]

This key would be added to a dependency release:

{
    "name": "bz2",
    "load_order": "02",
    "description": "Python bz2 module",
    "author": "wbond",
    "issues": "https://github.com/codexns/sublime-bz2/issues",
    "releases": [
        {
            "base": "https://github.com/codexns/sublime-bz2",
            "tags": true,
            "sublime_text": "*",
            "python_versions": ["3.3"],
        }
    ]
}

rchl commented 3 years ago

How do you envision those versions (both for dependencies and packages through .python-version) will be handled when, let's say, the 3.8 runtime is replaced with 3.9 at some point?

wbond commented 3 years ago

I don’t foresee us migrating versions. If anything in the future maybe we’ll add say 3.11. However, as plugins migrate to newer runtimes we may make runtime initialization lazy, and possibly even runtime installation will be lazy.

FichteFoll commented 3 years ago

Should python_versions be required? If it isn't specified, should it default to all currently supported versions or only 3.3?
Should it support the special "*" string to satisfy potential future supported versions as well or do we say we don't need to bother with this until it happens?
(How) do we envision the repository setup to change? The current folder structure of dependencies shipping all distributions for platform and ST version targets in the same package is quite suboptimal and only gets worse if we add another factor, the python version, to the matrix.

For the latter, I could see .whl artifacts on Github releases that follow the standard naming conventions for Python wheel to be promising.

wbond commented 3 years ago

We’ll need to support the old schema and translate that to 3.3 only. I think for the new schema python_versions should be required.

I don’t think we should support *. We need dependency authors to explicitly state support - it should be tested first.

We’ll probably support the old repository structure with some sort of flag, along with bare repositories and likely .whl files. I’m open to other ideas.

Releases support in genera would be fine, but I’m not going to say that has to happen for the initial release. Especially since packages will want to use those too.

deathaxe commented 3 years ago

Anyone already up for it?

Ultra-Instinct-05 commented 3 years ago

@deathaxe Feel free to take it (In case my statement above is making you not work on this) :-)

deathaxe commented 3 years ago

Tests are actually run from within Sublime Text.

Step 1: Obtain a GitHub Client ID and Client Secret by creating an OAuth Application at https://github.com/settings/applications/new. You can enter whatever URLs for your app homepage and callback. You won't actually being using the Oauth auth flow.

Step 2: Add your client id and secret to https://github.com/wbond/package_control/blob/four-point-oh/package_control/tests/__init__.py#L12-L13.

It appears I don't have luck with that.

When running tests, downloads from github fails with 403 on Windows 10. Downoading with Firefox works well though. Any ideas?

ERROR: test_github_client_branch_downloads (Package Control.package_control.tests.clients.GitHubClientTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Apps\Sublime Text 40xx\Data\Packages\Package Control\package_control\tests\clients.py", line 86, in test_github_client_branch_downloads
    client.download_info('https://github.com/packagecontrol-test/package_control-tester')
  File "C:\Apps\Sublime Text 40xx\Data\Packages\Package Control\package_control\clients\github_client.py", line 112, in download_info
    repo_info = self.fetch_json(self._make_api_url(user_repo))
  File "C:\Apps\Sublime Text 40xx\Data\Packages\Package Control\package_control\clients\json_api_client.py", line 51, in fetch_json
    repository_json = self.fetch(url, prefer_cached)
  File "C:\Apps\Sublime Text 40xx\Data\Packages\Package Control\package_control\clients\json_api_client.py", line 35, in fetch
    content = manager.fetch(url, 'Error downloading repository.', prefer_cached)
  File "C:\Apps\Sublime Text 40xx\Data\Packages\Package Control\package_control\download_manager.py", line 342, in fetch
    return self.downloader.download(url, error_message, timeout, 3, prefer_cached)
  File "C:\Apps\Sublime Text 40xx\Data\Packages\Package Control\package_control\downloaders\wininet_downloader.py", line 678, in download
    raise DownloaderException(error_string)
Package Control.package_control.downloaders.downloader_exception.DownloaderException: Error downloading repository. HTTP error 403 downloading https://api.github.com/repos/packagecontrol-test/package_control-tester?client_secret=...&client_id=....

deathaxe commented 3 years ago

Nevermind, I found the issue. Fix following.

shardulbee commented 3 years ago

@deathaxe have you settled on updating the schema as @wbond suggested above, with a python_versions array of strings? If so, I can get started on https://github.com/wbond/package_control/issues/1517 with that assumption, to work a bit in parallel

deathaxe commented 3 years ago

Yes I have started work on it.

Set "schema_version": "4.0.0"
Older schemes are set to python_versions: ["3.3"] automatically, if the key is missing. "2.6" is not supported as ST 3143 is the oldest built being supported by PC4.0

Scheme 4.0 requires an explicit python_versions. The current implementation accepts

"python_versions: "3.3"
"python_versions: ["3.3"]
"python_versions: ["3.8"]
"python_versions: ["3.3", "3.8"]

deathaxe commented 3 years ago

Do we want to accept python_versions keys in repositories of scheme version < 4.0 at all?

If we do so, everyone can continue using 3.0.0 repositories and just add "python_versions: ["3.3", "3.8"] keys. As those are ignored by PC 3 and accepted by PC 4 addin support for that key alone is no reason to change scheme version then, is it?

wbond commented 3 years ago

I think python_versions in schema_version < 4.0.0 should be considered an error since other code that may exist to parse the JSON files won't know that python_versions exists, or that it should affect what release is selected.

I think if someone wants to opt-in to support Python 3.8, I think updating the schema_version is fine. In practice, this will likely only affect the default channel.

Additionally, while finding an unexpected key should be an error, I'm not sure if the current implementation enforces that.

I did just realize since we've settled on keeping the sublime_text selector, and only adding python_versions, the schema doesn't need to go up a major version. It could just be 3.1.0.

deathaxe commented 3 years ago

Additionally, while finding an unexpected key should be an error, I'm not sure if the current implementation enforces that.

It's not enforced.

It could just be 3.1.0.

That's something I also considdered to be enough.

Also find it quite hard to setup test cases for those huge monolitic functions, especially as those require upstream repos with proper test files. Also the need to run and wait for all of them while working on one special piece is not very convenient - especially when searching typos in new test cases hehehe..

wbond commented 3 years ago

Yes, most of the current tests are more integration tests for the hosting-specific providers.

rchl commented 3 years ago

I think python_versions in schema_version < 4.0.0 should be considered an error since other code that may exist to parse the JSON files won't know that python_versions exists, or that it should affect what release is selected.

If it wasn't explicitly defined before whether extra properties are accepted or not then probably you can decide it now and either state that explicitly or even create a proper schema to make it explicit.

In JSON schema the additionalProperties property defines if extra properties are allowed or not so schema can go either way (with the default being that extra properties are allowed).

That said, I feel that the review bot used on the package control channel probably complains about extra properties now but haven't checked to make sure.

deathaxe commented 3 years ago

Already thought about using jsonschema to check repositories of different versions with common methods as well, but I've not started investigating how it would apply nor am I sure wheter additional dependencies are wanted. Basically the task is just to add support for python_versions. I already found GitlabClient not being used to downoad repo/download infos and prepared a fix for that. Also found those current implementations hard to add unittests for, ... etc. But I don't want to reinvent the wheel at this point.

FichteFoll commented 3 years ago

I also considered using a JSONschema validator for CI on the channel repo, as it would a) simplify the tests quite a bit, and b) allow users using a linter for that, e.g. LSP-json, to validate them on the fly. On the other hand, it would also prevent users that don't have that to run the verification tests locally if we purged that from the Python test file, which even works inside ST.

Shoudn't hurt to have one regardless, if only for a standardized and single source of truth.

deathaxe commented 3 years ago

The issue just seems to be jsonschema library requiring python 3.5 or higher. It doesn't run on 3.3 and I don't think I want to try to backport it.

deathaxe commented 3 years ago

Are there any plans to reject branch based releases as of any scheme version?

wbond / package_control

Update Dependency Schemas #1516