go-selfupdate icon indicating copy to clipboard operation
go-selfupdate copied to clipboard

Add 386 architecture mappings and GitHub Proxy support

Open sinspired opened this issue 1 week ago • 4 comments

I use main fork in my project,I will cancel the old pr and now reopen a new PR。

Summary by CodeRabbit

  • New Features

    • Added support for downloading assets via GitHub Proxy
    • Extended 386 architecture support with additional architecture aliases
  • Tests

    • Enhanced test coverage for 386 architecture variants

✏️ Tip: You can customize this high-level summary in your review settings.

sinspired avatar Dec 20 '25 07:12 sinspired

Walkthrough

The changes extend architecture mapping to support 386 aliases (i386, x86, x86_32), add corresponding test cases for 386 architecture handling, and implement GitHub Proxy asset download detection with direct HTTP GET requests triggered by multiple "https://" occurrences in AssetURL.

Changes

Cohort / File(s) Change Summary
Architecture mapping enhancements
arch.go, arch_test.go
Adds conditional logic to map "386" architecture to additional aliases (i386, x86, x86_32); extends TestAdditionalArch with two new test cases covering 386 with and without universal arch
GitHub Proxy asset download handling
github_source.go
Detects proxy usage by counting "https://" occurrences in AssetURL; when detected, performs direct HTTP GET request instead of standard GitHub API path; selects download URL based on AssetID matching; adds strings import

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • github_source.go: Proxy detection via string occurrence counting and conditional URL selection logic requires careful verification of edge cases and error handling for HTTP non-OK responses
  • arch.go: Verify that new 386 mappings are applied in correct order relative to universalArch handling
  • arch_test.go: Confirm test expectations cover both universal and non-universal cases for 386

Poem

🐰 A rabbit hops through architectures new,
From x86 to 386, we brew,
GitHub proxies found in URLs so long,
Direct downloads whisper their song! 🎵

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the two main changes: adding 386 architecture mappings and GitHub Proxy support for asset downloads.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 20 '25 07:12 coderabbitai[bot]

Codecov Report

:x: Patch coverage is 14.28571% with 18 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 76.95%. Comparing base (d7d55b9) to head (496a584). :warning: Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
github_source.go 5.26% 17 Missing and 1 partial :warning:

:x: Your patch check has failed because the patch coverage (14.29%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #51      +/-   ##
==========================================
- Coverage   77.07%   76.95%   -0.12%     
==========================================
  Files          28       28              
  Lines        1435     1167     -268     
==========================================
- Hits         1106      898     -208     
+ Misses        279      218      -61     
- Partials       50       51       +1     
Flag Coverage Δ
unittests 76.95% <14.29%> (-0.12%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Dec 21 '25 11:12 codecov[bot]

Hi!

Thank you for this pull request! Sorry it took me some time to look at it.

I'm a little bit concerned that the check for the double https:// could break some other users workflow.

Have you tried to check if the Github API is returning the redirected url as a second argument?

On line 139 in the PR, the s.api.Repositories.DownloadReleaseAsset could return nil, redirectedURL, nil If this is the case, we can download the asset manually once we know the Github API cannot.

From the GitHub documentation:

DownloadReleaseAsset returns an io.ReadCloser that reads the contents of the specified release asset. It is the caller's responsibility to close the ReadCloser. If a redirect is returned, the redirect URL will be returned as a string instead of the io.ReadCloser. Exactly one of rc and redirectURL will be zero.

Sorry I cannot try it myself as I don't know what kind of setup you have.

creativeprojects avatar Dec 21 '25 16:12 creativeprojects

I didn't change your original code, It will be working as normal as it would be. My code will be work Only when the url has two "https" like https://githubporxy.com/https://raw.githubuserconten.com/...

sinspired avatar Dec 21 '25 17:12 sinspired