Fix CFLAGS & Makefile issues #2168
Conversation
Split by space instead of dash.
This prevents incorrect parsing where space-separated arguments are
merged (in particular, .c or .o files were appended to -I or -D
arguments).
Handle shell escape: quotes and escaped quotes \" and shell
substitutions are recognised. This is done by verifying that no special
character (" ' ` ()) has not a matching character.
Fixes #2049
|
I added a test that fails with current master and is fixed with this PR. |
|
Sure, as long as there are tests to cover all of the interesting cases, I'm happy. |
|
Cheers! 🍻 |
|
It looks like there were some regressions with this patch. As mentioned on #2131, we shouldn't be passing -o and other flags through this parsing step. I'm manually testing immediately before and after this change by adding an |
|
If there are regressions, it's because there weren't enough tests. The author of that other PR should add more tests and cover everything. |
|
I don't. I tested with my Makefiles which include -o flags, and I didn't see them in the results. Do you have an example where it passes a -o flag? I took a look at the PR you linked, and I think it doesn't handle the cases where the argument contains spaces. |
|
Here's an example: It looks like it goes into the if on line 77 of |
|
Thanks, I can reproduce the bug with your example. The issue was with the matching of shell special characters. It should be fixed with #2194. |
This PR fixes #2049
The arguments are split by
' 'instead of'-', so the bugs where other filenames are appended to the flags are fixed. The cases with quoted arguments containing spaces are handled, as well as arguments separated with spaces (eg.-D directory).