Bring PkgConfig parsing to greater parity with pkg-config

[aemino]

This PR includes two main changes:

• Parses Required.private dependencies in pc files and adds their cFlags to the main PkgConfig instance. The pkg-config command line tool seems to add only private cFlags (not libs) when using the --cflags flag.
• Adds the pc_sysrootdir global variable to PkgConfigParser. This is simply a global variable that pkg-config includes but the parser does not.

[aciidgh]

Thanks for the PR. Do you have use cases where this is required for Swift packages?

[aemino]

@aciidb0mb3r Yeah, the reason that I made this PR was because I was trying to develop a systemLibrary wrapper package, but I couldn't get it to compile because it was missing header search paths. Eventually I realized that pkg-config was showing these header search paths with --cflags, but SPM was not using these, and the reason that this was occurring was because pkg-config includes private cflags when using --cflags, whereas SPM does not when aggregating these flags.

Also, I added the pc_sysrootdir variable because this same package used this variable, and it was not already implemented.

[aemino]

Hi @aciidb0mb3r, just a friendly request to review this PR. Thanks! 😄

[aciidgh]

Sorry, this slipped my mind. You're always free to ping me after a few days of inactivity!

This PR looks good! Can you add a unit test? Look at PkgConfigTests.swift for existing pkgconfig parser tests.

[aemino]

@aciidb0mb3r I added two very simple tests to PkgConfigParserTests. Let me know if more in-depth tests are needed or if it's sufficient as is.

[aciidgh]

Looks great! Thanks!

[aciidgh]

@swift-ci smoke test

[aemino]

It seems there was a test failure because some of the tests require every pkg-config variable to be accounted for. Is the best solution here to add that variable to all of the checks?

[aemino]

@aciidb0mb3r Friendly ping :)

[aciidgh]

I think so? Unless you think that won't be valuable.

[aemino]

Well, I don't think that it will be very valuable as a test. The implementation I added for the one instance of variable checking is already rather useless; it just replicates the functionality of the code it is testing. That said, it would be out of the scope of this PR to refactor the code such that it doesn't need to check for the environment variable that's being added.

I'm not even certain what the PKG_CONFIG_PATH env variable would be... If I just assume that it isn't set, then I can add "pc_sysrootdir": "/" to every parser.variables assertion.

[aciidgh]

Is using PKG_CONFIG_PATH even right? That is supposed to contain a colon separated list of paths, how will that work as a sysroot?

[aemino]

Ah! I have no idea how that slipped past me. It should be PKG_CONFIG_SYSROOT_DIR. I have made that change locally, but I'm still not sure what to do about the tests.

[aemino]

@aciidb0mb3r Ignoring the gaffe with PKG_CONFIG_SYSROOT_DIR (will fix that in the next commit), I'm not sure what to do about the tests... should the parser.variables assertion always expect pc_sysrootdir to be "/"?

[aciidgh]

We can create an extension in the test code that has a computed property of all the variables without the pc_sysrootdir key and we assert that in the tests?

[aemino]

I've attempted to fix the pkgconfig tests and (rather messily) resolved the conflicts created by the TSC restructure.

[aciidgh]

@swift-ci smoke test

[aemino]

@aciidb0mb3r The smoke tests seem to have completed.