-
Notifications
You must be signed in to change notification settings - Fork 12
Add flake package installation #144
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: jamy/feature/dev-flake
Are you sure you want to change the base?
Conversation
|
@screwyprof pinging you to review too, since we discussed nix in tinty before. The next PR I will create adds home-manager integration lightly discussed in #83 |
| asset="${ASSETS[$system]}" | ||
| url="https://github.com/$REPO/releases/download/v$VERSION/$asset" | ||
| echo "Fetching hash for $asset..." | ||
| hash=$(nix-prefetch-url "$url" 2>/dev/null) |
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.
If nix-prefetch-url fails, you'll get an empty hash
Maybe better to fail explicitly or validate the hash isn't empty
If a release is missing an asset for a platform, the script will still try to update it could result in a broken package.nix
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.
Thanks, added a conditional below which throws if it's empty
|
|
||
| for system in "${!HASHES[@]}"; do | ||
| # Update hash for each system (matches the line after the system name) | ||
| sed -i "/$system/,/hash =/{s|hash = \".*\";|hash = \"${HASHES[$system]}\";|}" "$PACKAGE_FILE" |
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 might be wrong, but -/$system/,/hash =/{s|hash = \".*\";|...} assumes hash is always on the line after system. If file structure changes, this breaks or may accidentally replace wrong hashes
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.
It matches from system up to (and including) the hash property line (currently name property is before hash), then it does the replacement of hash using those lines as input, so as long as a single hash follows system, it will be ok.
screwyprof
left a comment
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.
LGTM, just a few minor comments
Hey @JamyGolden. The dev environment via A few things I've noticed. Docker doesn't use any caching for Rust, meaning that every run will download all Cargo deps every time. I don't know the intention behind using Docker in the project vs running tests on the host, but if it's used as part of development, running it every time would be pretty time-consuming. |
nix run github:tinted-theming/tinty/jamy/feature/nix-package -- --version
tinty 0.30.0Running a package works. It's it in main, the full branch path could be omitted, so |
e090b6f to
88b4e03
Compare
b6ed825 to
62d0d52
Compare
|
Added caching for the test_docker just command. It's there because a few of the tests check ~/.local/share/tinted-theming/tinty Thanks for the review @screwyprof 🙏 |
Depends on jamy/feature/dev-flake branch