Helpers in config/makefiles/debugmake.mk don't work properly with quotes and other special characters

RESOLVED FIXED in mozilla28

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla28
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

$ make -f config/makefiles/debugmake.mk echo-variable-FOO FOO='-Dfoo=\"bar\"'
-Dfoo="bar"

$ make -f config/makefiles/debugmake.mk echo-variable-FOO FOO='-Dfoo="bar"'
-Dfoo=bar

Sure, that matches what a command would see in its arguments list, but it's not usually what i'm after when I use echo-variable-%.

Moreover,

$ make -f config/makefiles/debugmake.mk echo-variable-FOO FOO="-Dfoo=\"foo='bar'\""
-Dfoo=foo=bar

Here this doesn't match what a command would see in its arguments list. It would see "-Dfoo=foo='bar'".

Another problem comes with spaces:

$ make -f config/makefiles/debugmake.mk echo-variable-FOO FOO="-Dfoo=\"foo bar\" -Dbar=baz"
-Dfoo=foo bar -Dbar=baz

Finally, if double quotes are unbalanced for some reason:
$ make -f config/makefiles/debugmake.mk echo-variable-FOO FOO="-Dfoo=\"foo"
/bin/sh: 1: Syntax error: Unterminated quoted string
make: *** [echo-variable-FOO] Error 2
Blocks: 944569
Comment on attachment 8340155 [details] [diff] [review]
Refactor config/makefiles/debugmake.mk for more correctness, and remove old cruft

Review of attachment 8340155 [details] [diff] [review]:
-----------------------------------------------------------------

Conditional on not deleting print-depth-path.sh.

::: build/unix/print-depth-path.sh
@@ -1,1 @@
> -#!/bin/sh

This file is still referenced in rules.mk as part of UPDATE_TITLE and that makes it POTB.

::: config/makefiles/debugmake.mk
@@ +28,3 @@
>  
>  echo-dirs:
> +	@echo $(call shell_quote,$(DIRS))

But we shouldn't have any directories that require escaping. Meh.
Attachment #8340155 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] from comment #2)
> This file is still referenced in rules.mk as part of UPDATE_TITLE and that
> makes it POTB.

How about removing that too and using $(relativesrcdir) instead?
(In reply to Mike Hommey [:glandium] from comment #3)
> (In reply to Gregory Szorc [:gps] from comment #2)
> > This file is still referenced in rules.mk as part of UPDATE_TITLE and that
> > makes it POTB.
> 
> How about removing that too and using $(relativesrcdir) instead?

Submit a patch and I'll review it. Or if you feel it is trivial, just r=gps.
https://hg.mozilla.org/mozilla-central/rev/13d589530998
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.