| Summary: | Unqoting values of requested variables in pkg-config might break builds | ||
|---|---|---|---|
| Product: | pkg-config | Reporter: | Marco Trevisan (Treviño) <mail> |
| Component: | src | Assignee: | Dan Nicholson <nicholson> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | CC: | mkasik, nicholson |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| See Also: | https://bugs.freedesktop.org/show_bug.cgi?id=67904 | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
Revert "Unquote values of requested variables"
Only unquote --variable when it appears quoted check: More thoroughly test variable usage Revert "Quote pc_path virtual variable" |
||
Created attachment 121398 [details] [review] Revert "Unquote values of requested variables" This reverts commit 50c2867f4a6981e085c721d936c96f174f11f415. g_shell_quote doesn't match the quoting behavior of pkg-config when the same variables are output via --cflags and similar. Created attachment 121399 [details] [review] Only unquote --variable when it appears quoted The change to unquote values in the --variable output broke users that had shell special characters in the variable. Instead, only unquote if the value starts with " or '. A larger fix to do a full unquote, split and escaping like --cflags/--libs is possible, but that might break the old semantics even further. Add a new function, parse_package_variable(), to handle that logic. Created attachment 121400 [details] [review] check: More thoroughly test variable usage Add some more tests for handling unusual variables such as those that are quoted or that contain shell characters. This should help make the --variable output more reliable in the future. Created attachment 121401 [details] [review] Revert "Quote pc_path virtual variable" This reverts commit a6e8749ada5af1737b27f1eca1babe83e82af38c. With the --variable output only being unquoted when it appears needed, this can return to being a normally defined value. Argh, sorry. I should have realized that unconditionally calling unquote would break things. Can you try the attached patches? I changed it to only unquote if the value starts with ' or ". BTW, this issue originally came from bug67904. Adding Marek to CC to make sure this fix doesn't break his case. (In reply to Dan Nicholson from comment #6) > BTW, this issue originally came from bug67904. Adding Marek to CC to make > sure this fix doesn't break his case. Hi, I confirm that the changes also fixes the problem I was fixing by the patch. Btw, it was this one: https://bugzilla.redhat.com/show_bug.cgi?id=961855. Regards Thanks, Marek. Marco, do you think you could test this or point me to the issue you're facing? (In reply to Dan Nicholson from comment #8) > Thanks, Marek. Marco, do you think you could test this or point me to the > issue you're facing? This was the issue we were getting (explained on comment #0): - https://bugs.launchpad.net/compiz/+bug/1521366 We've currently "fixed" it by using a workaround at CMakeFile level, but I guess reverting the commit and adding a --unquoted-variable cmd line would be the best choice to do to avoid breakage. (In reply to Marco Trevisan (Treviño) from comment #9) > > We've currently "fixed" it by using a workaround at CMakeFile level, but I > guess reverting the commit and adding a --unquoted-variable cmd line would > be the best choice to do to avoid breakage. What I'd be doing here would be reverting so that --variable had the behavior it's had since it was introduced years ago except in the case that the value appears to be quoted. For your downstream case, you could revert that change and depend on pkg-config-0.29.1 (or build-conflict with pkg-config-0.29). But I really think this is a regression and the proper fix is to change back to the original behavior as much as possible. Unfortunately, that makes this particular case more difficult, but people using such complex values in variables is almost certainly a rarity. I'm going to apply this and release 0.29.1 with it. Please reopen if you need something else to handle this correctly (e.g., a special --unquoted-variable or such). Applied in 5164b9dbabdca00fbd9d6bb962c4ac9b252448a2. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
I'm Marco, from the Ubuntu desktop team; and we're facing an issue with recent pkg-config when using it in cmake. As per commit 50c2867f4a6981e085c721d936c96f174f11f415 in pkg-config, variables are unquoted. This is fine for most of scenarios, but when there are variables such as CPPflags=-I${includedir} -I${includedir}/xorg -I${sourcedir} \ -DDUMMY_CONF_PATH=\"${datarootdir}/xorg/gtest/dummy.conf\" \ -DLOGFILE_DIR=\"/tmp\" Then when using the new pkg-config "pkg-config --variable=CPPflags" returns something like: -I/usr/include -I/usr/include/xorg -I/usr/src/xorg-gtest \ -DDUMMY_CONF_PATH="/usr/share/xorg/gtest/dummy.conf" \ -DLOGFILE_DIR="/tmp" Which is wrong since the quotes aren't escaped. Changing the variable so that it uses something like -DLOGFILE_DIR=\\\"/tmp\\\" Makes it return the proper thing, but not in a subshell, but it doesn't seem the case. In fact pkg-config seems to return the proper thing when called in a subshell, but not when used directly. I.e.: $ pkg-config --variable=CPPflags xorg-gtest + pkg-config --variable=CPPflags xorg-gtest -I/usr/include -I/usr/include/xorg -I/usr/src/xorg-gtest -DDUMMY_CONF_PATH="/usr/share/xorg/gtest/dummy.conf" -DLOGFILE_DIR="/tmp" $ gcc-5 $(pkg-config --variable=CPPflags xorg-gtest) foo.c ++ pkg-config --variable=CPPflags xorg-gtest + gcc-5 -I/usr/include -I/usr/include/xorg -I/usr/src/xorg-gtest '-DDUMMY_CONF_PATH="/usr/share/xorg/gtest/dummy.conf"' '-DLOGFILE_DIR="/tmp"' foo.c As you can see when called in a subshell, the variables are properly single-quoted, making the thing work. But when calling this directly there are no single quotes. So I'm wondering what's the best way to fix this case.