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

Run Clean in Windows SCsub to clean up debug files #85197

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

brno32
Copy link
Contributor

@brno32 brno32 commented Nov 21, 2023

Addresses only the Windows platform in #75570

The original issue mentions it's a problem with the "Clean Solution" button in Visual Studio, but the problem is with the call to scons --clean itself, which Visual Studio is using under the hood.

scons --clean only drops the .exe files from the bin folder, but it leaves everything else that's in there.

I couldn't find a way to get SCons to tell me the list of files in the output directory it made from the build, so deriving it from the program name seemed like the next best thing. I don't think we want to just clear the bin folder as that could delete files from targets other than the one being cleaned.

Bugsquad edit:

@YuriSizov
Copy link
Contributor

YuriSizov commented Nov 23, 2023

I don't think we want to just clear the bin folder as that could delete files from targets other than the one being cleaned.

This is correct, but I think the implementation with a wildcard still has this problem, because different build configurations would only differ by suffixes. So the wildcard mask can overlap.

I think we could use an explicit list of extensions instead.

@brno32
Copy link
Contributor Author

brno32 commented Nov 23, 2023

This is correct, but I think the implementation with a wildcard still has this problem, because different build configurations would only differ by suffixes. So the wildcard mask can overlap.

I think we could use an explicit list of extensions instead.

fair point. I've modified it to use a list. I'm still using a glob to catch things like bin\godot.windows.editor.dev.x86_64.console.pdb

@brno32
Copy link
Contributor Author

brno32 commented Nov 24, 2023

Well, on second thought that still won't work, as the glob could grab something it's not meant to.

I see the console program is a separate SCons Program so I've opted to instead perform this process for both programs

@brno32
Copy link
Contributor Author

brno32 commented Nov 30, 2023

@YuriSizov would you mind taking another look now that I've refactored the approach?

@YuriSizov
Copy link
Contributor

@YuriSizov would you mind taking another look now that I've refactored the approach?

The approach looks fine to me now. But I'm not fluent in SCons to review the implementation for correctness. So we'll need to wait a bit for someone else to take a look.

@YuriSizov YuriSizov requested a review from a team December 14, 2023 18:24
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I don't use Windows frequently so I haven't tested, but the rationale makes sense and the implementation seems ok.

@akien-mga akien-mga merged commit 5f6790a into godotengine:master Jan 15, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build system: cleaning Visual Studio solution does not erase .pdb, .ilk, .lib files
3 participants