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

Download and install tailwindcss binary without opam #2718

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cuihtlauac
Copy link
Collaborator

@cuihtlauac cuihtlauac commented Sep 26, 2024

  • Remove tailwindcss as a dependence in dune-project and ocamlorg.opam
  • Remove Opam package pin in dune-workspace and ocamlorg.opam.template
  • Add tools/tailwindcss/dune that
    • Download tailwindcss binary from tailwindlabs/tailwindcss
    • Installs it in local switch

Remark: files tools/tailwindcss/URL and tools/tailwindcss/RELEASE must not have end of line characters.

@cuihtlauac cuihtlauac marked this pull request as draft September 26, 2024 08:31
@cuihtlauac cuihtlauac marked this pull request as ready for review September 26, 2024 11:25
@tmattio
Copy link
Collaborator

tmattio commented Oct 1, 2024

Generally in favor, but can you expand on the benefits you see here?
Two comments:

  • I believe we now have an action in Dune to download things, so we can avoid using curl, which won't work on Windows
  • We should only download the binary that's relevant to the user's system, the PR currently downloads all of the binaries.

@cuihtlauac
Copy link
Collaborator Author

cuihtlauac commented Oct 1, 2024

Generally in favor, but can you expand on the benefits you see here?

One potential benefit is outside ocaml.org, not storing binaries in https://github.com/tmattio/opam-tailwindcss. @MisterDA shared his concern about it.

But mostly, this is proof-of-concept code, I needed it whilst developing branch bb to make it possible to downgrade to ocaml.4.14 and dune.3.9.

I believe we now have an action in Dune to download things, so we can avoid using curl, which won't work on Windows

I wasn't aware this existed. You're right; it's better.

We should only download the binary that's relevant to the user's system, the PR currently downloads all of the binaries.

Is this what happens? Rules with action (run curl are triggered by rules with action (copy, which have enabled_if stanzas made to only trigger a single download per system/hardware.

Update: Fixed

@MisterDA
Copy link
Contributor

MisterDA commented Oct 1, 2024

I believe we now have an action in Dune to download things, so we can avoid using curl, which won't work on Windows

There's a curl shipped by Microsoft since Windows 10.
https://curl.se/windows/microsoft.html

@cuihtlauac
Copy link
Collaborator Author

cuihtlauac commented Oct 1, 2024

BTW, has anybody ever tried to compile ocaml.org in Windows? This PR's dune stanzas do not allow to have a tailwindcss binary for Windows.

@samoht
Copy link
Member

samoht commented Oct 1, 2024

Could these Dune rules be packaged in the tailwindcss package? I know a few other projects that could re-use this (including mirage).

@cuihtlauac
Copy link
Collaborator Author

cuihtlauac commented Oct 1, 2024

There's already an unused dune file in https://github.com/tmattio/opam-tailwindcss. It could be patched to include those rules (or look like ones). The .opam file could call dune in install: (or elsewhere). But wouldn't it be a useless level of indirection? opam calling dune calling curl instead of dune calling curl. I'd rather vendor a project with a single directory such as tool/tailwindcss that only contains the dune file I need to download the binaries.

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.

4 participants