-
Notifications
You must be signed in to change notification settings - Fork 1.5k
hwinfo: Move to Sourceforge #16452
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
base: master
Are you sure you want to change the base?
hwinfo: Move to Sourceforge #16452
Conversation
WalkthroughManifest refactored and version bumped to 8.32. Download URL moved to SourceForge, license field structured with URL, architecture-specific pre_install scripts added, top-level Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as Scoop
participant PI as pre_install (arch)
participant IS as installer.script
participant P as persist_dir
rect rgba(120,180,240,0.12)
U->>S: scoop install hwinfo --arch [arch]
S->>S: extract archive
end
rect rgba(180,220,160,0.12)
S->>PI: run pre_install for chosen arch
PI->>PI: remove other-arch files\nrename arch executable -> HWiNFO.exe
end
rect rgba(240,200,140,0.12)
S->>IS: run installer.script
IS->>P: create/link HWiNFO_[ARCH].INI (persist)
IS->>P: copy/restore *_KEY.txt between $dir and $persist_dir
IS->>S: create shim & shortcut, finish install
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
|
All changes look good. Wait for review from human collaborators. hwinfo
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/hwinfo.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (4)
bucket/hwinfo.json (4)
53-57: Checkver regex and replace pattern verified as correct.The regex
Windows_Portable/hwi_(\d)(\d{2})\.zipwith replace$1.$2successfully extracts and formats versions correctly across all test cases: current version (hwi_832.zip → 8.32) and future versions (hwi_930.zip → 9.30, hwi_100.zip → 1.00, etc.). No issues found.
58-60: No issues found. The$cleanVersionvariable is correctly used.HWInfo's SourceForge downloads confirm the correct naming: the latest version
hwi_832.zipcorresponds to version 8.32. This matches how Scoop's$cleanVersionremoves separators from version strings. The autoupdate URL will resolve correctly when Scoop processes version updates.
10-10: Verify SHA1 hash and explain algorithm downgrade from SHA256.No published checksums for HWiNFO v8.32 are available on the official download page or SourceForge. Manually verify the SHA1 hash (
2d56090a3f75020d7801d5d7175c791868dfc60c) matches the downloadedhwi_832.zipfile usingcertutil -hashfile hwi_832.zip SHA1(Windows) orsha256sumafter download. Additionally, clarify why the hash algorithm was downgraded from SHA256 to SHA1—SHA1 is cryptographically weaker and deprecated for security purposes.
31-40: Functions are valid Scoop functions; verify file pattern assumption in SourceForge package.The concerns about
persist_dataandunlink_persist_datacan be removed: these are standard Scoop functions implemented in Scoop's PowerShell libs (lib/install.ps1, lib/versions.ps1) and available within manifest installer/uninstaller hook contexts.Verified as correct:
- Line 33 regex (
\\d{2}(?=bit)): Correctly extracts 32 or 64 from "32bit" and "64bit" strings; correctly excludes "arm64".- Line 37 & 51 function signatures: Align with expected Scoop patterns for restoring and unlinking persisted data.
Requires manual verification:
- Line 38 file pattern (
*64_KEY.txt): The manifest assumes license key files matching this pattern exist in the SourceForge HWiNFO package. Verify that the actual SourceForge Windows Portable distribution contains files matching this pattern; if not, the Copy-Item command will silently produce no errors, resulting in missing license files on fresh installs.
|
When writing the manifest in #16366, I actually considered using SourceForge. However, the For that reason, I decided not to adopt this approach. Furthermore, I don’t think the Since the Moreover, Scoop’s manifest update mechanism appears to be forced, meaning the hash can still be corrected even without relying on this |
f094fff to
aaa715a
Compare
|
With regex, it's not too hard to segment the majorVer yourself (as long as some version doesn't hit 100 minor vers, since there's no zero, I'd hope not), ofc it would be better to have the /ver.txt endpoint, but I think this is the safest/best option for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
bucket/hwinfo.json (1)
48-52: Verifyunlink_persist_datafunction availability.The pre_uninstall script calls
unlink_persist_data(line 51), which mirrors the installer's use ofpersist_data. This is subject to the same verification requirement: these functions must be defined and available at runtime or the uninstall will fail.Refer to the critical issue raised at lines 31–40 for verification requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/hwinfo.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T09:37:06.093Z
Learnt from: o-l-a-v
PR: ScoopInstaller/Extras#16328
File: bucket/winutil.json:0-0
Timestamp: 2025-10-13T09:37:06.093Z
Learning: When reviewing code in Scoop manifests, always verify the actual file content if the diff appears incomplete or unusual, as diff context can be truncated or displayed incorrectly. The pattern `[string[]](...).ForEach{...}` is commonly used in post_uninstall scripts across the Extras repository.
Applied to files:
bucket/hwinfo.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (4)
bucket/hwinfo.json (4)
41-47: Clean and correct bin/shortcuts configuration.The unified executable name aligns well with the architecture-specific renaming logic in pre_install. After all architectures normalize their executables to
HWiNFO.exe, the top-level bin and shortcuts correctly reference this single name.
2-10: SourceForge only provides SHA1 and MD5 hashes—SHA256 is unavailable. The hash algorithm change is necessary and correct.The concern about the SHA1 downgrade is based on incomplete information. Verification of SourceForge's file metadata confirms that only SHA1 and MD5 are available for this release; SHA256 is not provided. The SHA1 hash used in the manifest (
2d56090a3f75020d7801d5d7175c791868dfc60c) matches SourceForge's records exactly. Using SHA1 is not optional—it's the sole cryptographic hash option SourceForge offers for this file, making the algorithm change a necessary tradeoff when migrating from the original download source.Likely an incorrect or invalid review comment.
53-60: ****The SourceForge checkver regex is correctly configured. Verification confirms that actual SourceForge releases (hwi_762.zip through hwi_832.zip) match the expected pattern and parse correctly:
- Current releases (3-digit format): hwi_762 → 7.62, hwi_832 → 8.32
- Future releases (4-digit format): hwi_1032 → 10.32 via regex backtracking
The regex
Windows_Portable/hwi_(\d+)(\d{2})\.zipwith replacement$1.$2properly handles both cases and resolves the prior concern about major version ≥ 10.
31-40: The original review comment is based on incorrect assumptions. Bothpersist_dataandunlink_persist_dataare standard Scoop built-in functions implemented in Scoop's install/version libraries, and$original_diris a standard variable available inside Scoop manifest scripts. The installer script code is correct as written, and installation will proceed without runtime errors related to undefined functions or variables.Likely an incorrect or invalid review comment.
Yes, if However, this would inevitably increase maintenance frequency and could introduce other hidden issues — which is exactly why I chose not to use SourceForge as the Hopefully, the author of |
This is an inevitability, and there's no reason to alter it in the future, the |
|
Oh, sorry, I didn’t notice before that you had already updated the regex to I hadn’t given this issue much thought earlier and hadn’t considered that this approach could solve the problem. Thank you for your response. As for the possibility of |
|
Hello, upstream has relaxed the rules, so we can now use If you agree, you can close this PR. I'll revert to the original |
|
So I think that the |
aaa715a to
a18c87c
Compare
|
I have no problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
bucket/hwinfo.json (1)
14-14: Replace-ErrorAction SilentlyContinuewith explicit error handling.The pre_install scripts continue to use
-ErrorAction SilentlyContinueto silently ignore failures during file removal and renaming. This masks errors and violates Scoop best practices. Failures should abort the installation using explicittry { … } catch { Write-Error "…"; throw }blocks to fail fast.Apply this diff to add explicit error handling across all three architecture sections:
- "Get-ChildItem -Path $dir -Include '*32*', '*ARM*' -Recurse | Remove-Item -Force -ErrorAction SilentlyContinue", + "Get-ChildItem -Path $dir -Include '*32*', '*ARM*' -Recurse | ForEach-Object { try { Remove-Item -Path $_.FullName -Force } catch { Write-Error \"Failed to remove $_: $_\"; throw } }", - "Get-ChildItem -Path \"$dir\\*32*\" -Recurse | Rename-Item -NewName { $_.FullName -replace '\\d{2}\\.', '.' }", - "Get-ChildItem -Path $dir -Include '*64*' -Recurse | Remove-Item -Force -ErrorAction SilentlyContinue" + "Get-ChildItem -Path \"$dir\\*32*\" -Recurse | ForEach-Object { try { Rename-Item -Path $_.FullName -NewName ($_.FullName -replace '\\d{2}\\.', '.') } catch { Write-Error \"Failed to rename $_: $_\"; throw } }", + "Get-ChildItem -Path $dir -Include '*64*' -Recurse | ForEach-Object { try { Remove-Item -Path $_.FullName -Force } catch { Write-Error \"Failed to remove $_: $_\"; throw } }"Similarly for ARM64 (lines 26–27).
Also applies to: 20-21, 26-27
🧹 Nitpick comments (1)
bucket/hwinfo.json (1)
53-59: Hybrid checkver/autoupdate approach addresses SourceForge limitations; monitor for future version edge cases.The PR uses a hybrid approach (as discussed in the PR comments): checkver sources
/ver.txtfor version detection, while autoupdate constructs the URL from SourceForge using$majorVersion$minorVersionconcatenation. This mitigates Cloudflare bot protections on hwinfo.com while avoiding the SourceForge naming ambiguity.Caveat: For future versions >= 10 (e.g., "10.32"), the autoupdate URL pattern
hwi_$majorVersion$minorVersion.zipwill producehwi_1032.zip. Verify this matches SourceForge's actual naming scheme when version 10 is released, as the pattern relies on majorVersion/minorVersion splitting, which can be ambiguous for multi-digit major versions.Consider adding a comment in the manifest documenting this limitation for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/hwinfo.json(1 hunks)
🔇 Additional comments (4)
bucket/hwinfo.json (4)
31-40: Verify Scoop API function signatures forpersist_dataandunlink_persist_data.The manifest uses
persist_data @{ persist = "$setting_name" } $original_dir $persist_dir(line 37) andunlink_persist_data @{ persist = "$setting_name" } $dir(line 51). Confirm:
- These are valid Scoop built-in functions and the parameter syntax is correct.
$original_diris the correct variable name (should it be$dirduring install?).- The hashtable format
@{ persist = "..." }matches Scoop's expected API.Search Scoop's documentation or codebase for these function signatures to confirm correctness.
Also applies to: 48-52
33-33: Verify Scoop passes$architecturevariable in the expected format.The installer and pre_uninstall scripts use regex patterns like
$architecture -match '\\d{2}(?=bit)'(lines 33, 50) to extract version numbers from architecture strings (e.g., "64bit" → "64"). Confirm that:
- Scoop provides
$architecturein a format matching this pattern (e.g., "64bit", "32bit").- The lookahead regex correctly captures the version digits.
- The fallback for ARM64 (
else { 'HWiNFO_ARM64.INI' }) is accurate.Also applies to: 50-50
10-10: I'll search for information about Scoop's manifest format requirements regarding hash algorithms, then provide guidance.Verify SHA1 hash value is correct; SHA1 format is supported but SHA256 is recommended.
Scoop manifests support SHA1, SHA256, SHA512, and MD5 hash algorithms via prefixed format, so the syntax at line 10 is valid. However, SHA1 is no longer collision-resistant as of 2017, making SHA256 recommended over SHA1.
Two remaining concerns:
- Hash correctness: Unable to verify the specific SHA1 value against the SourceForge file in this environment. Manually confirm the hash matches the downloaded
hwi_832.zipfile.- Algorithm change rationale: Clarify why the hash algorithm changed from SHA256 to SHA1—whether due to a version bump, different build, or other reason. If the file wasn't re-verified, consider updating to SHA256 for better security.
13-29: Let me search for information about HWiNFO's SourceForge release naming conventions.Based on the available information, I can confirm SourceForge distributes a portable package containing HWiNFO® 32, HWiNFO® 64 and HWiNFO® ARM64. However, I was unable to directly inspect the archive contents due to network constraints, so I cannot definitively verify the exact binary naming patterns in the hwi_832.zip file or confirm that the pre_install regexes produce the expected renamed filenames.
The regex patterns themselves follow standard PowerShell conventions and the naming scheme aligns with industry standards for architecture-specific binaries. However, without direct access to the archive, I cannot confirm:
- The exact filenames of the architecture-specific binaries in the SourceForge archive
- Whether the pre_install scripts correctly rename files to the unified
HWiNFO.exeentry expected by the manifestManually verify the archive structure and test the pre_install rename regexes with actual SourceForge binaries to ensure correct behavior across all architectures.
hwinfo's website should be avoided as aDiscussed with the developer in https://www.hwinfo.com/forum/threads/10829/.checkversource, the site uses strong Cloudflare bot protectionsCloses #16366
Fixes #16361
Relates to #16379
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit