Conversation
Summary of ChangesHello @kalbasit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines how NAR (Nix Archive) files are served and processed, primarily by enhancing the test server's ability to handle various NAR URL formats, including those with and without compression extensions, and supporting lookups by different hash representations. Concurrently, it streamlines the upstream cache logic by removing a manual URL prefix trimming workaround, relying instead on a centralized normalization utility. These changes collectively improve the robustness and consistency of NAR file handling. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request effectively improves NAR file serving in the testdata server and removes an outdated URL prefix workaround in the upstream cache. The changes ensure that the test server correctly handles NAR file paths with compression extensions and supports lookups by both original and normalized NAR hashes. The removal of the strings.ReplaceAll workaround in pkg/cache/upstream/cache.go is a good step towards cleaner code, as URL normalization is now handled by the nar.URL.Normalize() method.
372be8b to
3d6c7ca
Compare
fd85478 to
7800e7c
Compare
3d6c7ca to
6b35b08
Compare
7800e7c to
0b7597e
Compare
6b35b08 to
4a589ea
Compare
Update testdata server to properly handle NAR file paths with compression extensions and support lookups by both original and normalized NAR hashes. Also remove the old string replacement workaround from upstream cache since URLs are now normalized through the Normalize() method. Changes: - testdata/server.go: - Fix path construction to only append compression extension if non-empty - Add support for fetching narinfo by NAR hash (used when only NAR hash available) - Add support for fetching NARs by normalized hash (with prefix stripped) - Build normalized NAR paths with proper compression extension handling - pkg/cache/upstream/cache.go: - Remove ReplaceAll workaround for URL prefix trimming - URL normalization now handled via nar.URL.Normalize() This fixes issues with NAR files that have no compression (extension would have been ".nar." instead of ".nar") and ensures test server can serve NARs regardless of whether they're requested using the original prefixed hash or the normalized hash. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
4a589ea to
44963b2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #838 +/- ##
=======================================
Coverage 85.41% 85.41%
=======================================
Files 2 2
Lines 480 480
=======================================
Hits 410 410
Misses 65 65
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ackport #838] Update testdata server to properly handle NAR file paths with compression extensions and support lookups by both original and normalized NAR hashes. Also remove the old string replacement workaround from upstream cache since URLs are now normalized through the Normalize() method. Changes: - testdata/server.go: - Fix path construction to only append compression extension if non-empty - Add support for fetching narinfo by NAR hash (used when only NAR hash available) - Add support for fetching NARs by normalized hash (with prefix stripped) - Build normalized NAR paths with proper compression extension handling - pkg/cache/upstream/cache.go: - Remove ReplaceAll workaround for URL prefix trimming - URL normalization now handled via nar.URL.Normalize() This fixes issues with NAR files that have no compression (extension would have been ".nar." instead of ".nar") and ensures test server can serve NARs regardless of whether they're requested using the original prefixed hash or the normalized hash. Part of #806 --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com> (cherry picked from commit 2831749)
…ackport #838] Update testdata server to properly handle NAR file paths with compression extensions and support lookups by both original and normalized NAR hashes. Also remove the old string replacement workaround from upstream cache since URLs are now normalized through the Normalize() method. Changes: - testdata/server.go: - Fix path construction to only append compression extension if non-empty - Add support for fetching narinfo by NAR hash (used when only NAR hash available) - Add support for fetching NARs by normalized hash (with prefix stripped) - Build normalized NAR paths with proper compression extension handling - pkg/cache/upstream/cache.go: - Remove ReplaceAll workaround for URL prefix trimming - URL normalization now handled via nar.URL.Normalize() This fixes issues with NAR files that have no compression (extension would have been ".nar." instead of ".nar") and ensures test server can serve NARs regardless of whether they're requested using the original prefixed hash or the normalized hash. Part of #806 --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com> (cherry picked from commit 2831749)

Update testdata server to properly handle NAR file paths with compression
extensions and support lookups by both original and normalized NAR hashes.
Also remove the old string replacement workaround from upstream cache since
URLs are now normalized through the Normalize() method.
Changes:
testdata/server.go:
pkg/cache/upstream/cache.go:
This fixes issues with NAR files that have no compression (extension would have
been ".nar." instead of ".nar") and ensures test server can serve NARs regardless
of whether they're requested using the original prefixed hash or the normalized hash.
Part of #806
Co-Authored-By: Claude Haiku 4.5 noreply@anthropic.com