fennel: Add version 1.6.1#18016
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds bucket/fennel.json: a Scoop package manifest for Fennel v1.6.1. The JSON specifies metadata (version, description, homepage, MIT license), runtime/build dependencies (lua, gnupg), a PowerShell pre_install that downloads the tagged source and signature, verifies the tag via GPG and git, runs Lua bootstrap steps to precompile bootstrap/*.lua and assemble fennel.lua and fennel.cmd, declares installed binaries, and supplies checkver and autoupdate metadata. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with fennel
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bucket/fennel.json (1)
1-65: Please run the standard Scoop manifest validation flow before merge.Use this local checklist for this manifest (
fennel) before submission:scoop config debug true scoop config gh_token <your-github-token> # read-only token, optional for GitHub release API usage .\bin\checkver.ps1 -App fennel -f .\bin\formatjson.ps1 -App fennel scoop install bucket\fennel.json -a 64bitAlso keep these references handy for policy/format edge cases:
- https://github.com/ScoopInstaller/.github/blob/main/.github/CONTRIBUTING.md
- https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests
As per coding guidelines: “Provide clear instructions for testing the manifest locally before submission” and “always provide a link to the official contribution guide or wiki.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bucket/fennel.json` around lines 1 - 65, The manifest "fennel" (fennel.json) must be validated locally before merge: enable debug (scoop config debug true), optionally set GH token (scoop config gh_token), run the repo checks .\bin\checkver.ps1 -App fennel -f and formatting .\bin\formatjson.ps1 -App fennel, then test install with scoop install bucket\fennel.json -a 64bit; update the PR description to state these steps were run and include links to the contributor guide (https://github.com/ScoopInstaller/.github/blob/main/.github/CONTRIBUTING.md) and wiki (https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests) so reviewers can reproduce the validation.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bucket/fennel.json`:
- Around line 21-30: The gpg key fetch command invoked via "$gpg_exe --homedir
$gnupg_home --keyserver ... --recv-keys 9D13D942..." and the network git
operation "git fetch --depth 1 origin tag $version" must be wrapped with retry +
backoff logic: detect failure by checking $LASTEXITCODE after each attempt,
retry a configurable number of times (e.g., 3-5) with exponential backoff (sleep
increasing between attempts), and only throw the existing error ("GPG key fetch
failed", "signature verification failed", etc.) after all retries are exhausted;
apply the same pattern to the "Write-Output ... | & $gpg_exe
--import-ownertrust" step if it can fail transiently, and ensure the wrapper
preserves exit codes and logs each retry attempt for visibility.
---
Nitpick comments:
In `@bucket/fennel.json`:
- Around line 1-65: The manifest "fennel" (fennel.json) must be validated
locally before merge: enable debug (scoop config debug true), optionally set GH
token (scoop config gh_token), run the repo checks .\bin\checkver.ps1 -App
fennel -f and formatting .\bin\formatjson.ps1 -App fennel, then test install
with scoop install bucket\fennel.json -a 64bit; update the PR description to
state these steps were run and include links to the contributor guide
(https://github.com/ScoopInstaller/.github/blob/main/.github/CONTRIBUTING.md)
and wiki (https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests) so
reviewers can reproduce the validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9cb1db8b-4179-4db2-ab67-fa13e3f1fe88
📒 Files selected for processing (1)
bucket/fennel.json
|
I think SourceHut recently changed the HTML layout of their repository pages. There's only two fixes I need to make. Effective date for that will be today within the hour. |
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with fennel
|
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with fennel
|
12:33 AM UTC: I just used Gemini for that particular fix to save me time. I should of reviewed it. This is what it said (doing this for context regarding that commit): The Root Cause
The linter failed because I tried to be a little too
helpful in the previous step. In the Invoke-Retry function,
I included the Write-Host command to print
"Attempt X failed, retrying..." to the console.
Scoop has an extremely strict linter rule
(enforced by PSScriptAnalyzer) that completely
bans the use of Write-Host in manifest scripts
(PSAvoidUsingWriteHost). This is because
Write-Host bypasses standard output streams
and can mess up automated logging.
CodeRabbit's original suggestion actually
omitted Write-Host for exactly this reason, and I
inadvertently tripped the linter by adding it! |
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with fennel
|
|
Oh I simply forgot about .\bin\formatjson.ps1 -App appName.... |
|
/verify |
|
All changes look good. Wait for review from human collaborators. fennel
|
|
At this point I gotta do some other stuff, but it's probably an issue with the Push-Location and Pop-Location commands based off the log. I'm going to wait for manual review now as this is essentially 99.99% of the way there. Actually, let me just see if it will work. If not I'll have to move on to some other stuff for now. 🦆Edit: Reinitialized existing Git repository in C:/Users/andre/scoop-project/Extras/.git/
error: remote origin already exists.
fatal: couldn't find remote ref refs/tags/1.6.1From what I know, Scoop's pre_install scripts are executed automatically inside the application's root directory $dir. Push-Location "$dir", was an unnecessary stack operation. And because I was running the test command inside the local bucket repository, that Push-Location (or cd) combined with the git init command caused the script to accidentally run Git commands against the bucket instead of a temporary build folder.it was performing an unnecessary stack operation. Push-Location performs a push operation; or in other words, it pushes your cwd into a list in memory. Pop-Location takes the top path off, then sets it as your CWD again. But scoop already does this operation by default, via $dir. |
|
1 participant
fennel: Add version 1.6.1
Closes #18015
<manifest-name[@version]|chore>: <general summary of the pull request>Description
This PR adds Fennel (a Lisp that compiles to Lua) to the Extras bucket.
Because this manifest leaves the "fairly standard install" criteria behind in favor of a ustomized
pre_installblock, I have decided to provided the context regarding why this package is compiled from source and cryptographically verified the way it is.1. The Challenge: Upstream Deprecated Binaries
Fennel does not distribute pre-built Windows binaries. Following the discovery of signature verification failures on historical release artifacts, the upstream maintainer explicitly deprecated binary releases. The officially supported distribution method is now exclusively package managers or building from source from a verified Git tag.
2. Cryptographic Tag Verification (SLSA L2)
Because of the fact that dynamically generated source tarballs can shift their compression metadata (so, changing the hash even when the code is identical), this manifest should verify the maintainer's intent rather than just the transit hash (thus, mitigating for example: CWE-494, CWE-345).
.gnupgfolder.git verify-tag $versionto cryptographically PROVE authenticity before compiling.3. GPG / MSYS2 "Split-Brain" Mitigation
During testing,
git verify-tagfailed because Git for Windows uses a bundled MSYS2 GPG shim that cannot resolve Windows absolute paths for custom keyrings. In order to fix this untrusted search path issue (CWE-426, CWE-807), we explicitly do the following:gpg.exe.gpg-verify.cmdwrapper that hardcodes the--homedirto the isolated.gnupgfolder.4. AOT Bootstrapping via Lua
Because Windows lacks an adequate out-of-the-box for (also for security reason; see the ADR's)
make, thepre_installhook natively translates the upstream Makefile logic. It uses the host'sluaruntime to precompile Fennel's core macros and stitch together the finalfennel.luaexecutable.Additional Manifest Checks That Where Completed
exactly 4 spaces.
licenseproperty is a valid SPDX Identifier (MIT).architecturefield is included.binshims have been tested and work properly.NB for Reviewers: * I/O Handling: This manifest relies heavily on native
cmd.exestdout redirection (>>) rather than PowerShell'sOut-File. This is intentional; PowerShell silently injects BOMs and corrupts the generated Lua bytecode during the compilation phase./verifycomment immediately after opening this PR to trigger the automatic manifest verifier.0002-adr.md
0006-adr.md
0007-apr.md
0008-adr.md
The ADR's can also be found here.