-
Notifications
You must be signed in to change notification settings - Fork 31
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
Fix the tailwind dependency breaking the Nix flake #609
Conversation
Currently the `oranda-generate-css` crate will always attempt to download a precompiled `tailwindcss` binary from GitHub Releases, and store it in the user's cache directory. This doesn't work on platforms such as NixOS, where the global `.cache` directory is not writable while building a package. Thus, the Nix flake in the Oranda repo doesn't actually work at all. This commit changes the `oranda-generate-css` to first check if a binary named `tailwindcss` is on the current `PATH`, and uses that rather than downloading the precompiled binary if Tailwind is already available. We check for the existence of `tailwindcss` by running `tailwindcss --help` and testing if the exit code is 0, which seemed like the most cross-platform way to check if the binary exists. Potentially, we may want to change this to check for a specific `tailwindcss` version in the future, but I noticed that the current code downloads the latest release, rather than a fixed version, so I wanted the same behavior here. This will allow us to modify the Nix flake to depend on `tailwindcss` from nixpkgs, so that the download is skipped. This should fix the flake build. I noticed that the tailwindlabs/tailwindcss repo also contains a [Rust implementation of the Tailwind CLI][2]. We may, eventually, want to consider changing the approach used here to simply depending on the Rust tailwind implementation as a library dependency, so that it can be resolved by Cargo instead of downloaded from GitHub. However, that's a larger change, and I wanted to do the simplest change that would fix the Nix flake. Additionally, it might be desirable to add some kind of CI step that ensures the flake builds successfully, if the maintainers want to officially support it. That way, we could avoid breaking the flake in the future. [1]: https://search.nixos.org/packages?channel=unstable&show=tailwindcss&from=0&size=50&sort=relevance&type=packages&query=tailwindcss [2]: https://github.com/tailwindlabs/tailwindcss/blob/master/oxide/crates/cli/src/main.rs
Follow up from 1e66acc. This commit changes the Nix flake to depend on `tailwindcss` from nixpkgs instead of downloading a pre-built binary from GitHub. Now that the build script will run a pre-existing `tailwindcss` binary if one exists, this avoids the need to write to the cache directory, which is not writable inside of a Nix flake, as well as the precompiled binary, which may not work on NixOS at all.
@hawkw quick q about how flakes work- is it building from source or is it able to point at a binary? the tailwind binary is really only necessary for local development. we "trigger" the download on a |
@ashleygwilliams Nix builds everything from source. I'm not totally sure OTTOMH whether In addition, the flake also provides a dev shell for local development, so it's not just for installing Oranda; it also provides an environment for Nix users to develop Oranda. |
@hawkw thank you for elaborating, especially the last point. we've been talking internally about switching away from |
(we may also go back to having a |
yes, flakes have a separate concept of "package(s)" and "dev shell(s)"; the "package" builds the actual packaged application, while a dev shell is typically the package's dependencies plus additional tools that are useful for development but not necessarily required to build the final artifact. for example, the Oranda flake defines Lines 76 to 79 in 05039c3
as well as a default dev shell which inherits the oranda package's dependencies: Lines 81 to 109 in 05039c3
the actual package and its dependencies are defined here: Lines 33 to 68 in 05039c3
|
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.
This looks good to me, I'm hoping to push a change this week that'll remove the implicit Tailwind dependency (keeping it as an option behind a flag, basically), but this shouldn't break this, and would at most require a small change to include said flag. Thanks for taking the effort to do this!
This commit updates Oranda from v0.3.1 to v0.6.1. I don't believe any of the new Oranda features are particularly relevant to us, but there are some nice bugfixes, including axodotdev/oranda#654, which fixes unreadable CSS colors for mdBook draft chapters in the Oranda CSS themes which we use. Also, since axodotdev/oranda#609 has merged upstream, we can change the Nix flake to depend on upstream Oranda rather than my fork.
This commit updates Oranda from v0.3.1 to v0.6.1. I don't believe any of the new Oranda features are particularly relevant to us, but there are some nice bugfixes, including axodotdev/oranda#654, which fixes unreadable CSS colors for mdBook draft chapters in the Oranda CSS themes which we use. Also, since axodotdev/oranda#609 has merged upstream, we can change the Nix flake to depend on upstream Oranda rather than my fork.
This commit updates Oranda from v0.3.1 to v0.6.1. I don't believe any of the new Oranda features are particularly relevant to us, but there are some nice bugfixes, including axodotdev/oranda#654, which fixes unreadable CSS colors for mdBook draft chapters in the Oranda CSS themes which we use. Also, since axodotdev/oranda#609 has merged upstream, we can change the Nix flake to depend on upstream Oranda rather than my fork.
Currently the
oranda-generate-css
crate will always attempt todownload a precompiled
tailwindcss
binary from GitHub Releases, andstore it in the user's cache directory. This doesn't work on platforms
such as NixOS, where the global
.cache
directory is not writable whilebuilding a package. Thus, the Nix flake in the Oranda repo doesn't
actually work at all.
This branch changes
oranda-generate-css
to first check if a binarynamed
tailwindcss
is on the currentPATH
, and uses that rather thandownloading the precompiled binary if Tailwind is already available. We
check for the existence of
tailwindcss
by runningtailwindcss --help
and testing if the exit code is 0, which seemed like the most
cross-platform way to check if the binary exists. Potentially, we may
want to change this to check for a specific
tailwindcss
version in thefuture, but I noticed that the current code downloads the latest
release, rather than a fixed version, so I wanted the same behavior
here.
Now that the
oranda-generate-css
crate will use a pre-existingtailwindcss
binary, we can modify the Nix flake to depend on thetailwindcss
package from nixpkgs. This avoids the need to writeto the cache directory, which is not writable inside of a Nix flake, as
well as the precompiled binary, which may not work on NixOS at all.
I noticed that the tailwindlabs/tailwindcss repo also contains a Rust
implementation of the Tailwind CLI. We may, eventually, want to
consider changing the approach used here to simply depending on the Rust
tailwind implementation as a library dependency, so that it can be
resolved by Cargo instead of downloaded from GitHub. However, that's a
larger change, and I wanted to do the simplest change that would fix the
Nix flake. Additionally, it might be desirable to add some kind of CI
step that ensures the flake builds successfully, if the maintainers want
to officially support it. That way, we could avoid breaking the flake in
the future.