-
-
Notifications
You must be signed in to change notification settings - Fork 414
fix: outdated and broken nix flake #846
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
Conversation
WalkthroughThe update modifies the Nix build configuration for the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
flake.nix
Outdated
| version = "1.3.0"; | ||
| src = ./.; | ||
| modules = ./gomod2nix.toml; | ||
| doCheck = false; |
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.
@coderabbitai What it this check ? What are the implications of disabling it ?
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.
@lazysegtree The doCheck parameter controls whether the check phase runs during the Nix build process. Here's what it does:
What is the check phase?
- For Go applications built with
buildGoApplication, the check phase typically runsgo testto execute the project's test suite - It's part of the standard Nix build pipeline: unpack → patch → configure → build → check → install → fixup
What doCheck = false does:
- Disables the test execution entirely during the build
- The build process skips from the build phase directly to the install phase
- Tests are not run as part of the package creation process
Implications of disabling it:
Pros:
- Faster builds - no time spent running tests
- Avoids environment-specific test failures (which is the case here)
- Still produces a working binary if the build phase succeeds
Cons:
- No test validation during packaging - potential for shipping untested code
- Less confidence in the package integrity
- May miss regressions that tests would catch
Context for this specific change:
According to the PR description, the tests were failing in the Nix build environment with errors like missing /homeless-shelter/.local/share/superfile/pinned.json, but these same tests pass when run outside of Nix. This indicates environment-specific issues rather than actual code problems.
Since the build itself succeeds and the resulting binary works correctly, disabling the check phase is a pragmatic solution to make the Nix package buildable while the underlying test environment issues could be addressed separately.
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.
@Frost-Phoenix I dont know much about nix. I will attempt to fix the tests. If that doesn't works, we will go with your changes to skip tests.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
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.
I dont know much about nix. I will attempt to fix the tests. If that doesn't works, we will go with your changes to skip tests.
I think I found a way to make the tests work, as they managed to make it work in the upstream nixpkgs repo NixOS/nixpkgs#410196
I don't have time today, but I will look into it tomorrow.
|
This fixes the checkPhase, so it is no longer needed to disable it. |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [yorukot/superfile](https://github.com/yorukot/superfile) | patch | `v1.3.1` -> `v1.3.3` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>yorukot/superfile (yorukot/superfile)</summary> ### [`v1.3.3`](https://github.com/yorukot/superfile/releases/tag/v1.3.3) [Compare Source](yorukot/superfile@v1.3.2...v1.3.3) Hey folks. Releasing v1.3.3 with bug fix related to config file, and improvements in metadata panel feature. #### Install: [**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation) #### Highlights - Fixed a bug in config file correction that would make config file invalid. Thanks [@​estebandpena](https://github.com/estebandpena) for reporting the issue, and [@​wassup05](https://github.com/wassup05) for the fix. #### Detailed Change Summary <details><summary>Details</summary> <p> ##### Update - feat: Metadata loading via bubbletea's tea.Cmd method, removed usage channels and custom goroutines [#​947](yorukot/superfile#947) by @​lazysegtree - feat: Metadata panel into separate package, UI bug fixes, Code improvements [#​950](yorukot/superfile#950) by @​lazysegtree - feat: add support for Python virtual environment in testsuite setup [#​956](yorukot/superfile#956) by @​Khitoshi ##### Bug fix - fix: windows test ci [#​941](yorukot/superfile#941) by @​claykom - fix: fixing `config.toml` [#​952](yorukot/superfile#952) by @​wassup05 </p> </details> **Full Changelog**: yorukot/superfile@v1.3.2...v1.3.3 ### [`v1.3.2`](https://github.com/yorukot/superfile/releases/tag/v1.3.2) [Compare Source](yorukot/superfile@v1.3.1...v1.3.2) Hey folks. Releasing v1.3.2 with multiple new features including implementation image preview using kitty protocol, and various bug fixes and internal improvements. ##### **You can now preview images in high quality — no more pixelation!** <img width="1267" height="670" alt="image" src="https://github.com/user-attachments/assets/0f7ec48e-9386-4716-92a5-4a02783f6612" /> #### Install: [**Click me to know how to install**](https://github.com/yorukot/superfile?tab=readme-ov-file#installation) #### Highlights - Added image preview via kitty protocol. Thanks @​JassonCordones and @​yorukot for the implementation. - Added Zoxide support for path resolution in initial configuration. Thanks @​yorukot for this. - New 0x96f Theme by @​filipjanevski - Various usability and layout bug fixes. See "Detailed Change Summary" below. Thanks to @​larsn03 @​Frost-Phoenix @​TheLazron @​vkhatsiur @​Khitoshi @​claykom ##### Developer Improvements - Added CI Action to run unit tests and build for windows. - Updated contribution guidelines and MR template - Added a dev.sh script for building, linting and running tests for faster development workflows. #### Detailed Change Summary <details><summary>Details</summary> <p> ##### Update - Normalize user-facing naming to superfile [#​880](yorukot/superfile#880) by @​yorukot - Add kitty protocol for image preview [#​841](yorukot/superfile#841) by @​JassonCordones - feat: add Zoxide support for path resolution in initial configuration [#​892](yorukot/superfile#892) by @​yorukot - feat: update superfile's help output [#​908](yorukot/superfile#908) by @​yorukot - feat: Add Action to Publish to Winget [#​925](yorukot/superfile#925) by @​claykom - feat: update superfile build test for the windows and macOS [#​922](yorukot/superfile#922) by @​yorukot - Theme: add 0x96f theme [#​860](yorukot/superfile#860) by @​filipjanevski ##### Bug fix - fix: outdated and broken nix flake [#​846](yorukot/superfile#846) by @​Frost-Phoenix - fix: handle UTF-8 BOM in file reader [#​865](yorukot/superfile#865) by @​vkhatsiur - fix icon displayed on spf prompt when nerdfont disabled [#​878](yorukot/superfile#878) by @​claykom - fix: create item check for dot-entries [#​817](yorukot/superfile#817) by @​TheLazron - fix: prevent pasting a directory into itself, avoiding infinite loop [#​887](yorukot/superfile#887) by @​yorukot - fix: clear search bar value on parent directory reset [#​906](yorukot/superfile#906) by @​yorukot - fix: enhance terminal pixel detection and response handling [#​904](yorukot/superfile#904) by @​yorukot - fix: Cannot Build superfile on Windows [#​921](yorukot/superfile#921) by @​claykom - fix: Improve command tokenization to handle quotes and escapes [#​931](yorukot/superfile#931) by @​Khitoshi - fix: Dont read special files, and prevent freeze [#​932](yorukot/superfile#932) by @​lazysegtree - Compress all files selected [#​821](yorukot/superfile#821) by @​larsn03 ##### Optimization - Metadata and filepanel rendering refactor [#​867](yorukot/superfile#867) by @​JassonCordones - refactor: simplify panel mode handling in file movement logic [#​907](yorukot/superfile#907) by @​yorukot - refactor: standardize TODO comments and ReadMe to README [#​913](yorukot/superfile#913) by @​yorukot ##### Documentation - enhance: add detailed documentation for InitIcon function and update … [#​879](yorukot/superfile#879) by @​yorukot - docs: add documentation for image preview [#​882](yorukot/superfile#882) by @​yorukot - docs: update contributing guide and MR template [#​885](yorukot/superfile#885) by @​yorukot - docs: update README and plugin documentation for clarity and structure [#​902](yorukot/superfile#902) by @​yorukot - feat(docs): Update arch install package docs [#​929](yorukot/superfile#929) by @​booth-w ##### CI/CD - ci: add MR title linting with semantic-pull-request action [#​884](yorukot/superfile#884) by @​yorukot - ci: improve MR workflows with contributor greeting and title linter fix [#​886](yorukot/superfile#886) by @​yorukot ##### Dependencies - build(deps): bump prismjs from 1.29.0 to 1.30.0 in /website [#​786](yorukot/superfile#786) by @​dependabot[bot] - fix(deps): update dependency astro to v5.8.0 [#​787](yorukot/superfile#787) by @​renovate[bot] - chore(deps): bump vite from 6.3.3 to 6.3.5 in /website [#​822](yorukot/superfile#822) by @​dependabot[bot] - fix(deps): update dependency sharp to v0.34.2 [#​909](yorukot/superfile#909) by @​renovate[bot] - fix(deps): update astro monorepo [#​894](yorukot/superfile#894) by @​renovate[bot] - fix(deps): update fontsource monorepo to v5.2.6 [#​910](yorukot/superfile#910) by @​renovate[bot] ##### Misc - chore(license): update copyright year [#​895](yorukot/superfile#895) by @​yorukot - feat: add ignore missing field flag [#​881](yorukot/superfile#881) by @​claykom - feat: add sitemap integration and update giscus input position [#​912](yorukot/superfile#912) by @​yorukot </p> </details> #### New Contributors * @​filipjanevski made their first contribution in yorukot/superfile#860 * @​larsn03 made their first contribution in yorukot/superfile#821 * @​vkhatsiur made their first contribution in yorukot/superfile#865 * @​claykom made their first contribution in yorukot/superfile#878 * @​TheLazron made their first contribution in yorukot/superfile#817 * @​Khitoshi made their first contribution in yorukot/superfile#931 **Full Changelog**: yorukot/superfile@v1.3.1...v1.3.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC42Mi4xIiwidXBkYXRlZEluVmVyIjoiNDAuNjIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
This PR fixes two different problem related to the nix flake
Outdated pkgs
The flake pkgs (
flake.lockfile) are really outdated, they haven't been updated in over a year. This causes a build failure.After updating the lock file the build process can start.
Check phase fail
Now the package start to build, finishes the build phase (compiling), but fail on the check phase (when it runs the go tests)
This problem seem to be caused by the absence of certain files that are expected to be present during the test phase. This is a nix only problem as the go test runs without any issues.
A simple fix is to simply disable the check phase, as the build in itself is successful.
Result
After these two changes the build is successful and the flake can be used by nix users to get the latest version of superfile.
Works as expected.
Test
To test you can run the current broken flake with
nix run github:yorukot/superfile, and the flake changed by this pr withnix run "github:frost-phoenix/superfile?ref=flake-fix"Summary by CodeRabbit