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

Keep looking inside project #29

Open
qm3ster opened this issue Sep 25, 2020 · 5 comments
Open

Keep looking inside project #29

qm3ster opened this issue Sep 25, 2020 · 5 comments

Comments

@qm3ster
Copy link

qm3ster commented Sep 25, 2020

For example, the target dir at <rust_project>/examples/<example>/target is ignored, only <rust_project>/target is removed.
And if all examples were built for test purposes, the sum of those targets can often exceed the crate's main target

@tbillington
Copy link
Owner

tbillington commented Sep 26, 2020

I run into that even on this project :) the kondo-ui subdirectory is not part of the workspace so it has it's own target directory, which is skipped because it's inside a project.

The code causing this is here

kondo/kondo-lib/src/lib.rs

Lines 224 to 225 in 7a549b6

if let Some(project_type) = p_type {
self.it.skip_current_dir();
Essentially, stop recursing if a project is found. This is helpful in nodejs projects because node_modules has projects within, and it's not useful to work with those. It also gave a massive speed boost because of how much work it could skip.

Perhaps this decision should be on a project type basis.

@qm3ster
Copy link
Author

qm3ster commented Sep 26, 2020

Or a project type basis. With node_modules you either want to delete the whole tree, including transient dependencies, or you want to keep all of it.
But with Rust target, the other targets are not under the top target. In fact both specifically examples and any other workspace/non-workspace nested projects are whole projects, precisely because they are source projects and not just published dependencies.
So, should it ask for each nested project, or just once for the topmost project and apply that ruling to all nested projects? 🤔

Also sometimes you have node_modules in Rust projects and Rust target in npm module projects...

@tbillington
Copy link
Owner

That's a good point, I hadn't considered directories that are more than one type of project, eg cargo and node project. Hmm... I'll have to think on that, also how to recurse directories that are not artifacts of the current project, the implementation now might make that a little tricky.

@qm3ster
Copy link
Author

qm3ster commented Sep 29, 2020

Still, at least looking at just Rust and nodejs, it seems like differentiating between:

  • artifact directories (delete, never enter)
  • literally everything else (don't delete, recurse)

should be sufficient?

So:

  1. get directory listing (actually collect to memory), while looking for marker files that signify a project. You can actually collect into two Vecs by DirEntry::file_type() which unlike metadata() is said not to involve additional syscalls.
  2. look through directory listing, looking for marker files and immediately mark the corresponding artifacts for "deletion".
  3. recurse on directories that have not been marked.

@tbillington
Copy link
Owner

That makes sense. I'll think through it. I think this might also be a good opportunity to add integration tests for the project, and model these situations.

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

No branches or pull requests

2 participants