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] Implement versions db generator #13777

Merged
merged 34 commits into from
Oct 26, 2020

Conversation

vicroms
Copy link
Member

@vicroms vicroms commented Sep 28, 2020

Reduced scope of this PR to only generating version files from our Git history

  • Modify x-history to allow JSON output and add more version related information
  • Make x-history actually parse CONTROL and vcpkg.json files to fix several edge cases
  • Create script that generates version database files for each port
  • Detect version scheme when loading CONTROL files
  • Separate version string from port version in old CONTROL files

@vicroms
Copy link
Member Author

vicroms commented Sep 28, 2020

I'm not sure whether to check in the db files, there are too many of them.
Right now the DB files are one per port (located inside scripts/port_versions_db and use JSON format. Generating them took about 1 hour on my computer.

Also, it seems like I forgot to clean up some unrelated code.

@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 Sep 28, 2020
@vicroms vicroms changed the title [vcpkg] Add script to generate version database files [vcpkg] Add script to generate version database files and fetch sources Sep 28, 2020
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this PR!

I haven't finished reading more deeply into the algorithm, but here are some more trivial comments.

toolsrc/src/vcpkg/commands.porthistory.cpp Outdated Show resolved Hide resolved
toolsrc/include/vcpkg/versions.h Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/commands.porthistory.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/commands.porthistory.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/versions.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/versions.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/versions.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/versions.cpp Outdated Show resolved Hide resolved
ras0219 pushed a commit to ras0219/vcpkg that referenced this pull request Oct 15, 2020
* Introduce FeatureFlagSettings struct to more easily access feature flags throughout the program
* To avoid users accidentally starting to write "version" instead of "version-string" in their manifests, vcpkg explicitly detects and prevents usage of ports with schemes other than "String"
* Drive-by fix of copiable SourceControlFileLocation and an exposed use-after-move bug

This code is largely extracted from PR microsoft#13777

Co-authored-by: Victor Romero <romerosanchezv@gmail.com>
@vicroms vicroms changed the title [vcpkg] Add script to generate version database files and fetch sources [vcpkg] Implement versions db generator Oct 20, 2020
Copy link
Contributor

@ras0219 ras0219 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've lightly reviewed the changes to port history since that's an x- command.

toolsrc/src/vcpkg/paragraphs.cpp Show resolved Hide resolved
toolsrc/src/vcpkg/paragraphs.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/sourceparagraph.cpp Show resolved Hide resolved
toolsrc/src/vcpkg/sourceparagraph.cpp Show resolved Hide resolved
toolsrc/src/vcpkg/vcpkgcmdarguments.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/vcpkgcmdarguments.cpp Outdated Show resolved Hide resolved
toolsrc/src/vcpkg/vcpkgcmdarguments.cpp Outdated Show resolved Hide resolved
ras0219-msft added a commit that referenced this pull request Oct 21, 2020
#14079)

* [vcpkg] Add `versions` feature flag and version field manifest parsing

* Introduce FeatureFlagSettings struct to more easily access feature flags throughout the program
* To avoid users accidentally starting to write "version" instead of "version-string" in their manifests, vcpkg explicitly detects and prevents usage of ports with schemes other than "String"
* Drive-by fix of copiable SourceControlFileLocation and an exposed use-after-move bug

This code is largely extracted from PR #13777

Co-authored-by: Victor Romero <romerosanchezv@gmail.com>

* [vcpkg] Address CR Comments. Fix test crash on Linux.

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Victor Romero <romerosanchezv@gmail.com>
Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked at the rest of the code, but here are the things I see that should be corrected with the python.

scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
scripts/generatePortVersionsDb.py Show resolved Hide resolved
scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
scripts/generatePortVersionsDb.py Outdated Show resolved Hide resolved
Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes victor!

@vicroms vicroms merged commit e9f8cc6 into microsoft:master Oct 26, 2020
Jimmy-Hu added a commit to Jimmy-Hu/vcpkg that referenced this pull request Oct 27, 2020
[vcpkg] Implement versions db generator (microsoft#13777)
@vicroms vicroms deleted the versioning/get-versions-from-history branch October 27, 2020 17:01
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
microsoft#14079)

* [vcpkg] Add `versions` feature flag and version field manifest parsing

* Introduce FeatureFlagSettings struct to more easily access feature flags throughout the program
* To avoid users accidentally starting to write "version" instead of "version-string" in their manifests, vcpkg explicitly detects and prevents usage of ports with schemes other than "String"
* Drive-by fix of copiable SourceControlFileLocation and an exposed use-after-move bug

This code is largely extracted from PR microsoft#13777

Co-authored-by: Victor Romero <romerosanchezv@gmail.com>

* [vcpkg] Address CR Comments. Fix test crash on Linux.

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Victor Romero <romerosanchezv@gmail.com>
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
* [vcpkg] Add script to generate ports versions history

* [vcpkg] Fix formatting

* Fetch port versions from commit ID

* Use global --x-json switch

* Use --no-checkout when cloning secondary instance

* Clone from local repository instead of from GitHub

* Use CmdLineBuilder to build git commands

* Use CmdLineBuilder and reduce repeated code

* Fetch version at baseline and code cleanup

* Guess version scheme from old CONTROL files

* Rename version db generator script

* Simplify x-history json output

* Use CONTROL/manifest parsers on x-history

* Use git-tree instaed of commit-id

* Remove 'ports' field from root object

* Clean up code

* More code cleanup

* Improve port version detection

* Improve generator logging

* Do not ignore parsing errors in CONTROL files

* PR review comments in Python script

* Fix subprocess.run() calls

* Make `canonicalize()` return error instead of terminating

* [vcpkg] Add tests for new test_parse_control_file paths

* Remove unnecessary std::move() calls

* Fix formatting

* Python formatting

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
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.

6 participants