Skip to content

Fix background indexing behavior if a source file is included in two targets via a symlink #1858

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

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Dec 6, 2024

Consider the following scenario: A project has target A containing A.swift an target B containing B.swift. B.swift is a symlink to A.swift. When A.swift is modified, both the dependencies of A and B need to be marked as having an out-of-date preparation status, not just A.

@ahoppen
Copy link
Member Author

ahoppen commented Dec 6, 2024

@swift-ci Please test

…targets via a symlink

Consider the following scenario: A project has target A containing A.swift an target B containing B.swift. B.swift is a symlink to A.swift. When A.swift is modified, both the dependencies of A and B need to be marked as having an out-of-date preparation status, not just A.
@ahoppen
Copy link
Member Author

ahoppen commented Dec 9, 2024

Updated the PR to dod the inference of source files with equivalent realpaths to Workspace.filesDidChange and also added a realpath cache in there.

} ?? []
}

func filesDidChange(_ events: [FileEvent]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you check what event ends up being sent for changing a symlink? Is it a delete + add or just a change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not. But it shouldn’t matter because we clear the realpath cache on any file event.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, I was wondering if we could limit it to eg. just delete/add rather than on change

Copy link
Member Author

Choose a reason for hiding this comment

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

Even if I checked how it behaves on macOS, I don’t want to make inferences about how it behaves on various Linux distributions, so I think erroring on the side of caution is better here. Plus, recomputing a single realpath when a file is changed shouldn’t be that bad.

@ahoppen
Copy link
Member Author

ahoppen commented Dec 10, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Dec 10, 2024

@swift-ci Please test Windows

@ahoppen ahoppen enabled auto-merge December 10, 2024 19:25
@ahoppen ahoppen merged commit 7a06eb0 into swiftlang:main Dec 10, 2024
3 checks passed
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.

3 participants