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

Cache Typst packages (2) #17

Closed
wants to merge 18 commits into from
Closed

Conversation

jcbhmr
Copy link
Member

@jcbhmr jcbhmr commented Jan 25, 2024

@yusancky this is a version that i got working

changes from #16 are:

  1. using https://bun.sh instead of vite. you're absolutely right that vite didn't seem to work. there were some spurious "require()"s where they should have been converted to imports. bun seems to handle things better. 🤷‍♀️

  2. mirrored the https://github.com/actions/setup-node/blob/d86ebcd40b3cb50b156bfa44dd277faf38282d12/src/cache-restore.ts#L15 for restoring the cache (but simpler) and https://github.com/actions/setup-node/blob/d86ebcd40b3cb50b156bfa44dd277faf38282d12/src/cache-save.ts#L34 for saving the cache in post (but simpler)

  3. switched to bun as package manager and primary dev tool (instead of node/npm) and bundler (instead of vite)

  4. using example from https://github.com/typst/packages/tree/main/packages/preview/droplet/0.2.0 as test.typ

  5. bun doesn't work on windows so the compilation step to create the bundle cannot happen on a windows runner. the regular using: node20 actual runtime can work on windows. its just theres currently no test-action.yml for windows-latest. it's easiest just to wait until Windows Support oven-sh/bun#43

  6. using cache: true instead of packages-id. dependant on the hashFiles() of the **/*.typ files in the current workspace so that theres a new cache every time the dependencies needed by the workspace change (since #import could be in any .typ file, not just a common lockfile like typst.lock or something).

  7. TODO: allow users to specify a cache-dependency-path input just like https://github.com/actions/setup-node does so that users can override the overly cautious **/*.typ glob with their own custom files/globs.

  8. TODO: change the readme to reflect ☝️ all those changes

this pr is currently targeting @yusancky 's original pr #16 and when merged will merge into that branch

@jcbhmr jcbhmr requested a review from yusancky January 25, 2024 18:38
@jcbhmr jcbhmr self-assigned this Jan 25, 2024
@jcbhmr jcbhmr mentioned this pull request Jan 25, 2024
@jcbhmr jcbhmr marked this pull request as ready for review January 25, 2024 18:50
@Enter-tainer
Copy link

Enter-tainer commented Jan 26, 2024

hashFiles() of the **/*.typ files in the current workspace so that theres a new cache every time the dependencies needed by the workspace change

This might not make sense because whenever your typst file changes, it will invalidate all cache.

In typst when you use a package, you always need specify the full version string and the package registry ensures versions are "append-only". So this is quite different from nodejs ecosys. I think typst's local package cache is: a) safe to clear, b) never invalidate. I haven't clear my package cache on my personal computer yet and Idk when I need do this.

I start to understand this. Maybe we can switch to hash files in the cache dir, rather than the workspace? IDK how github action works but this might not make sense either 💀

@jcbhmr
Copy link
Member Author

jcbhmr commented Jan 26, 2024

In typst when you use a package, you always need specify the full version string and the package registry ensures versions are "append-only". So this is quite different from nodejs ecosys. I think typst's local package cache is: a) safe to clear, b) never invalidate. I haven't clear my package cache on my personal computer yet and Idk when I need do this.

you're right. it is "append-only" where future changes don't impact the previous stuff. i think that could go in the RUNNER_TOOL_CACHE then instead of actions/cache? ill try it.

@yusancky yusancky deleted the branch package-cache-230124 February 8, 2024 00:52
@yusancky yusancky closed this Feb 8, 2024
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