Handle Python Dependency Versions Correctly #1078
Conversation
Change logic of python dependency version ranges. Parse semver number either as specific version or range and output to setup.py. Fix aws#676
|
The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged. |
|
Some questions for reviewers:
If we want to keep limiting the version numbers in jsii to not be ranges we can remove one of the branches here. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| @@ -32,7 +32,7 @@ | |||
| "install_requires": [ | |||
| "jsii~=0.20.8", | |||
| "publication>=0.0.3", | |||
| "scope.jsii-calc-base-of-base~=0.20.8" | |||
| "scope.jsii-calc-base-of-base==0.20.8" | |||
There was a problem hiding this comment.
The original dependency is a caret dependency (^), does == means the same thing in Python?
jsii/packages/jsii-calc-base/package.json
Line 32 in 8fc81ca
There was a problem hiding this comment.
Yeah that looks suspicious; but I reckon because the jsii compiler forwards the "resolved" (not "declared") version. That needs to be fixed as well (possibly in a separate PR, but then we probably shouldn't merge this without that).
| if (parsedVersion) { | ||
| versionSpecifier = `==${version}`; | ||
| } else if (parsedVersionRange) { | ||
| versionSpecifier = parsedVersionRange.replace(' ', ', '); | ||
| } else { | ||
| versionSpecifier = `~=${versionParts.slice(0, 2).join('.')},>=${versionParts.slice(0, 3).join('.')}`; | ||
| throw new Error(`Unexpected version format '${version}' for dependency: 'depName'`); | ||
| } |
There was a problem hiding this comment.
This feels like it should have tests. Also - similar work should be done on Java and .NET dependencies.
| @@ -32,7 +32,7 @@ | |||
| "install_requires": [ | |||
| "jsii~=0.20.8", | |||
| "publication>=0.0.3", | |||
| "scope.jsii-calc-base-of-base~=0.20.8" | |||
| "scope.jsii-calc-base-of-base==0.20.8" | |||
There was a problem hiding this comment.
Yeah that looks suspicious; but I reckon because the jsii compiler forwards the "resolved" (not "declared") version. That needs to be fixed as well (possibly in a separate PR, but then we probably shouldn't merge this without that).
|
So seems like solution here is:
Will reopen when this is ready for review |
Change logic of python dependency version ranges. Parse semver number either as specific version or range and output to setup.py.
Fix #676
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.