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

Add prerelease string when NormalizedVersion doesn't exist (but prelease string does) #1681

Conversation

sean-r-williams
Copy link
Contributor

PR Summary

This PR conditionally adds the package prerelease string to the to-be-installed package version name (passed to InstallVersion()) inside InstallHelper.BeginPackageInstall()

PR Context

Currently, the version string is backfilled with the package's raw Version field if NormalizedVersion was not returned by the package management service. This causes a difference in behavior:

  • For sources that provide NormalizedVersion, the prerelease string is included in NormalizedVersion and is included in pkgVersion as a result.
  • For sources that don't provide it, System.Version does not include prerelease info and as-such the prerelease string is not included in pkgVersion.

This PR reconciles the second case to include prerelease strings, aligning it with the first.

Fixes #1680.

Concerns

I don't have visibility into NormalizedVersion behavior on other package management services (only Artifactory and PSGallery), so I'm not 100% sure if pkgToInstall.Prerelease is the correct field to rely on.

PR Checklist

@alerickson
Copy link
Member

Thanks @sean-r-williams! The PR looks good, pkgToInstall.Prerelease is the correct field to rely on. Could you add a comment related to why you're fixing this to help prevent regression until we can get proper Artifactory tests in place?

@sean-r-williams
Copy link
Contributor Author

@alerickson I added a comment in e05bbef.

I want to double-click on the context here: This should impact any NuGet provider where NormalizedVersion isn't sent. (The PR where these lines were last changed mentioned ACR - does ACR send NormalizedVersion?) Artifactory is an example of this (and Artifactory integration tests would've caught it, sure) but I don't think they're unique in that regard.

@alerickson
Copy link
Member

@sean-r-williams, yup that makes sense, I just haven't been able to replicate in an environment that we have tests for at the moment.

@alerickson
Copy link
Member

/azp

Copy link

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@alerickson
Copy link
Member

/azp run PowerShell.PSResourceGet

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@alerickson alerickson enabled auto-merge (squash) August 6, 2024 23:26
@alerickson alerickson merged commit 8389b04 into PowerShell:master Aug 6, 2024
12 checks passed
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.

Artifactory+NuGet v2: Save/Install broken for prereleases due to incorrect URL
2 participants