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

(#3613) Add source uri override for download uris #3615

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

AdmiringWorm
Copy link
Member

@AdmiringWorm AdmiringWorm commented Jan 31, 2025

Description Of Changes

This adds a new Package Downloader class and have it call a resolve
redirection for download URIs to the original source URI.

This allows the same credentials to be used for the download URI as with
the source URI when the server is configured to store binary files in
a different location.

Motivation and Context

Without this change, it makes it impossible to install packages that have a download location that do not have the source URI as its prefix.

Testing

  1. Install a package from the following location. CCR, Licensed Feed, Nexus Feed and ProGet feed, local package directory and a remote package directory.
  2. Install another package, that have one or more dependencies with the above feed types.

Operating Systems Testing

  • Windows 10

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed? (A few failures locally, but these have been seen before and only on my VM)
  • PowerShell code changes: PowerShell v3 compatibility checked?

Related Issue

@AdmiringWorm AdmiringWorm self-assigned this Jan 31, 2025
@AdmiringWorm AdmiringWorm requested review from corbob and gep13 January 31, 2025 13:46
@corbob corbob linked an issue Jan 31, 2025 that may be closed by this pull request
6 tasks
This adds a new Package Downloader class and have it call a resolve
redirection for download URIs to the original source URI.

This allows the same credentials to be used for the download URI as with
the source URI when the server is configured to store binary files in
a different location.
@AdmiringWorm AdmiringWorm marked this pull request as ready for review January 31, 2025 15:41
Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

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

Just one minor style thing. And a gripe about BOM and diff interfaces: I have no way of telling without a bunch of manual work if the files that got new copyright lines also changed their BOM/encoding like the csproj file 😬

@corbob
Copy link
Member

corbob commented Jan 31, 2025

The ubuntu GHA is failing because mono isn't on that version anymore. Would we want to bring a maint commit that mirrors this one: ecd3e4e

@AdmiringWorm
Copy link
Member Author

The ubuntu GHA is failing because mono isn't on that version anymore. Would we want to bring a maint commit that mirrors this one: ecd3e4e

I plan to, but it won't be until after this PR (most likely).

However, feel free to cherry-pick it into the hotfix branch if you want.

Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

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

Looks good to me. All of the tests I put at it worked as expected.

@corbob corbob merged commit 4004540 into chocolatey:hotfix/1.4.3 Jan 31, 2025
4 of 5 checks passed
@corbob
Copy link
Member

corbob commented Jan 31, 2025

Thanks for getting this updated @AdmiringWorm 👍

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.

Unable to upgrade packages on certain authenticated feeds
2 participants