Implement watch mode for development#1548
Merged
Merged
Conversation
Docker’s tmpfs mounts are owned by root by default. When a mountpoint doesn’t already exist, it’s created with `a+rwx` permissions by default, but those permissions become `ug+rwx` when the container is restarted for some reason (moby/moby#20437). This change creates the mountpoints in advance with the usual permissions. Tmpfs mounts having open permissions doesn’t have any security implications at all (as far as I know…) as long as the permissions of the path leading to them are correctly restrictive, but we don’t use user-based permissions inside a container as a security boundary in any case. Container restarts are notably useful for Docker Compose’s new watch mode.
instead of adding the mountpoints to the image. The permissions are also more restrictive this way, not that it really matters for dev containers.
> Compose can now delegate builds to bake for better performance. > To do so, set `COMPOSE_BAKE=true`.
Deno makes some nice improvements over Node, especially on what I’d consider the single most important issue in software: permissions! We can’t apply effective permissions yet because of the current sassc and esbuild setup, but it’s a start. Also, since Deno makes it convenient enough, I’ll use TypeScript, even though I’m not a fan of its type system. Considering its permission model, Deno’s language server is probably safe to use, too. The Web Crypto API doesn’t support streaming hash calculations (which it really should by now…) and Deno doesn’t expose `O_TMPFILE` temporary files, so copying static files has become a little more awkward; however, the new approach does have the advantages of being better suited to copy-on-write (in theory – it looks like Deno uses Rust’s `std::fs::copy`, which doesn’t seem to support `ioctl(FICLONE)` yet) and never producing partially-written output files (which is a step on the path to skipping more work for output files that are already fresh, completing the ad hoc reimplementation of a build system). These improvements will soften the blow if the initial motivation for switching (functioning inotify-based recursive watch) doesn’t work out for whatever reason. A v5 lockfile was necessary because of denoland/deno#28161; it’s stable in the version of Deno used by the Docker setup, but `--unstable-lockfile-v5` is included in deno.json for now for convenience. This also upgrades the asset builder’s dependency tree to the latest versions of all packages.
I started out with a solution based on merging a dynamic set of async iterables to implement working recursive watch in Node, but eventually, switching to Deno just seemed nicer. There are some leftovers from that direction in this implementation. All of the libraries (Node’s `fs`, chokidar on npm, Deno’s `Deno.watchFs`) for watching seem… bad, which is understandable for a complicated cross-platform problem (the filesystem itself is a bad interface!), but I really just wanted guaranteed inotify with a minimum of layers of confusion.
- less verbose output when watching - shorter debounce (this delay is added to Docker Compose’s, which I don’t think is configurable) - better error formatting, especially for Sass
(original implementation predates Deno)
using some new features (new relative to the original Dockerfile, anyway) - Use `COPY --link` to improve cacheability - Avoid populating unnecessary layers, making use of `from=image` bind mounts - Remove unnecessary packages: we use prebuilt wheels now, so we don’t need headers and associated build tools, and lxml’s links libxslt (and libxml2) statically This is also the package set that’s used by the ci-base-image introduced in an ancestor commit and includes Deno, which is why it’s unfortunately mixed in with the otherwise unrelated assets-watch feature.
It looks like using `--checksum` stops the builder from rechecking the URL on every build, which was the previous problem with `ADD`. `--link` makes the layer cacheable even when previous instructions change (I would hope Docker would cache the file itself with `--checksum` alone), and in theory this is more parallelizable too.
See `libweasyl.models.media.MediaItem.display_url`. This is a symlink in production.
Runs faster; is cleaner; saves 36 MB in `docker image ls`’s “SIZE” field’s accounting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.