409956 - Python preprocessor ignores #filter emptyLines

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Robert Kaiser]

Bug 409805 comment #2 talks of a case that would have worked with the perl preprocessor but doesn't work with the python version (AFAIK I had tried both versions when working on bug 397246):

|#filter emptyLines| should remove all the empty lines until |#unfilter emptyLines| but the python preprocessor seems to ignore that.

Comment 1

[Axel Hecht [:Pike]]

It's implemented inhttp://mxr.mozilla.org/mozilla/source/config/Preprocessor.py#376.

I wonder if there's a line endings problem in that defines.inc.

Comment 2

[Robert Kaiser]

We had that with multiple incarnations of that file, and it's always unix line endings from what I can tell...

Comment 3

[Axel Hecht [:Pike]]

I found the problem, and I have a patch and a test case offline. I'll attach that next year.

Comment 4

[Robert Kaiser]

It's been "next year" now for some time, did this patch get lost? I saw today that we are running into the same problem with normal SeaMonkey langpacks :(

Comment 5

[Axel Hecht [:Pike]]

Attachment: use universal line ending io and replace lineending, don't add

So the basic culprit was that the do_filter routine returned an empty string, and write appended a line ending.

Now I do a replace, so empty stays empty.

Going through this, I moved over to open with 'rU' for universal line ending support, which reads in files correctly independent of OS line endings, and converts them to '\n', which makes the checks easier. That only works for real files, though, it seems.

Added a few tests to verify the behaviour here, too.

Comment 6

[Axel Hecht [:Pike]]

Marking FIXED, http://hg.mozilla.org//mozilla-central/index.cgi/rev/4157b3484b8e.

Robert, you want this on CVS trunk, right?

Comment 7

[Robert Kaiser]

Yes, please, at least for the time being where SeaMonkey (and others) are all still based on CVS.

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 322570 [details] [diff] [review]
use universal line ending io and replace lineending, don't add

This stuck on mozilla-central, I'd like to land it on cvs trunk, too.

Comment 9

[Samuel Sidler (old account; do not CC)]

Robert, since SeaMonkey is moving to hg, is this still wanted on the 1.9 branch?

Comment 10

[Robert Kaiser]

After the switch, I don't care if it's on 1.9.0, but not sure if Axel would like to have it for e.g. easing Firefox localizers' work.

Comment 11

[Axel Hecht [:Pike]]

Yes, we should take this for fx3.0.x localizations. The line ending stuff discovered other bugs.

Comment 12

[Axel Hecht [:Pike]]

Actually, drop the rationale.

The rationale is really that I hope to land other patches to config which may very well touch Preprocessor.py, and it'd be good to have that tool on par with mozilla-central to reduce the risk in merges.

Comment 13

[Samuel Sidler (old account; do not CC)]

Comment on attachment 322570 [details] [diff] [review]
use universal line ending io and replace lineending, don't add

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 14

[Axel Hecht [:Pike]]

Landed in CVS, too.