Closed Bug 758354 Opened 12 years ago Closed 12 years ago

Stop preprocessing chrome unnecessarily


(Toolkit Graveyard :: Build Config, defect)

Not set


(Not tracked)



(Reporter: neil, Assigned: neil)




(4 files)

I wrote a shell script to see which files were being preprocessed unnecessarily. Within toolkit it found 55 preprocessed files that lacked any preprocessor directives. It increased to 128 when you included files that lacked preprocessor directives that started with a a letter, which I assume to mean that the file only contains a preprocessed licence block. Now that these are only three lines long, they are probably no longer worth preprocessing.

The script's first parameter is either '' or '\w' depending on whether you're looking for actual preprocessor statements. (The ^# or ^% is added by the script.) The remaining parameters, if any, are the directories to search.
Seems like we could also make the preprocessor emit a warning when it ends up doing nothing.
I excluded 222 DOM Inspector files because of its funky locales setup, but I suspect that none of its locales need to be preprocessed ;-)
I have a script that automatically fixes useless preprocessing (including both ones that do nothing and ones that just use it for stripping out comments) attached; it can be found on bug 760126. I disclaim any and all problems that could come from using the script :-P
See Also: → 760126
Attached patch Proposed patchSplinter Review
Assignee: nobody → neil
Attachment #636116 - Flags: review?(dolske)
Attachment #636116 - Flags: review?(dolske) → review+
(In reply to :Gavin Sharp (use for email) from comment #1)
> Seems like we could also make the preprocessor emit a warning when it ends
> up doing nothing.

I really like this idea.
Attached file Sample output
So, this is edited output from a depend rebuild of SeaMonkey using a locally hacked Preprocessor and JarMaker. There are three sources of mispreprocessing:
1. Unnecessarily preprocessed chrome
2. Unnecessarily preprocessed modules or components
3. Duplicate preprocessing
application.ini and update-settings.ini appears to fall in category 3.
Attachment #636173 - Flags: feedback?(
Attachment #636173 - Flags: feedback?(dolske)
Attachment #636173 - Attachment is patch: false
Pushed integration/mozilla-inbound changeset 1718e78e0cb0.
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Comment on attachment 636173 [details]
Sample output

Not sure what feedback you're looking for here, but sure.
Attachment #636173 - Flags: feedback?(dolske) → feedback+
(followup bug for any such build changes though, please)
Comment on attachment 636173 [details]
Sample output

yes, let's fix these!?
Attachment #636173 - Flags: feedback?(
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.