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

[PkgConfig] Avoid creating dummy dependency #5741

Merged
merged 2 commits into from
Aug 26, 2022

Conversation

stevapple
Copy link
Contributor

Dummy dependency can be seen in some awkwardly generated .pc files and it can crash SwiftPM silently at least on Windows.

Motivation:

To enhance robustness and avoid silent crashes on Windows.

Modifications:

Ignore parsed dependency with an empty name from a .pc file.

Result:

SwiftPM will not crash when parsing dependency list like dep1, , dep2.

This can be seen in some awkwardly generated .pc files and
it can crash SwiftPM silently at least on Windows.
@tomerd
Copy link
Contributor

tomerd commented Aug 22, 2022

this seems fine to me. @neonichu ?

@neonichu
Copy link
Contributor

LGTM, but I'm curious about the "silent crash" that is mentioned. What exactly happens and is there more to fix around that?

@stevapple
Copy link
Contributor Author

LGTM, but I'm curious about the "silent crash" that is mentioned. What exactly happens and is there more to fix around that?

From my experience Swift programs are very likely to crash silently with edge cases on Windows, and there could be various reasons.

The simplest one is caused by lacking runtime library, and it’s easy to find because although CMD won’t emit anything, the GUI shell can tell you what is missing.

The others are really hard to debug because LLDB works not so well on Windows. I can remember of silent crashes related to stack overflow and invalid system calls.

When I first worked on Swift Driver test suite, the test always crashes at a specific function call and I can’t tell why🤷‍♂️

@stevapple
Copy link
Contributor Author

stevapple commented Aug 22, 2022

I suspect this one to be solved by swiftlang/swift-tools-support-core#175, which introduces a fix to avoid creating empty path on Windows since the current implementation of fileSystemRepresentation in swift-corelibs-foundation will lead to such crash when used on an empty String.

I’ve been pushing this PR for years, and just as curious why it still doesn’t get merged🤷‍♂️

@stevapple
Copy link
Contributor Author

@neonichu One thing I can think of to help with crashes is to make verbose mode even more verbose and configurable. cl supports a default /VERBOSE flag as well as a set of /VERBOSE:kind flags to provide amazing debugging experience, and it’s a possible direction (supporting --verbose=<kinds>) to evolve Swift Driver and SwiftPM.

@tomerd
Copy link
Contributor

tomerd commented Aug 25, 2022

@stevapple @neonichu what do we need to do to take this forward?

@neonichu
Copy link
Contributor

I think this change makes sense by itself.

@neonichu
Copy link
Contributor

@swift-ci please smoke test

@neonichu neonichu merged commit 6595cd2 into swiftlang:main Aug 26, 2022
@stevapple stevapple deleted the dummy-dependency branch August 26, 2022 07:32
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