Skip to content

Conversation

@bnoordhuis
Copy link
Member

Fix parsing of pkg-config --cflags-only-I. The configure_library()
step sometimes appended a list in a list instead of list of strings to
include_dirs.

This commit removes the default handling for includes and libpath
options. They don't have defaults at the moment and I don't see that
changing anytime soon. Fixing the code is more work and because it's
dead code anyway, I opted to remove it instead.

Fixes: #1985

R=@jbergstroem

CI: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/7/

@jbergstroemjbergstroem added the build Issues and PRs related to build files or the CI. label Jun 15, 2015
@Haifen
Copy link

Can confirm this fixes #1985 on the same build machine and environment where I first encountered the bug.

Referenced in Gentoo bugtracker at:
Bug 552232
Attachment 405206

@jbergstroem
Copy link
Member

I've tried adding all kinds of weird stuff to my .pc configs - seems to be robust. LGTM. I'm keen on exploring how we can avoid parsing output at all..

Fix parsing of `pkg-config --cflags-only-I`. The configure_library() step sometimes appended a list in a list instead of list of strings to include_dirs. This commit removes the default handling for includes and libpath options. They don't have defaults at the moment and I don't see that changing anytime soon. Fixing the code is more work and because it's dead code anyway, I opted to remove it instead. Fixes: nodejs#1985 PR-URL: nodejs#1986 Reviewed-By: Johan Bergström <[email protected]>
@bnoordhuisbnoordhuis deleted the fix-issue-1985 branch June 15, 2015 23:16
@bnoordhuisbnoordhuis merged commit c207e8d into nodejs:masterJun 15, 2015
@rvaggrvagg mentioned this pull request Jun 16, 2015
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@bnoordhuis@Haifen@jbergstroem