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

[vcpkg format-manifest] Add convert-control flag #12471

Merged
merged 21 commits into from
Aug 2, 2020

Conversation

strega-nil
Copy link
Contributor

@strega-nil strega-nil commented Jul 17, 2020

vcpkg format-manifest learned how to convert CONTROL files to manifest files; additionally, it normalizes manifest files now, so that equivalent manifest files will have a single "canonical" representation.

This is a list of the actual changes made:

  • First, fix Json::parse -- "\c", for any c, was incorrectly parsed.
    It would emit the escaped character, and then parse the character, so
    that \b would give you { '\b', 'b' }.
  • Second, canonicalize source paragraphs as we're parsing them. This found
    an error in qt5 -- The declarative feature was listed twice, and we
    now catch it, so I removed the second paragraph.
  • Add PlatformExpression::complexity to allow ordering platform
    expressions in a somewhat reasonable way.

Notes:

  • We allow all_modules as a feature name for back-compat with
    paraview
  • qt5 has a default feature which is intended to be something like default-features, but it doesn't actually do anything, so it's deleted
  • This code formats all control files into manifest files without error.

Things still to do:

  • add support for $directives in x-format-manifest

TODO: manifest comments! we should keep $directives
First, fix Json::parse -- "\c", for any c, was incorrectly parsed.
It would emit the escaped character, and then parse the character, so
that `\b` would give you { '\b', 'b' }.

Second, canonicalize source paragraphs as we're parsing them. This found
an error in qt5 -- The `declarative` feature was listed twice, and we
now catch it, so I removed the second paragraph.

Add PlatformExpression::complexity to allow ordering platform
expressions in a somewhat reasonable way.

Notes:
  - We allow `all_modules` as a feature name for back-compat with
    paraview
  - In order to actually convert CONTROL to vcpkg.json, we'd need to
    rename the qt5 `default` feature.
  - We need to add support for $directives in x-format-manifest
@strega-nil strega-nil requested review from BillyONeal and ras0219-msft and removed request for BillyONeal July 20, 2020 19:08
@JackBoosY JackBoosY added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:internal This PR or Issue was filed by the vcpkg team. labels Jul 22, 2020
@strega-nil
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

toolsrc/include/vcpkg/base/json.h Show resolved Hide resolved
toolsrc/src/vcpkg/sourceparagraph.cpp Outdated Show resolved Hide resolved
toolsrc/include/vcpkg/sourceparagraph.h Show resolved Hide resolved
toolsrc/src/vcpkg/sourceparagraph.cpp Outdated Show resolved Hide resolved
@strega-nil strega-nil merged commit 1c2af99 into microsoft:master Aug 2, 2020
@strega-nil strega-nil deleted the x-format-manifest branch September 10, 2020 21:21
hellozee pushed a commit to hellozee/vcpkg that referenced this pull request Sep 11, 2020
* [vcpkg format-manifest] initial convert-control attempt

TODO: manifest comments! we should keep $directives

* Finalize x-format-manifest

First, fix Json::parse -- "\c", for any c, was incorrectly parsed.
It would emit the escaped character, and then parse the character, so
that `\b` would give you { '\b', 'b' }.

Second, canonicalize source paragraphs as we're parsing them. This found
an error in qt5 -- The `declarative` feature was listed twice, and we
now catch it, so I removed the second paragraph.

Add PlatformExpression::complexity to allow ordering platform
expressions in a somewhat reasonable way.

Notes:
  - We allow `all_modules` as a feature name for back-compat with
    paraview
  - In order to actually convert CONTROL to vcpkg.json, we'd need to
    rename the qt5 `default` feature.
  - We need to add support for $directives in x-format-manifest

* fix qt5 port

* format

* fix compile

* fix tests for canonicalization

* Clean up code

* add error message for nothing to format

* add extra_info field

* add `const X&` overloads for `Object::insert[_or_replace]`

* fix compile

* simple CRs

* add tests

* format

* Fix mosquitto port file

also unmerge a line

* fail the tests on malformed manifest

* fix format_all

* fix coroutine port-version

* format manifests
strega-nil added a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
* [vcpkg format-manifest] initial convert-control attempt

TODO: manifest comments! we should keep $directives

* Finalize x-format-manifest

First, fix Json::parse -- "\c", for any c, was incorrectly parsed.
It would emit the escaped character, and then parse the character, so
that `\b` would give you { '\b', 'b' }.

Second, canonicalize source paragraphs as we're parsing them. This found
an error in qt5 -- The `declarative` feature was listed twice, and we
now catch it, so I removed the second paragraph.

Add PlatformExpression::complexity to allow ordering platform
expressions in a somewhat reasonable way.

Notes:
  - We allow `all_modules` as a feature name for back-compat with
    paraview
  - In order to actually convert CONTROL to vcpkg.json, we'd need to
    rename the qt5 `default` feature.
  - We need to add support for $directives in x-format-manifest

* fix qt5 port

* format

* fix compile

* fix tests for canonicalization

* Clean up code

* add error message for nothing to format

* add extra_info field

* add `const X&` overloads for `Object::insert[_or_replace]`

* fix compile

* simple CRs

* add tests

* format

* Fix mosquitto port file

also unmerge a line

* fail the tests on malformed manifest

* fix format_all

* fix coroutine port-version

* format manifests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants