Skip to content

Don’t reload package manifest when a header is added #7691

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

Closed
wants to merge 3 commits into from

Conversation

MaxDesiatov
Copy link
Contributor

Currently, any new header file causes a package reload. This means that while background indexing is writing header files into the index build directory, the package will get reloaded, which is unnecessary.

Currently, any new header file causes a package reload. This means that while background indexing is writing header files into the index build directory, the package will get reloaded, which is unnecessary.
@MaxDesiatov MaxDesiatov added bug performance Performance optimizations and improvements workspace Package workspace support labels Jun 20, 2024
@MaxDesiatov MaxDesiatov self-assigned this Jun 20, 2024
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) June 20, 2024 21:55
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

While this fixes the most common issue that I have seen during a build, there are a couple of others that this change would not cover. For example, SourceKit-LSP is operating on a .index-build build folder when background indexing is enabled. Now say I run the first build on the command line into .build, which will clone the dependencies and thus generate .swift files in .build. For all of these files, we’ll consider them as affecting the build settings for SourceKit-LSP because they match the .swift rule.

Is there any way in SwiftPM to figure out whether a file is in a Sources/Tests folder of an existing target or the build plugin equivalent of these? Such a check would fix the scenario described above.

But thanks for the fix. I think this is very valuable to take on its own because it should fix the majority of package manifest reloads.

@MaxDesiatov MaxDesiatov requested a review from ahoppen June 20, 2024 22:17
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test windows

@MaxDesiatov
Copy link
Contributor Author

Is there any way in SwiftPM to figure out whether a file is in a Sources/Tests folder of an existing target or the build plugin equivalent of these? Such a check would fix the scenario described above.

There's a complication as there can be files generated by plugins in the build directory, and we don't know which files until we run those plugins.

@ahoppen
Copy link
Member

ahoppen commented Jun 22, 2024

I just drafted an implementation that relies on directories in the build plan instead of file extensions, which also avoids the issue of .swift files created in locations that don’t affect the build, which I described above

#7699

auto-merge was automatically disabled June 24, 2024 15:32

Pull request was closed

@MaxDesiatov MaxDesiatov deleted the maxd/exclude-headers-from-reloading branch June 24, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug performance Performance optimizations and improvements workspace Package workspace support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants