Skip to content

tools.vpm: fix installing of modules with conflicting names, extend tests - #19961

Merged
spytheman merged 6 commits into
vlang:masterfrom
ttytm:tools/vpm/validate-manifest-before-install
Nov 23, 2023
Merged

spytheman merged 6 commits into
vlang:masterfrom
ttytm:tools/vpm/validate-manifest-before-install

Conversation

@ttytm

@ttytm ttytm commented Nov 21, 2023 •

Copy link
Copy Markdown
Member

fixes: https://discord.com/channels/592103645835821068/618363736503353374/1176466062682882058

This includes validating module data running the actual install. This is also a major step to improve dependency resolution.

🤖[deprecated] Generated by Copilot at 9025e7a

This pull request refactors and tests the vpm tool for managing modules in V. It uses the vmod.Manifest struct for module metadata, improves the handling of module identifiers and statuses, and removes unused or redundant code. It also adds or updates tests for the fetch_manifest function and the installation of modules.

🤖[deprecated] Generated by Copilot at 9025e7a

  • Add a new field manifest to the Module struct to store the parsed data from the v.mod file of a module (link)
  • Add a new constant vexe to hold the quoted path of the V executable and use it instead of the VEXE environment variable in the tests and the source code (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Add a new function fetch_manifest to download and decode the manifest from the module URL, handle different cases of branch names, URL formats, and missing or invalid manifests, and relocate the module based on the manifest name (link, link, link, link, link)
  • Add a new method get_installed to the Module struct to check the installation status and version of a module using the git ls-remote command and set the is_installed and installed_version fields accordingly (link)
  • Move the code for installing a module from the install.v file to the common.v file as part of the install_modules function, which is shared by both the install.v and the update.v files (link, link, link)
  • Add a comment to clarify that the identifier of an external module is an URL, not a name (link)
  • Resolve the path of the module based on the manifest name, instead of the identifier name, to ensure the correct installation directory (link)
  • Remove the unused variable last_errors, which was used to compare the number of errors before and after installing a module (link, link)
  • Remove the unused variable vexe, which was used to get the path of the V executable from the VEXE environment variable (link)

@ttytm ttytm changed the title refactor: abstract getting installed module data, extend tests tools.vpm: fix installing of modules with conflicting names, extend tests Nov 21, 2023
@ttytm
ttytm force-pushed the tools/vpm/validate-manifest-before-install branch from 3e43ab4 to 9cb9129 Compare November 21, 2023 14:19
Comment thread cmd/tools/vpm/common.v Outdated
@ttytm
ttytm requested a review from spytheman November 23, 2023 12:15

@spytheman spytheman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Excellent work.

@spytheman
spytheman merged commit 29eda89 into vlang:master Nov 23, 2023
@ttytm
ttytm deleted the tools/vpm/validate-manifest-before-install branch November 23, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants