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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

$ 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

Comment 1

[Mike Hommey [:glandium]]

Attachment: Refactor config/makefiles/debugmake.mk for more correctness, and remove old cruft

Comment 2

[Gregory Szorc [:gps]]

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.

Comment 3

[Mike Hommey [:glandium]]

(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?

Comment 4

[Gregory Szorc [:gps]]

(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.

Comment 5

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/13d589530998

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/13d589530998