Skip to content

Fix a problem in the logic to generate manifest source from existing parsed manifest structs #3415

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

Conversation

abertelrud
Copy link
Contributor

The logic to generate manifest source from existing parsed manifest data structures misquoted some package version requirement ranges.

Changes

This fixes those cases, producing a semantically equivalent package manifests. Since the model does not capture how a particular range was specified, it is not possible to recreate the original shorthands.

A future improvement can either extend the model to record the original spelling of the range requirements, or can apply heuristics to shorten the rules in the future (e.g. automatically derive upToNextMajor() etc.

This also eliminates an extra whitespace before the minimum tools version declaration when the specified tools version allows it, so older parsers can parse the generated manifests.

rdar://76559428

@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud self-assigned this Apr 19, 2021
@abertelrud abertelrud added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Apr 19, 2021
@abertelrud abertelrud force-pushed the eng/76559428-generated-manifest-version-range-problems branch from 6bf809e to 919b2d8 Compare April 19, 2021 22:16
…ata structures misquoted some package version requirement ranges.

This fixes those cases, producing a semantically equivalent package manifests.  Since the model does not capture how a particular range was specified, it is not possible to recreate the original shorthands.

A future improvement can either extend the model to record the original spelling of the range requirements, or can apply heuristics to shorten the rules in the future (e.g. automatically derive `upToNextMajor()` etc.

This also eliminates an extra whitespace before the minimum tools version declaration when the specified tools version allows it, so older parsers can parse the generated manifests.

rdar://76559428
@abertelrud abertelrud force-pushed the eng/76559428-generated-manifest-version-range-problems branch from 919b2d8 to 1cc2265 Compare April 19, 2021 22:22
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test macos

1 similar comment
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test macos

@abertelrud abertelrud merged commit 8ee8d40 into swiftlang:main Apr 20, 2021
@abertelrud abertelrud deleted the eng/76559428-generated-manifest-version-range-problems branch April 20, 2021 16:24
abertelrud added a commit to abertelrud/swift-package-manager that referenced this pull request Apr 21, 2021
…ata structures misquoted some package version requirement ranges. (swiftlang#3415)

This fixes those cases, producing a semantically equivalent package manifests.  Since the model does not capture how a particular range was specified, it is not possible to recreate the original shorthands.

A future improvement can either extend the model to record the original spelling of the range requirements, or can apply heuristics to shorten the rules in the future (e.g. automatically derive `upToNextMajor()` etc.

This also eliminates an extra whitespace before the minimum tools version declaration when the specified tools version allows it, so older parsers can parse the generated manifests.

rdar://76559428
(cherry picked from commit 8ee8d40)
abertelrud added a commit that referenced this pull request Apr 21, 2021
…ata structures misquoted some package version requirement ranges. (#3415) (#3422)

This fixes those cases, producing a semantically equivalent package manifests.  Since the model does not capture how a particular range was specified, it is not possible to recreate the original shorthands.

A future improvement can either extend the model to record the original spelling of the range requirements, or can apply heuristics to shorten the rules in the future (e.g. automatically derive `upToNextMajor()` etc.

This also eliminates an extra whitespace before the minimum tools version declaration when the specified tools version allows it, so older parsers can parse the generated manifests.

rdar://76559428
(cherry picked from commit 8ee8d40)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Author believes the PR is ready to be merged & any feedback has been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants