Pymake doesn't handle spacing in front of an endef

RESOLVED FIXED

Status

defect
RESOLVED FIXED
9 years ago
Last year

People

(Reporter: khuey, Assigned: khuey)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Posted patch PatchSplinter Review
No description provided.
Attachment #483066 - Flags: review?(ted.mielczarek)
That print directive is obviously debug code.  Ignore that.
Comment on attachment 483066 [details] [diff] [review]
Patch

># HG changeset patch
># Parent 4f7a8e2c7b205ff37585d8670f45a3ce57067f52
>diff --git a/build/pymake/pymake/parser.py b/build/pymake/pymake/parser.py
>--- a/build/pymake/pymake/parser.py
>+++ b/build/pymake/pymake/parser.py
>@@ -228,31 +228,32 @@ def itercommandchars(d, offset, tokenlis
>         if token in tokenlist or (token[0] == '$' and '$' in tokenlist):
>             yield starttext, token, mstart, mend
>         else:
>             yield starttext + token, None, None, mend
>         offset = mend
> 
>     yield s[offset:d.lend].replace('\n\t', '\n'), None, None, None
> 
>-_redefines = re.compile('define|endef')
>+_redefines = re.compile('\s*define|\s*endef')
> def iterdefinelines(it, startloc):
>     """
>     Process the insides of a define. Most characters are included literally. Escaped newlines are treated
>     as they would be in makefile syntax. Internal define/endef pairs are ignored.
>     """
> 
>     results = []
> 
>     definecount = 1
>     for d in it:
>         m = _redefines.match(d.s, d.lstart, d.lend)
>         if m is not None:
>             directive = m.group(0)

Just strip it right here to avoid having to do it below?

>diff --git a/build/pymake/tests/define-directive.mk b/build/pymake/tests/define-directive.mk
>--- a/build/pymake/tests/define-directive.mk
>+++ b/build/pymake/tests/define-directive.mk
>@@ -45,15 +45,25 @@ ifneq ($(TEST8),is this \# a comment?)
> endif
> 
> # A backslash continuation "hides" the endef
> define TEST9
> value \
> endef
> endef
> 
>+# Test ridiculous spacing
>+ define TEST9

You probably want to start with TEST10, for consistency.

r=me with those nits fixed.
Attachment #483066 - Flags: review?(ted.mielczarek) → review+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.