Skip to content

Conversation

@brian6932
Copy link
Contributor

@brian6932 brian6932 commented Oct 24, 2025

hwinfo's website should be avoided as a checkver source, the site uses strong Cloudflare bot protections Discussed with the developer in https://www.hwinfo.com/forum/threads/10829/.
Closes #16366
Fixes #16361
Relates to #16379

  • Use conventional PR title: <manifest-name[@version]|chore>: <general summary of the pull request>
  • I have read the Contributing Guide

Summary by CodeRabbit

  • Chores
    • Updated to version 8.32
    • Switched download and autoupdate source to SourceForge
    • License now includes an official reference URL
    • Unified executable and shortcut configuration for easier access
    • Installer and uninstall behavior improved to better preserve and restore user data
    • Architecture-specific install scripts added for 64-bit, 32-bit, and ARM64 builds

@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2025

Walkthrough

Manifest 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 bin/shortcuts introduced, persist replaced by an installer script that handles persistence and pre_uninstall restore, and checkver/autoupdate updated.

Changes

Cohort / File(s) Summary
hwinfo manifest refactor
bucket/hwinfo.json
Version updated to 8.32. homepage normalized. license changed from string to object with identifier and url. Download url changed to SourceForge and hash replaced with sha1:2d56090a3f75020d7801d5d7175c791868dfc60c. Removed top-level persist; added per-architecture pre_install scripts and a unified installer.script implementing persistence (create/link config INI, copy key files). Top-level bin and shortcuts added. pre_uninstall adjusted to restore persisted files. checkver and autoupdate updated with refined regex and SourceForge patterns.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to the installer script logic that replaces persist (edge cases for copying/restoring keys and INI linking).
  • Verify architecture-specific pre_install pruning/renaming for correctness and no accidental deletions.
  • Confirm checkver/autoupdate regex and URL substitutions align with SourceForge naming.

Suggested reviewers

  • z-Fng

Poem

🐰 Hopped a patch to SourceForge bright,

Trimmed the extras, set each arch aright.
INI links stitched and keys kept near,
One exe to run, shortcuts appear —
Eight‑thirty‑two now hops with cheer.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "hwinfo: Move to Sourceforge" directly describes the primary change in the changeset, which involves migrating the download source and related configurations from the original source to SourceForge. The title follows the conventional format specified in the repository template and is concise, clear, and specific enough for a developer scanning the history to understand the primary objective of this change.
Linked Issues Check ✅ Passed The code changes comprehensively address the objectives from linked issues #16366 and #16361. From #16366, the PR implements version update to 8.32, migration to SourceForge URL with updated hash, structured license field with reference URL, per-architecture pre_install scripts, installer script with persistence logic, simplified bin/shortcuts definitions, and updated checkver/autoupdate rules. From #16361, the changes fix the version detection issue by updating checkver and resolve the 404 error by switching to a valid SourceForge mirror. All primary coding requirements from both issues are fulfilled.
Out of Scope Changes Check ✅ Passed All modifications in the changeset are directly aligned with and necessary to support the objectives outlined in the linked issues. The homepage trailing slash addition, license field restructuring, SourceForge URL migration, hash update, architecture-specific pre_install refactoring, installer script implementation, bin/shortcuts consolidation, and checkver/autoupdate revisions are all explicit requirements from #16366 or #16361. No extraneous changes unrelated to the stated objectives are present.
Description Check ✅ Passed The pull request description adheres to the repository template by including proper issue references (Closes #16366, Fixes #16361, Relates to #16379), providing a clear rationale about Cloudflare bot protections, and confirming both required checkboxes regarding the conventional PR title format and the Contributing Guide review. All mandatory template elements are present and filled out appropriately.
✨ Finishing touches
🧪 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.

@github-actions
Copy link
Contributor

All changes look good.

Wait for review from human collaborators.

hwinfo

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

Check the full log for details.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 448bf3d and f094fff.

📒 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})\.zip with replace $1.$2 successfully 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 $cleanVersion variable is correctly used.

HWInfo's SourceForge downloads confirm the correct naming: the latest version hwi_832.zip corresponds to version 8.32. This matches how Scoop's $cleanVersion removes 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 downloaded hwi_832.zip file using certutil -hashfile hwi_832.zip SHA1 (Windows) or sha256sum after 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_data and unlink_persist_data can 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.

@SorYoshino
Copy link
Contributor

SorYoshino commented Oct 24, 2025

When writing the manifest in #16366, I actually considered using SourceForge.

However, the $majorVersion and $minorVersion values retrieved from there are concatenated together, making it difficult to distinguish between them when $majorVersion is greater than or equal to 10.

For that reason, I decided not to adopt this approach.


