Open Bug 861801 Opened 7 years ago Updated 2 years ago

can we silence "no preprocessor directives found" warning if we must pass a file to preprocessor for line-endings cleanup?

Categories

(Firefox Build System :: General, defect)

defect
Not set
trivial

Tracking

(Not tracked)

People

(Reporter: aceman, Unassigned)

Details

In bug 810675 for comm-central we have got some files that report "WARNING: no preprocessor directives found" when building. We identified those are pref *.js files that really do not have any directives, but we pass them to the preprocessor so that the line endings can be fixed with the --line-endings=crlf flag. Also, the flag is only passed for Windows builds. On the other platforms this flag is not set so that the preprocessor really does nothing (as there are no directives) so the warning is correct.
Maybe we could pass --line-endings=lf in those platforms but I do not think it will silence the warning.

Is there any solution for this in the preprocessor to not get this warning?
Perhaps we shouldn't be using the preprocessor for just converting line endings.
But maybe that is handy when you need converting line ending and also preprocessor macros. So that you do not need to run 2 programs.
Why do we need line ending conversion at all? It's not like Windows can't handle lines without CR.
Just a guess, but maybe this was because we used to make files look nice and easily readable on all platforms, so that people could hack them (e.g. I suspect we'd want the default prefs.js in the profile to be formatted nicely - http://mxr.mozilla.org/comm-central/source/mozilla/browser/app/profile/prefs.js), but the ones in omni.ja, that's probably not really necessary now.
People would still like to hack on files in omni.ja but it is quite messed up now.
(In reply to Mark Banner (:standard8) from comment #4)
> Just a guess, but maybe this was because we used to make files look nice and
> easily readable on all platforms

I haven't seen an editor that can't handle lines without CR on Windows for a while, except maybe the default notepad. Is it really still a concern in 2013?
But Mozilla is supposedly about standards (in this case platform standards), not relying on editor quirks :)

But I do agree that if we do not do it for other code in xul and js files, then it is not necessary for prefs js inside omni.jar .
Can anybody tell if firefox does this? So we can do the same in TB.
The following files in /browser/ have CRLF line endings:

04/10/2012  13:55             968  app_bug575561.html
28/03/2013  11:42           3,100  browser_aboutHealthReport.js
04/10/2012  13:55           3,855  browser_bug575561.js
13/04/2013  11:42           8,531  browser_bug722691_rule_view_increment.js
13/03/2013  11:20           3,814  browser_inspector_bug_835722_infobar_reappears.js
02/04/2013  12:12             231  browser_onscreen_keyboard.html
02/04/2013  12:12           2,295  browser_remotetabs.js
02/04/2013  12:12           3,651  browser_sanitize_ui.js
09/04/2013  12:14           3,018  browser_tabs.js
02/04/2013  12:12           2,232  browser_tilegrid.xul
12/04/2013  12:04          13,009  browser_tiles.js
04/10/2012  13:56             870  bug494328-data.xml
13/02/2013  12:07           5,384  commandUtil.js
04/10/2012  13:56             408  ContentA.html
04/10/2012  13:56             408  ContentB.html
04/10/2012  13:56             463  ContentWithFrames.html
04/10/2012  13:56             696  head_dirprovider.js
28/03/2013  11:42           3,191  healthreport_testRemoteCommands.html
04/10/2012  13:55         120,763  jquery.js
08/10/2012  12:05          92,556  jquery.min.js
04/04/2013  19:36           3,766  nsIEHistoryEnumerator.cpp
12/01/2013  19:21             892  nsIEHistoryEnumerator.h
04/10/2012  13:56             252  plugin_both.html
04/10/2012  13:56             252  plugin_both2.html
04/10/2012  13:56             166  plugin_clickToPlayAllow.html
04/10/2012  13:56             166  plugin_clickToPlayDeny.html
04/10/2012  13:56             166  plugin_test.html
04/10/2012  13:56             162  plugin_test3.html
04/10/2012  13:56             172  plugin_unknown.html
24/02/2013  12:12           4,149  preferences.css
28/03/2013  11:42           3,230  sanitizeUI.js
15/03/2013  12:19          19,268  SelectorSearch.jsm
04/10/2012  13:56             559  test_bookmark_pref.js
08/03/2013  12:46           1,333  test_util_extend.js
08/03/2013  12:46              53  xpcshell.ini
We should probably make mercurial keep LF only.
http://mercurial.selenic.com/wiki/EolExtension
(In reply to Philip Chee from comment #9)
> The following files in /browser/ have CRLF line endings:
> 
If those are in the tree, isn't that a bug? I think official source files are supposed to have LF line endings only.

Here, we discuss which of them should be converted to CRLF when built into the binary package.
> We should probably make mercurial keep LF only.
> http://mercurial.selenic.com/wiki/EolExtension
1. Except for Thunderbird test files that specifically test for RFC conformance.
2. Third-party libraries such as JQuery.
3. ?
http://hg.mozilla.org/mozilla-central/file/a6c1abbdf6f9/config/rules.mk#l1144 reminds me that the line endings conversion for pref files was introduced in bug 206029.

10 years ago, the Flash Player 7 installer completely messed up the pref files if they had unix-style line endings.
At the time, that turned Firebird 0.6 into something that looked like Seamonkey (but didn't work at all of course). Don't miss the screenshot I attached back then to bug 206029 !

I haven't tested if that's still an issue.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.