pkgconf doesn't preserve quotes #82

New Issue

ariadne · 2015-09-11T00:36:30Z

dankegel commented

2015-09-11 00:36:30 +00:00

(Migrated from github.com)

I authored a pc file against pkgconf, then discovered it didn't work with pkg-config:

$ pkgconf --libs libcef
-framework 'Chromium Embedded Framework' 
$ pkg-config --libs libcef
-framework \'Chromium\ Embedded\ Framework\'

pkgconf should probably follow pkg-conf's quoting behavior, i.e. it should preserve rather than discard quote marks.

https://github.com/pkgconf/pkgconf/pull/81 is a draft commit to do this.

I authored a pc file against pkgconf, then discovered it didn't work with pkg-config: ``` $ pkgconf --libs libcef -framework 'Chromium Embedded Framework' $ pkg-config --libs libcef -framework \'Chromium\ Embedded\ Framework\' ``` pkgconf should probably follow pkg-conf's quoting behavior, i.e. it should preserve rather than discard quote marks. https://github.com/pkgconf/pkgconf/pull/81 is a draft commit to do this.

dankegel commented

2015-09-11 01:01:38 +00:00

(Migrated from github.com)

pkg-conf seems to always convert to backslash escaping, but I doubt that matters... preserving quotes is probably ok?

ariadne commented

2015-09-11 19:22:26 +00:00

#81 seems reasonable. I merged it. Thanks!

dankegel commented

2015-09-15 16:57:12 +00:00

(Migrated from github.com)

Alas, I spoke too soon. I assumed pkg-config could handle that, but I didn't test properly.

Turns out that to write a .pc file for a framework with spaces in its name, you have to use backslashes for it to work with pkg-config. (See worked example at http://kegel.com/cmake/spaces-in-framework-names/ )

Next step would be to write some more tests for quoting corner cases that pass on both pkg-config and pkgconf and/or revert the new behavior.

Alas, I spoke too soon. I assumed pkg-config could handle that, but I didn't test properly. Turns out that to write a .pc file for a framework with spaces in its name, you have to use backslashes for it to work with pkg-config. (See worked example at http://kegel.com/cmake/spaces-in-framework-names/ ) Next step would be to write some more tests for quoting corner cases that pass on both pkg-config and pkgconf and/or revert the new behavior.

ariadne commented

2015-09-16 19:22:15 +00:00

I'm not so sure, I think that the new behavior is desirable, and it's probably a bug in legacy pkg-config too. I think that perhaps they should just implement the same behavior as us here?

ariadne commented

2015-09-19 17:06:35 +00:00

@bapt thoughts?

dankegel commented

2015-09-19 17:46:36 +00:00

(Migrated from github.com)

https://bugs.freedesktop.org/show_bug.cgi?id=3571 seemed to indicate legacy pkg-config wanted to support quotes, but they have never really documented the behavior.

If backslashes turn out to suffice in the real world, there's less motivation for the change.
Are there examples where it doesn't work?

https://bugs.freedesktop.org/show_bug.cgi?id=3571 seemed to indicate legacy pkg-config wanted to support quotes, but they have never really documented the behavior. If backslashes turn out to suffice in the real world, there's less motivation for the change. Are there examples where it doesn't work?

bapt commented

2015-09-19 18:08:05 +00:00

(Migrated from github.com)

The current behaviour seems fine to me, I also think pkg-config should follow our behaviour here

dankegel commented

2015-09-19 18:15:33 +00:00

(Migrated from github.com)

What's their motivation for changing? Is there an unmet need?

ariadne commented

2015-09-19 20:26:23 +00:00

well i think having it as a single fragment without escape is better for consumers outside of the pkgconf(1) tool, such as libpkgconf consumers. stuff like rust/cargo will be moving away from pkgconf(1) and consuming libpkgconf directly in this way. so ideally, we want to solve this in a way that makes it easy for all consumers of the modules I suppose.

dankegel commented

2015-09-19 20:36:10 +00:00

(Migrated from github.com)

IMHO forking the .pc format is a dangerous thing to do, and needs to be done very carefully.
I would not have proposed this patch if I had accurately understood the behavior of legacy pkg-config.

IMHO forking the .pc format is a dangerous thing to do, and needs to be done very carefully. I would not have proposed this patch if I had accurately understood the behavior of legacy pkg-config.

ariadne commented

2015-09-19 20:47:39 +00:00

That is true, maybe we revert it for now. I believe pkgconf will respect the backslashes for quoting already.

@bapt thoughts?

That is true, maybe we revert it for now. I believe pkgconf will respect the backslashes for quoting already. @bapt thoughts?

bapt commented

2015-09-19 21:18:49 +00:00

(Migrated from github.com)

@kaniini fine with me.

Sign in to join this conversation.