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 System Module Search Paths #257

Merged
merged 4 commits into from
Apr 18, 2016
Merged

Conversation

aciidgh
Copy link
Contributor

@aciidgh aciidgh commented Apr 16, 2016

@aciidgh aciidgh changed the title [WIP] pkgconfig System Module Search Paths pkgconfig System Module Search Paths Apr 16, 2016
@mxcl
Copy link
Contributor

mxcl commented Apr 18, 2016

LGTM, but conflicts.

@aciidgh
Copy link
Contributor Author

aciidgh commented Apr 18, 2016

Fixed

@mxcl
Copy link
Contributor

mxcl commented Apr 18, 2016

@swift-ci Please test and merge

@swift-ci swift-ci merged commit 7bb14b2 into swiftlang:master Apr 18, 2016
@mxcl
Copy link
Contributor

mxcl commented Apr 18, 2016

We should look at the pkg-config sources, I'm not sure this path is even in the general set.

libs = resolveVariables(value(line: line)).chomp()
} else if line.hasPrefix("Cflags: ") {
cFlags = resolveVariables( value(line: line)).chomp()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not very familiar with the format, but I would recommend trying to get an else clause in here that will error out if we see something unexpected. This prevents the code from silently failing to work properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is not really an exhaustive list of possible cases in the file, there are some things for eg Name: Description: etc which we aren't interested in and nor want to parse, how to handle those ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a complete list available? If so, we could just check if it is in the known complete list and ignore if not there.

We can at least partition into the following three cases, right?

  1. Variable assignment
  2. Key-value binding (Name: Value)
  3. Comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think those are the only cases. I'll rework the parser in another PR which should address the parser related comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in fb5e87b

@aciidgh aciidgh mentioned this pull request Apr 19, 2016
@aciidgh aciidgh deleted the pkg-config branch April 19, 2016 16:26
@rfdickerson
Copy link

Seems to work very well for swift build. Although, currently the XCode project generation seems to not consider these pkg-config produced header search paths. https://bugs.swift.org/browse/SR-1368

aciidgh pushed a commit to aciidgh/swift-package-manager that referenced this pull request Jan 11, 2019
Report extended results in commandProcessFinished
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.

6 participants