Skip to content

keep .git when cleaning dist #980

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

keep .git when cleaning dist #980

wants to merge 1 commit into from

Conversation

lucdar
Copy link

@lucdar lucdar commented Apr 19, 2025

When cleaning the dist directory before moving build artifacts from the staging area, if a .git file exists, it should not be removed. This allows Trunk to build into a git worktree, for example.

This behavior also exists in Vite which was introduced in this PR. Part of their justification is that a .git file could not be generated by Vite, but I'm not sure if this invariant holds for Trunk.

Copy link
Collaborator

@ctron ctron left a comment

Choose a reason for hiding this comment

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

Sounds reasonable and looks good to me.

Might be cool to have a list instead of "or".

@lucdar
Copy link
Author

lucdar commented Apr 22, 2025

Something like this?

if [STAGE_DIR, ".git"].iter().any(|s| entry.file_name() == *s) {
    continue;
}

I also tried using contains but converting the OsString to a &&str is a little cumbersome:

if [STAGE_DIR, ".git"].contains(&&*entry.file_name().to_string_lossy()) {
    continue;
}

I feel like the "or" is a little more readable than either of these, but there might be a better way that what I have here.

@ctron
Copy link
Collaborator

ctron commented Apr 24, 2025

I though about adding a const list, and the using that list. So that, later on, that can easily be extended.

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.

2 participants