tla-toolbox@1.7.4: Use stable version, internal java & persist config#16803
tla-toolbox@1.7.4: Use stable version, internal java & persist config#16803goyalyashpal wants to merge 5 commits into
Conversation
WalkthroughAdded a new Scoop manifest Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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. tla-toolbox
|
|
/verify Some notes:
|
ecb47ac to
d61a0cf
Compare
|
/verify |
|
All changes look good. Wait for review from human collaborators. tla-toolbox
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/tla-toolbox.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16378
File: bucket/compactgui.json:20-22
Timestamp: 2025-10-19T13:58:23.389Z
Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.
Learnt from: Gitoffthelawn
Repo: ScoopInstaller/Extras PR: 16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds in recent versions uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format. This was confirmed by a working 10.0.0 update with valid hash.
Learnt from: Gitoffthelawn
Repo: ScoopInstaller/Extras PR: 16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format.
Learnt from: haussmann
Repo: ScoopInstaller/Extras PR: 16863
File: bucket/browseros.json:15-17
Timestamp: 2025-12-21T13:49:44.001Z
Learning: In Scoop manifests, when a URL uses a fragment like `#/dl.7z`, Scoop automatically extracts the archive after download. For nested archives (like BrowserOS), the downloaded installer may contain another archive (e.g., `chrome.7z`) that requires explicit extraction via the installer script using `Expand-7zipArchive`. The installer script should reference the inner archive name, not the outer `dl.7z`.
Problem: Scoop created .cmd file uses `java`, mayn't run at all. Solution: Create custom runner file using the bundled java bin.
d61a0cf to
c268ce5
Compare
Problem: Tools usage was unclear due to constricted aliasing
Solution: Align aliases according to original usage intent as do-
cumented upstream in following file:
https://github.com/tlaplus/tlaplus/blob/c8a59811fc/README.md#L16-L30
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
bucket/tla-toolbox.json (1)
69-72: This issue was already flagged in previous reviews.The autoupdate hash regex
$sha1\|$basenameuses invalid syntax. As noted in previous reviews, either remove the hash block or fix the regex to properly capture the 40-character SHA1 hash (e.g.,([0-9a-f]{40})\|$basename).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/tla-toolbox.json
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16378
File: bucket/compactgui.json:20-22
Timestamp: 2025-10-19T13:58:23.389Z
Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.
Learnt from: Gitoffthelawn
Repo: ScoopInstaller/Extras PR: 16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds in recent versions uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format. This was confirmed by a working 10.0.0 update with valid hash.
Learnt from: haussmann
Repo: ScoopInstaller/Extras PR: 16863
File: bucket/browseros.json:15-17
Timestamp: 2025-12-21T13:49:44.001Z
Learning: In Scoop manifests, when a URL uses a fragment like `#/dl.7z`, Scoop automatically extracts the archive after download. For nested archives (like BrowserOS), the downloaded installer may contain another archive (e.g., `chrome.7z`) that requires explicit extraction via the installer script using `Expand-7zipArchive`. The installer script should reference the inner archive name, not the outer `dl.7z`.
Learnt from: Gitoffthelawn
Repo: ScoopInstaller/Extras PR: 16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format.
Learnt from: mokosiy
Repo: ScoopInstaller/Extras PR: 16428
File: bucket/multidrive.json:8-8
Timestamp: 2025-10-27T13:19:16.982Z
Learning: For MultiDrive manifests in the ScoopInstaller/Extras repository: the direct download URL at dl.atola.com causes 403 errors due to Cloudflare Bot Fight Mode blocking the Scoop bot, so GitHub Releases URLs should be used instead (e.g., https://github.com/atola-technology/multidrive/releases/download/...).
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16349
File: bucket/fvim.json:45-49
Timestamp: 2025-10-16T15:59:21.258Z
Learning: In Scoop manifests, the `autoupdate.url` and `autoupdate.architecture.<arch>.url` fields must be valid URIs according to the JSON schema (defined with `"format": "uri"`). Variables like `$matchUrlx64` that contain only path segments must be combined with a base URL (e.g., `https://github.com/.../releases/download/`) to form a complete valid URI.
📚 Learning: 2025-10-19T13:58:23.389Z
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16378
File: bucket/compactgui.json:20-22
Timestamp: 2025-10-19T13:58:23.389Z
Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.
Applied to files:
bucket/tla-toolbox.json
📚 Learning: 2025-09-05T09:41:52.653Z
Learnt from: Gitoffthelawn
Repo: ScoopInstaller/Extras PR: 16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds in recent versions uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format. This was confirmed by a working 10.0.0 update with valid hash.
Applied to files:
bucket/tla-toolbox.json
📚 Learning: 2025-10-16T15:59:21.258Z
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16349
File: bucket/fvim.json:45-49
Timestamp: 2025-10-16T15:59:21.258Z
Learning: In Scoop manifests, the `autoupdate.url` and `autoupdate.architecture.<arch>.url` fields must be valid URIs according to the JSON schema (defined with `"format": "uri"`). Variables like `$matchUrlx64` that contain only path segments must be combined with a base URL (e.g., `https://github.com/.../releases/download/`) to form a complete valid URI.
Applied to files:
bucket/tla-toolbox.json
📚 Learning: 2025-10-13T09:37:06.093Z
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 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/tla-toolbox.json
📚 Learning: 2025-09-05T09:41:52.653Z
Learnt from: Gitoffthelawn
Repo: ScoopInstaller/Extras PR: 16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format.
Applied to files:
bucket/tla-toolbox.json
📚 Learning: 2025-12-21T13:49:44.001Z
Learnt from: haussmann
Repo: ScoopInstaller/Extras PR: 16863
File: bucket/browseros.json:15-17
Timestamp: 2025-12-21T13:49:44.001Z
Learning: In Scoop manifests, when a URL uses a fragment like `#/dl.7z`, Scoop automatically extracts the archive after download. For nested archives (like BrowserOS), the downloaded installer may contain another archive (e.g., `chrome.7z`) that requires explicit extraction via the installer script using `Expand-7zipArchive`. The installer script should reference the inner archive name, not the outer `dl.7z`.
Applied to files:
bucket/tla-toolbox.json
🔇 Additional comments (4)
bucket/tla-toolbox.json (4)
33-56: LGTM: Bin entries are well-structured.The bin configuration correctly defines:
- Direct executables (
toolbox.exe,tla2tools.cmd)- Aliases with class module arguments that align with the modules verified in the pre_install script (lines 27-31)
The structure follows Scoop conventions properly.
63-63: LGTM: Configuration persistence aligns with PR objectives.The persist field correctly ensures user settings survive updates and reinstalls, which was a key objective from issue #16802.
67-68: LGTM: Autoupdate URL structure is correct.The autoupdate URL template properly uses the
$versionvariable and matches the structure of the main download URL.
74-77: LGTM: Helpful documentation references for maintainers.The comment section provides useful links to PowerShell quoting rules and upstream documentation, which aids future maintenance.
| " \"@pushd `\"$dir`\"\"", | ||
| " '@set TOOLS_JAR=.\\tla2tools.jar'", | ||
| " \"@set THISJAVA=`\"${JDKDIR}\\jre\\bin\\java`\"\"", | ||
| " '@%THISJAVA% -cp %TOOLS_JAR% %*'", | ||
| " '@popd'", |
There was a problem hiding this comment.
Consider avoiding directory changes in the wrapper script to improve usability.
The @pushd/@popd pattern changes the working directory to the install directory, which breaks relative paths from the user's perspective. This is why line 6 notes that users must "specify full path for the spec inputfile when running tlc".
A better design would use absolute paths for the Java executable and JAR file without changing directories, allowing users to work with relative paths naturally.
🔎 Alternative implementation without directory changes
"@(",
-" \"@pushd `\"$dir`\"\"",
-" '@set TOOLS_JAR=.\\tla2tools.jar'",
+" \"@set TOOLS_JAR=`\"$dir\\tla2tools.jar`\"\"",
" \"@set THISJAVA=`\"${JDKDIR}\\jre\\bin\\java`\"\"",
" '@%THISJAVA% -cp %TOOLS_JAR% %*'",
-" '@popd'",
") | Set-Content \"$dir\\tla2tools.cmd\" -Encoding ASCII",Note: This still depends on fixing $JDKDIR (lines 17-18), but once that's corrected to produce an absolute path like $dir\plugins\jdk.openjdk..., this approach would:
- Use absolute paths for both
THISJAVAandTOOLS_JAR - Preserve the user's working directory
- Allow users to work with relative paths for their spec files
- Make the note on line 6 unnecessary
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In bucket/tla-toolbox.json around lines 20 to 24, the wrapper script uses
@pushd/@popd which changes the process working directory and forces users to
supply absolute paths; remove the directory change and instead compute absolute
paths for THISJAVA and TOOLS_JAR from the installer $dir (ensure $JDKDIR is
resolved to an absolute path under $dir\plugins\...), then invoke the Java
executable with an absolute path and -cp to the absolute JAR while leaving the
current working directory untouched so users can supply relative spec paths;
also remove or update the note about requiring full paths.
Fix #16802
tla{plus ->}-toolbox.jsonpre_installscript to run thetla2tools.jarwith the bundled-in javaAlso see:
Please do not squash.
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit
New Features
Improvements
Notes
✏️ Tip: You can customize this high-level summary in your review settings.