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

(#40) Handle long file paths when caching files #41

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

gep13
Copy link
Member

@gep13 gep13 commented Jun 13, 2023

Description Of Changes

This commit adds a new helper method, GetHashedCacheFileName. which is
used to calculate the hashed file name, in a single location, rather
than duplicating the work in multiple places.

Motivation and Context

During testing, it was found that there are cases where a very long
query string was causing a problem when caching the file earlier in the
callstack than the earlier testing. This problem is caused when a call
into a BCL method throws a long file path exception, even though the
Operating System has been enabled to use long path file support. Due
to this, we have introduced a check to see if the file path is longer
than the allowed length, and if it is, immediately create the hashed
filename.

Testing

  1. Check out this PR
  2. Ensure you have the most recent chocolatey/choco code on your machine
  3. Run the update-nuget-client script in the chocolatey/choco repository
  4. Clear out the ChocolateyHttpCache folder
  5. Open fiddler, and have it running throughout the testing
  6. Run the following command choco search --source chocolatey.licensed (use the full URL if required)
  7. In fiddler, witness that there are three outgoing queries performed, and that two new files are added to the ChocolateyHttpCache folder. One of these will use a hashed file name, rather than the URI of the query
  8. Run choco search --source chocolatey.licensed again
  9. Verify that in Fiddler there is only one outgoing query this time
  10. Verify that the same list of packages is returned as the initial execution

Operating Systems Testing

Win10

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?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

@gep13 gep13 requested a review from AdmiringWorm June 13, 2023 09:51
@gep13 gep13 force-pushed the long-file-path-cache branch from 48a837a to cb518d1 Compare June 13, 2023 10:25
@gep13 gep13 requested a review from AdmiringWorm June 13, 2023 10:25
During testing, it was found that there are cases where a very long
query string was causing a problem when caching the file earlier in the
callstack than the earlier testing.  This problem is caused when a call
into a BCL method throws a long file path exception, even though the
Operating System has been enabled to use long path file support.  Due
to this, we have introduced a check to see if the file path is longer
than the allowed length, and if it is, immediately create the hashed
filename.

This commit adds a new helper method, GetHashedCacheFileName. which is
used to calculate the hashed file name, in a single location, rather
than duplicating the work in multiple places.
@gep13 gep13 force-pushed the long-file-path-cache branch from cb518d1 to da65b92 Compare June 13, 2023 13:09
Copy link
Member

@AdmiringWorm AdmiringWorm left a comment

Choose a reason for hiding this comment

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

LGTM

@AdmiringWorm AdmiringWorm merged commit a3a4643 into chocolatey:develop Jun 13, 2023
@gep13 gep13 deleted the long-file-path-cache branch June 13, 2023 13:23
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.

Handle potential for long file paths when creating cache files
2 participants