Skip to content
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

Merged
merged 3 commits into from
Aug 28, 2023

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Aug 25, 2023

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 branch changes 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.

Now that the oranda-generate-css crate will use a pre-existing
tailwindcss binary, we can modify the Nix flake to depend on the
tailwindcss package from nixpkgs. 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.

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.

hawkw added 3 commits August 25, 2023 09:58
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.
@ashleygwilliams
Copy link
Member

@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 cargo build but there's no way to differentiate cargo install's cargo build from a straight cargo build (as far as i know). our standalone binaries neither use nor download the tailwind binary- i am assuming that a flake is somehow running a cargo install?

@hawkw
Copy link
Contributor Author

hawkw commented Aug 25, 2023

@ashleygwilliams Nix builds everything from source. I'm not totally sure OTTOMH whether rustPlatform.buildRustPackage performs a cargo build or a cargo install, but I could check. In either case, it's necessary for the flake to build Oranda from source to distribute Oranda from NixOS.

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.

@ashleygwilliams
Copy link
Member

@hawkw thank you for elaborating, especially the last point. we've been talking internally about switching away from build.rs to a env var- though if Nix is also proving a development env for working on oranda- then it seems that tailwind would still be a dep (we'd ask users who are working on oranda locally to set the env var). is there a way to specify local dev dependences specifically? or would that just be a "dependency"

@shadows-withal
Copy link
Contributor

shadows-withal commented Aug 25, 2023

(we may also go back to having a package.json so that distro packagers have an easier time specifying a well-known dependency rather than tailwindcss, which may not be in every registry. in any case, we'll allow both to coexist)

@hawkw
Copy link
Contributor Author

hawkw commented Aug 25, 2023

is there a way to specify local dev dependences specifically? or would that just be a "dependency"

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 packages.oranda (and a default package which is the oranda package):

oranda/flake.nix

Lines 76 to 79 in 05039c3

packages = rec {
oranda = package;
default = oranda;
};

as well as a default dev shell which inherits the oranda package's dependencies:

oranda/flake.nix

Lines 81 to 109 in 05039c3

devShells = with pkgs; {
default = mkShell {
nativeBuildInputs = [
pkg-config
];
buildInputs =
[
(fenix.packages.${system}.complete.withComponents [
"cargo"
"clippy"
"rust-src"
"rustc"
"rustfmt"
])
]
++ darwinInputs;
# Allow rust-analyzer and other tools to see rust src
RUST_SRC_PATH = "${rustPlatform.rustLibSrc}";
# Fix missing OpenSSL
PKG_CONFIG_PATH = "${pkgs.openssl.dev}/lib/pkgconfig";
shellHook = ''
export NIX_LDFLAGS="${nixLdFlags}"
'';
};
};

the actual package and its dependencies are defined here:

oranda/flake.nix

Lines 33 to 68 in 05039c3

package = with pkgs;
rustPlatform.buildRustPackage {
pname = cargo_toml.package.name;
version = cargo_toml.package.version;
src = ./.;
cargoLock = {
lockFile = ./Cargo.lock;
};
# Don't run checks
doCheck = false;
# Package metadata
meta = with pkgs.lib; {
description = cargo_toml.package.description;
homepage = cargo_toml.package.repository;
license = with licenses; [ asl20 mit ];
};
nativeBuildInputs = [
pkgs.pkg-config
];
buildInputs = with pkgs; ([
bzip2
oniguruma
openssl
xz
zstd
]
++ darwinInputs);
RUSTONIG_SYSTEM_LIBONIG = true;
ZSTD_SYS_USE_PKG_CONFIG = true;
NIX_LDFLAGS = nixLdFlags;
};

Copy link
Contributor

@shadows-withal shadows-withal left a 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!

@shadows-withal shadows-withal merged commit 6d109c9 into axodotdev:main Aug 28, 2023
hawkw added a commit to tosc-rs/mnemos that referenced this pull request Jan 19, 2024
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.
hawkw added a commit to tosc-rs/mnemos that referenced this pull request Jan 19, 2024
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.
hawkw added a commit to tosc-rs/mnemos that referenced this pull request Jan 19, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants