Skip to content

fix: ask clangd to not write .pch/.include cache on disk #147

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

fix: ask clangd to not write .pch/.include cache on disk #147

wants to merge 1 commit into from

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Jan 5, 2023 •
edited
Loading

Please check if the PR fulfills these requirements

What kind of change does this PR introduce?
.pch files are not created on disk, but in-memory as specified with -pch-storage=memory command line flag to clangd
Should finally fix #145

What is the current behavior?
.pch files are created on temp directory and are not removed on exit.

What is the new behavior?
.pch files are no more created on temp directory.

@cmaglie cmaglie self-assigned this Jan 5, 2023
@cmaglie cmaglie requested a review from per1234 January 5, 2023 12:42
@cmaglie cmaglie added priority: high Resolution is a high priority type: imperfection Perceived defect in any part of project criticality: high Of high impact topic: code Related to content of the project itself labels Jan 5, 2023
per1234
per1234 reviewed Jan 18, 2023
View reviewed changes
Copy link
Contributor

@per1234 per1234 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think I might have done a poor job of communicating the situation in #145 (comment) and #146 (review)

Even though they were created during operation, I never observed any problems with .pch files being left behind in the temporary folder after exiting the IDE. I mentioned them in #145 only because I thought it might be significant that the .pch files were being cleaned up, but the other files were not. I mentioned them in #146 (review) because I saw that people had complained in the LLVM issue tracker at a time when a bug in clangd (since resolved) was causing the .pch files to not be cleaned up. I didn't find any equivalent complaints about the system-includes-<hash>.clangd files, which made me suspect the lack of cleanup of the system-includes-<hash>.clangd files was specific to the way Arduino Language Server/Arduino IDE was using clangd.

After #146, the remaining problem is many system-includes-<hash>.clangd files being left behind in the temporary folder. This pull request does not fix that.

Since the proposed change does not seem to have any benefits for resolving #145, I would not merge it for that purpose.

So the question now: Is there some other benefit to the proposed change?

@cmaglie
Copy link
Member Author

cmaglie commented Feb 7, 2023

I understand now, thanks.

So the question now: Is there some other benefit to the proposed change?

No, there is no more reason to keep this change.

@cmaglie cmaglie closed this Feb 7, 2023
@per1234 per1234 added the conclusion: declined Will not be worked on label Feb 7, 2023
@cmaglie cmaglie deleted the tmp_cleanup branch February 7, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@per1234 per1234 per1234 left review comments

Assignees

@cmaglie cmaglie

Labels
conclusion: declined Will not be worked on criticality: high Of high impact priority: high Resolution is a high priority topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Temporary files are not cleaned up
2 participants
@cmaglie @per1234