Bug 1210329 - Remove support for line endings munging in the preprocessor. r=gps
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 01 Oct 2015 18:01:12 +0900
changeset 266204 6b5dc81ee5789a0e4b7775c07c0941cb3a387df2
parent 266203 37bc093b6a78226e5790fa8aff8f72764a2ac58a
child 266205 1f0ba759efbc710e2db64b215127b7b0f2bf8910
push id29483
push usercbook@mozilla.com
push dateTue, 06 Oct 2015 10:01:59 +0000
treeherdermozilla-central@89732fcdb0ba [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs1210329, 206029
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1210329 - Remove support for line endings munging in the preprocessor. r=gps It was added back in https://github.com/mozilla/gecko-dev/commit/5147d5c69fa5c53ea34e2872136bf55fc8c37664 for unclear reasons (and the lack of bug number doesn't help), and hasn't been used, as far as I can see in the gecko-dev history, other than in bug 206029, which is the only use currently in the tree. Bug 206029 was working around the Flash player installer modifying Firefox's prefs file and not dealing with it properly or something depending on the line endings. 11 years later, all prefs files except channel-prefs.js are in omni.ja, so obviously, bug 206029 doesn't actually apply anymore. So, let's simplify it all and get rid of this.
config/rules.mk
python/mozbuild/mozbuild/backend/fastermake.py
python/mozbuild/mozbuild/preprocessor.py
python/mozbuild/mozbuild/test/test_preprocessor.py
--- a/config/rules.mk
+++ b/config/rules.mk
@@ -1147,21 +1147,16 @@ PREF_DIR = defaults/pref
 
 # If DIST_SUBDIR is defined it indicates that app and gre dirs are
 # different and that we are building app related resources. Hence,
 # PREF_DIR should point to the app prefs location.
 ifneq (,$(DIST_SUBDIR)$(XPI_NAME)$(LIBXUL_SDK))
 PREF_DIR = defaults/preferences
 endif
 
-# on win32, pref files need CRLF line endings... see bug 206029
-ifeq (WINNT,$(OS_ARCH))
-PREF_PPFLAGS += --line-endings=crlf
-endif
-
 ifneq ($(PREF_JS_EXPORTS),)
 ifndef NO_DIST_INSTALL
 PREF_JS_EXPORTS_PATH := $(FINAL_TARGET)/$(PREF_DIR)
 # We preprocess these, but they don't necessarily have preprocessor directives,
 # so tell them preprocessor to not complain about that.
 PREF_JS_EXPORTS_FLAGS := $(PREF_PPFLAGS) --silence-missing-directive-warnings
 PP_TARGETS += PREF_JS_EXPORTS
 endif
--- a/python/mozbuild/mozbuild/backend/fastermake.py
+++ b/python/mozbuild/mozbuild/backend/fastermake.py
@@ -112,19 +112,16 @@ class FasterMakeBackend(CommonBackend):
             # different from dist/bin.
             if obj.install_target == 'dist/bin':
                 pref_dir = 'defaults/pref'
             else:
                 pref_dir = 'defaults/preferences'
 
             dest = mozpath.join(obj.install_target, pref_dir,
                                 mozpath.basename(obj.path))
-            # on win32, pref files need CRLF line endings... see bug 206029
-            if self.environment.substs['OS_ARCH'] == 'WINNT':
-                defines.append('--line-endings=crlf')
             # We preprocess these, but they don't necessarily have preprocessor
             # directives, so tell the preprocessor to not complain about that.
             defines.append('--silence-missing-directive-warnings')
             self._preprocess_files[dest] = (obj.srcdir, obj.path, defines)
 
         elif isinstance(obj, Resources) and \
                 obj.install_target.startswith('dist/bin'):
             for path, strings in obj.resources.walk():
--- a/python/mozbuild/mozbuild/preprocessor.py
+++ b/python/mozbuild/mozbuild/preprocessor.py
@@ -269,17 +269,17 @@ class Preprocessor:
     """
     class Error(RuntimeError):
         def __init__(self, cpp, MSG, context):
             self.file = cpp.context['FILE']
             self.line = cpp.context['LINE']
             self.key = MSG
             RuntimeError.__init__(self, (self.file, self.line, self.key, context))
 
-    def __init__(self, line_endings='\n', defines=None, marker='#'):
+    def __init__(self, defines=None, marker='#'):
         self.context = Context()
         for k,v in {'FILE': '',
                     'LINE': 0,
                     'DIRECTORY': os.path.abspath('.')}.iteritems():
             self.context[k] = v
         self.actionLevel = 0
         self.disableLevel = 0
         # ifStates can be
@@ -306,36 +306,29 @@ class Preprocessor:
                            'filter': 0,
                            'unfilter': 0,
                            'include': 0,
                            'includesubst': 0,
                            'error': 0}.iteritems():
             self.cmds[cmd] = (level, getattr(self, 'do_' + cmd))
         self.out = sys.stdout
         self.setMarker(marker)
-        self.LE = line_endings
         self.varsubst = re.compile('@(?P<VAR>\w+)@', re.U)
         self.includes = set()
         self.silenceMissingDirectiveWarnings = False
         if defines:
             self.context.update(defines)
 
     def warnUnused(self, file):
         if self.actionLevel == 0 and not self.silenceMissingDirectiveWarnings:
             sys.stderr.write('{0}: WARNING: no preprocessor directives found\n'.format(file))
         elif self.actionLevel == 1:
             sys.stderr.write('{0}: WARNING: no useful preprocessor directives found\n'.format(file))
         pass
 
-    def setLineEndings(self, aLE):
-        """
-        Set the line endings to be used for output.
-        """
-        self.LE = {'cr': '\x0D', 'lf': '\x0A', 'crlf': '\x0D\x0A'}[aLE]
-
     def setMarker(self, aMarker):
         """
         Set the marker to be used for processing directives.
         Used for handling CSS files, with pp.setMarker('%'), for example.
         The given marker may be None, in which case no markers are processed.
         """
         self.marker = aMarker
         if aMarker:
@@ -368,17 +361,16 @@ class Preprocessor:
     def clone(self):
         """
         Create a clone of the current processor, including line ending
         settings, marker, variable definitions, output stream.
         """
         rv = Preprocessor()
         rv.context.update(self.context)
         rv.setMarker(self.marker)
-        rv.LE = self.LE
         rv.out = self.out
         return rv
 
     def processFile(self, input, output, depfile=None):
         """
         Preprocesses the contents of the ``input`` stream and writes the result
         to the ``output`` stream. If ``depfile`` is set,  the dependencies of
         ``output`` file are written to ``depfile`` in Makefile format.
@@ -417,26 +409,22 @@ class Preprocessor:
         """
         if not self.out:
             return
 
         if self.checkLineNumbers:
             self.writtenLines += 1
             ln = self.context['LINE']
             if self.writtenLines != ln:
-                self.out.write('//@line {line} "{file}"{le}'.format(line=ln,
-                                                                    file=self.context['FILE'],
-                                                                    le=self.LE))
+                self.out.write('//@line {line} "{file}"\n'.format(line=ln,
+                                                                  file=self.context['FILE']))
                 self.writtenLines = ln
         filteredLine = self.applyFilters(aLine)
         if filteredLine != aLine:
             self.actionLevel = 2
-        # ensure our line ending. Only need to handle \n, as we're reading
-        # with universal line ending support, at least for files.
-        filteredLine = re.sub('\n', self.LE, filteredLine)
         self.out.write(filteredLine)
 
     def handleCommandLine(self, args, defaultToStdin = False):
         """
         Parse a commandline into this parser.
         Uses OptionParser internally, no args mean sys.argv[1:].
         """
         def get_output_file(path):
@@ -499,18 +487,16 @@ class Preprocessor:
                 vals[1] = vals[1][1:-1]
             elif numberValue.match(vals[1]):
                 vals[1] = int(vals[1])
             self.context[vals[0]] = vals[1]
         def handleU(option, opt, value, parser):
             del self.context[value]
         def handleF(option, opt, value, parser):
             self.do_filter(value)
-        def handleLE(option, opt, value, parser):
-            self.setLineEndings(value)
         def handleMarker(option, opt, value, parser):
             self.setMarker(value)
         def handleSilenceDirectiveWarnings(option, opt, value, parse):
             self.setSilenceDirectiveWarnings(True)
         p = OptionParser()
         p.add_option('-I', action='append', type="string", default = [],
                      metavar="FILENAME", help='Include file')
         p.add_option('-D', action='callback', callback=handleD, type="string",
@@ -519,19 +505,16 @@ class Preprocessor:
                      metavar="VAR", help='Undefine a variable')
         p.add_option('-F', action='callback', callback=handleF, type="string",
                      metavar="FILTER", help='Enable the specified filter')
         p.add_option('-o', '--output', type="string", default=None,
                      metavar="FILENAME", help='Output to the specified file '+
                      'instead of stdout')
         p.add_option('--depend', type="string", default=None, metavar="FILENAME",
                      help='Generate dependencies in the given file')
-        p.add_option('--line-endings', action='callback', callback=handleLE,
-                     type="string", metavar="[cr|lr|crlf]",
-                     help='Use the specified line endings [Default: OS dependent]')
         p.add_option('--marker', action='callback', callback=handleMarker,
                      type="string",
                      help='Use the specified marker instead of #')
         p.add_option('--silence-missing-directive-warnings', action='callback',
                      callback=handleSilenceDirectiveWarnings,
                      help='Don\'t emit warnings about missing directives')
         return p
 
@@ -678,17 +661,17 @@ class Preprocessor:
             if v in self.context:
                 return str(self.context[v])
             return ''
         for i in range(1, len(lst), 2):
             lst[i] = vsubst(lst[i])
         lst.append('\n') # add back the newline
         self.write(reduce(lambda x, y: x+y, lst, ''))
     def do_literal(self, args):
-        self.write(args + self.LE)
+        self.write(args + '\n')
     def do_filter(self, args):
         filters = [f for f in args.split(' ') if hasattr(self, 'filter_' + f)]
         if len(filters) == 0:
             return
         current = dict(self.filters)
         for f in filters:
             current[f] = getattr(self, 'filter_' + f)
         filterNames = current.keys()
@@ -791,19 +774,18 @@ class Preprocessor:
         args = self.filter_substitution(args)
         self.do_include(args)
     def do_error(self, args):
         raise Preprocessor.Error(self, 'Error: ', str(args))
 
 
 def preprocess(includes=[sys.stdin], defines={},
                output = sys.stdout,
-               line_endings='\n', marker='#'):
-    pp = Preprocessor(line_endings=line_endings,
-                      defines=defines,
+               marker='#'):
+    pp = Preprocessor(defines=defines,
                       marker=marker)
     for f in includes:
         with open(f, 'rU') as input:
             pp.processFile(input=input, output=output)
 
 
 # Keep this module independently executable.
 if __name__ == "__main__":
--- a/python/mozbuild/mozbuild/test/test_preprocessor.py
+++ b/python/mozbuild/mozbuild/test/test_preprocessor.py
@@ -435,22 +435,16 @@ class TestPreprocessor(unittest.TestCase
         self.do_include_pass([
             '#ifdef LINE',
             'PASS',
             '#else',
             'FAIL',
             '#endif',
         ])
 
-    def test_lineEndings(self):
-        with MockedOpen({'f': 'first\n#literal second\n'}):
-            self.pp.setLineEndings('cr')
-            self.pp.do_include('f')
-            self.assertEqual(self.pp.out.getvalue(), "first\rsecond\r")
-
     def test_filterDefine(self):
         self.do_include_pass([
             '#filter substitution',
             '#define VAR AS',
             '#define VAR2 P@VAR@',
             '@VAR2@S',
         ])