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

clean before building #1568

Merged
merged 3 commits into from
Aug 26, 2024
Merged

clean before building #1568

merged 3 commits into from
Aug 26, 2024

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Aug 9, 2024

Fixes #1538. Now observable build is always clean.

Leaving as draft because I think we want a --clean flag to clean automatically, and without it we prompt you before cleaning the existing output root (if it exists) (and error in CI if the output root is dirty and --clean or --no-clean wasn’t specified).

And maybe --clean isn’t the right name because a “clean” build will still respect the cache so it’s not fully clean? Sigh.

@mbostock mbostock requested a review from mcglincy August 9, 2024 16:36
@mcglincy
Copy link
Contributor

mcglincy commented Aug 12, 2024

Code changes LGTM.

If "clean" seems too inaccurate, maybe something like "clean-dist" (and then later "clean-cache", "clean-all"?) could be used.

Copy link
Contributor

@mcglincy mcglincy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mbostock
Copy link
Member Author

Thinking about this some more, and I think cleaning the output directory is the right default, and probably doesn’t even need a flag. For precedence, Vite’s build command has an emptyOutDir option that likewise defaults to true, but only if the output directory is within the project root (so as to avoid accidentally deleting something important, such as /). I’ll add that to the PR and commit.

@mbostock mbostock marked this pull request as ready for review August 26, 2024 01:55
@mbostock mbostock enabled auto-merge (squash) August 26, 2024 06:18
@mbostock mbostock merged commit 8360797 into main Aug 26, 2024
4 checks passed
@mbostock mbostock deleted the mbostock/clean-rebuild branch August 26, 2024 06:20
@srenatus
Copy link

srenatus commented Sep 4, 2024

Sorry for posting to an old issue, but it seems like the best spot:

Thinking about this some more, and I think cleaning the output directory is the right default, and probably doesn’t even need a flag.

I've come here looking for a flag 😅 The problem is this: I'm using framework in a concourse pipeline, and the output directory is declared as the task's output, so that only that data is moved to the next task. Concourse seems to do that by mounting something into the output location, and that something cannot be removed:

> observable
> observable "build"

(node:2607) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Warning: the config file is missing the root option, which specifies the path to
the source root. The recommended source root is "src"; however, since docs
exists and was previously the default for this option, we will use "docs". You
can suppress this warning by specifying root: "docs" in the config file.

Unexpected error: EBUSY: resource busy or locked, rmdir 'dist'

Tip: To see the full stack trace, run with the --debug flag.


If you think this is a bug, please file an issue at
↳ https://github.com/observablehq/framework/issues

I think I can just use another directory within the output directory, so it's probably not a big deal.

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.

The automatic re-build on deploy should be clean
3 participants