Furthermore, I don’t think the -patch information you mentioned in the forum is a real concern.

Since the hwinfo manifest was first added to the Extras bucket in 2018, the -patch field has only had an actual effect in two cases — in commits 7bcb67e and 4982413. Even then, there was a suspicious detail: when updating from 7.22-4730 to 7.22-4731, the corresponding hash value didn’t change.

Moreover, Scoop’s manifest update mechanism appears to be forced, meaning the hash can still be corrected even without relying on this -patch part.

@brian6932
Copy link
Contributor Author

brian6932 commented Oct 24, 2025

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.

Copy link

@coderabbitai coderabbitai bot left a 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: Verify unlink_persist_data function availability.

The pre_uninstall script calls unlink_persist_data (line 51), which mirrors the installer's use of persist_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

📥 Commits

Reviewing files that changed from the base of the PR and between f094fff and aaa715a.

📒 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})\.zip with replacement $1.$2 properly handles both cases and resolves the prior concern about major version ≥ 10.


31-40: The original review comment is based on incorrect assumptions. Both persist_data and unlink_persist_data are standard Scoop built-in functions implemented in Scoop's install/version libraries, and $original_dir is 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.

@SorYoshino
Copy link
Contributor

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),

Yes, if $majorVersion reaches 10 or a higher number, it can be matched correctly by simply changing the pattern to \d{2}. Similarly, if $minorVersion reaches 100 or more, a similar adjustment can be made.

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 checkver source.

Hopefully, the author of hwinfo will allow us to use ver.txt, since both of the current checkver approaches have certain limitations.

@brian6932
Copy link
Contributor Author

Yes, if $majorVersion reaches 10 or a higher number, it can be matched correctly by simply changing the pattern to \d{2}.

This is an inevitability, and there's no reason to alter it in the future, the + is enough. The minor version getting to 100 I'm very doubtful of, as there's no zero to indicate the placement, it's a possibility, but extremely small.

@SorYoshino
Copy link
Contributor

Oh, sorry, I didn’t notice before that you had already updated the regex to hwi_(\d+)(\d{2})\.zip.

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 $minorVersion having more than two digits, since the manifest was added to the Extras bucket in 2018, such a case has never occurred and can be considered extremely unlikely. On this basis, this method indeed doesn’t pose any significant issues.

@SorYoshino
Copy link
Contributor

Hello, upstream has relaxed the rules, so we can now use ver.txt. There's no need for us to modify the checkver logic at this point—just keep the refactoring to the manifest.

If you agree, you can close this PR. I'll revert to the original checkver approach in #16366. Alternatively, you can directly restore the original checkver method in this PR—either way is fine with me.

@brian6932
Copy link
Contributor Author

So I think that the autoupdate bins, and hash should still use Sourceforge, but checkver should be using /ver.txt. This way we get hash checking, and don't have to use a mirror. I'll add it onto this PR since most of that's already here.

@SorYoshino
Copy link
Contributor

I have no problem.

Copy link

@coderabbitai coderabbitai bot left a 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 SilentlyContinue with explicit error handling.

The pre_install scripts continue to use -ErrorAction SilentlyContinue to silently ignore failures during file removal and renaming. This masks errors and violates Scoop best practices. Failures should abort the installation using explicit try { … } 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.txt for version detection, while autoupdate constructs the URL from SourceForge using $majorVersion$minorVersion concatenation. 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.zip will produce hwi_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

📥 Commits

Reviewing files that changed from the base of the PR and between aaa715a and a18c87c.

📒 Files selected for processing (1)
  • bucket/hwinfo.json (1 hunks)
🔇 Additional comments (4)
bucket/hwinfo.json (4)

31-40: Verify Scoop API function signatures for persist_data and unlink_persist_data.

The manifest uses persist_data @{ persist = "$setting_name" } $original_dir $persist_dir (line 37) and unlink_persist_data @{ persist = "$setting_name" } $dir (line 51). Confirm:

  1. These are valid Scoop built-in functions and the parameter syntax is correct.
  2. $original_dir is the correct variable name (should it be $dir during install?).
  3. 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 $architecture variable 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:

  1. Scoop provides $architecture in a format matching this pattern (e.g., "64bit", "32bit").
  2. The lookahead regex correctly captures the version digits.
  3. 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:

  1. 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.zip file.
  2. 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:

  1. The exact filenames of the architecture-specific binaries in the SourceForge archive
  2. Whether the pre_install scripts correctly rename files to the unified HWiNFO.exe entry expected by the manifest

Manually verify the archive structure and test the pre_install rename regexes with actual SourceForge binaries to ensure correct behavior across all architectures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: hwinfo stopped updating

2 participants