← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rharding/launchpad/use_lpjsmin into lp:launchpad

 

Richard Harding has proposed merging lp:~rharding/launchpad/use_lpjsmin into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rharding/launchpad/use_lpjsmin/+merge/94054

= Summary =
We want to pull out the JS minification into a new library. This allows us to
share the code and to not depend on the working tree for build tools. This
will help us implement the auto minifying daemon we want to replace make
jsbuild while developing.

== Proposed fix ==
The lpjsmin package has been created and upload to pypi. We're adding that as
a dependency and have submitted it to the lp-source-dependencies.

We then replace the instances of jsmin.py and jsmin_all.py by calling to the
bin/lpjsmin script.

== Implementation details ==
We remove the original scripts from utilities/js. 


== Demo and Q/A ==
Things should just work per usual. The launchpad.js file shold be minified
when in QA/production and the build/js/lp directory should still include both
unminified and minified versions of files.
-- 
https://code.launchpad.net/~rharding/launchpad/use_lpjsmin/+merge/94054
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/use_lpjsmin into lp:launchpad.
=== modified file 'Makefile'
--- Makefile	2012-02-17 17:33:04 +0000
+++ Makefile	2012-02-21 20:59:18 +0000
@@ -60,7 +60,7 @@
     bin/fl-credential-ctl bin/fl-install-demo bin/fl-monitor-ctl \
     bin/fl-record bin/fl-run-bench bin/fl-run-test bin/googletestservice \
     bin/i18ncompile bin/i18nextract bin/i18nmergeall bin/i18nstats \
-    bin/harness bin/iharness bin/ipy bin/jsbuild \
+    bin/harness bin/iharness bin/ipy bin/jsbuild bin/lpjsmin\
     bin/killservice bin/kill-test-services bin/lint.sh bin/retest \
     bin/run bin/run-testapp bin/sprite-util bin/start_librarian bin/stxdocs \
     bin/tags bin/test bin/tracereport bin/twistd bin/update-download-cache
@@ -185,7 +185,7 @@
 
 $(JS_OUT): $(JS_ALL)
 ifeq ($(JS_BUILD), min)
-	cat $^ | $(PY) -m lp.scripts.utilities.js.jsmin > $@
+	cat $^ | bin/lpjsmin > $@
 else
 	awk 'FNR == 1 {print "/* " FILENAME " */"} {print}' $^ > $@
 endif

=== modified file 'buildout-templates/bin/combo-rootdir.in'
--- buildout-templates/bin/combo-rootdir.in	2012-02-17 18:55:41 +0000
+++ buildout-templates/bin/combo-rootdir.in	2012-02-21 20:59:18 +0000
@@ -33,4 +33,4 @@
 find $BUILD_DIR/lp -name 'tests' -type d | xargs rm -rf
 
 # Minify our JS.
-bin/py -m lp.scripts.utilities.js.jsmin_all $BUILD_DIR/lp
+bin/lpjsmin -p $BUILD_DIR/lp

=== modified file 'buildout.cfg'
--- buildout.cfg	2012-02-02 06:01:27 +0000
+++ buildout.cfg	2012-02-21 20:59:18 +0000
@@ -67,6 +67,7 @@
 eggs = lp
     funkload
     zc.zservertracelog
+    lpjsmin
 # XXX gary 2009-5-12 bug 375751:
 # Make mailman built and installed in a more normal way.
 extra-paths =

=== removed file 'lib/lp/scripts/utilities/js/jsmin.py'
--- lib/lp/scripts/utilities/js/jsmin.py	2011-12-19 23:38:16 +0000
+++ lib/lp/scripts/utilities/js/jsmin.py	1970-01-01 00:00:00 +0000
@@ -1,219 +0,0 @@
-#!/usr/bin/python
-
-# This code is original from jsmin by Douglas Crockford, it was translated to
-# Python by Baruch Even. The original code had the following copyright and
-# license.
-#
-# /* jsmin.c
-#    2007-05-22
-#
-# Copyright (c) 2002 Douglas Crockford  (www.crockford.com)
-#
-# Permission is hereby granted, free of charge, to any person obtaining a copy of
-# this software and associated documentation files (the "Software"), to deal in
-# the Software without restriction, including without limitation the rights to
-# use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
-# of the Software, and to permit persons to whom the Software is furnished to do
-# so, subject to the following conditions:
-#
-# The above copyright notice and this permission notice shall be included in all
-# copies or substantial portions of the Software.
-#
-# The Software shall be used for Good, not Evil.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-# SOFTWARE.
-# */
-
-from StringIO import StringIO
-
-
-def jsmin(js):
-    ins = StringIO(js)
-    outs = StringIO()
-    JavascriptMinify().minify(ins, outs)
-    str = outs.getvalue()
-    if len(str) > 0 and str[0] == '\n':
-        str = str[1:]
-    return str
-
-def isAlphanum(c):
-    """return true if the character is a letter, digit, underscore,
-           dollar sign, or non-ASCII character.
-    """
-    return ((c >= 'a' and c <= 'z') or (c >= '0' and c <= '9') or
-            (c >= 'A' and c <= 'Z') or c == '_' or c == '$' or c == '\\' or (c is not None and ord(c) > 126));
-
-class UnterminatedComment(Exception):
-    pass
-
-class UnterminatedStringLiteral(Exception):
-    pass
-
-class UnterminatedRegularExpression(Exception):
-    pass
-
-class JavascriptMinify(object):
-
-    def _outA(self):
-        self.outstream.write(self.theA)
-    def _outB(self):
-        self.outstream.write(self.theB)
-
-    def _get(self):
-        """return the next character from stdin. Watch out for lookahead. If
-           the character is a control character, translate it to a space or
-           linefeed.
-        """
-        c = self.theLookahead
-        self.theLookahead = None
-        if c == None:
-            c = self.instream.read(1)
-        if c >= ' ' or c == '\n':
-            return c
-        if c == '': # EOF
-            return '\000'
-        if c == '\r':
-            return '\n'
-        return ' '
-
-    def _peek(self):
-        self.theLookahead = self._get()
-        return self.theLookahead
-
-    def _next(self):
-        """get the next character, excluding comments. peek() is used to see
-           if an unescaped '/' is followed by a '/' or '*'.
-        """
-        c = self._get()
-        if c == '/' and self.theA != '\\':
-            p = self._peek()
-            if p == '/':
-                c = self._get()
-                while c > '\n':
-                    c = self._get()
-                return c
-            if p == '*':
-                c = self._get()
-                while 1:
-                    c = self._get()
-                    if c == '*':
-                        if self._peek() == '/':
-                            self._get()
-                            return ' '
-                    if c == '\000':
-                        raise UnterminatedComment()
-
-        return c
-
-    def _action(self, action):
-        """do something! What you do is determined by the argument:
-           1   Output A. Copy B to A. Get the next B.
-           2   Copy B to A. Get the next B. (Delete A).
-           3   Get the next B. (Delete B).
-           action treats a string as a single character. Wow!
-           action recognizes a regular expression if it is preceded by ( or , or =.
-        """
-        if action <= 1:
-            self._outA()
-
-        if action <= 2:
-            self.theA = self.theB
-            if self.theA == "'" or self.theA == '"':
-                while 1:
-                    self._outA()
-                    self.theA = self._get()
-                    if self.theA == self.theB:
-                        break
-                    if self.theA <= '\n':
-                        raise UnterminatedStringLiteral()
-                    if self.theA == '\\':
-                        self._outA()
-                        self.theA = self._get()
-
-
-        if action <= 3:
-            self.theB = self._next()
-            if self.theB == '/' and (self.theA == '(' or self.theA == ',' or
-                                     self.theA == '=' or self.theA == ':' or
-                                     self.theA == '[' or self.theA == '?' or
-                                     self.theA == '!' or self.theA == '&' or
-                                     self.theA == '|' or self.theA == ';' or
-                                     self.theA == '{' or self.theA == '}' or
-                                     self.theA == '\n'):
-                self._outA()
-                self._outB()
-                while 1:
-                    self.theA = self._get()
-                    if self.theA == '/':
-                        break
-                    elif self.theA == '\\':
-                        self._outA()
-                        self.theA = self._get()
-                    elif self.theA <= '\n':
-                        raise UnterminatedRegularExpression()
-                    self._outA()
-                self.theB = self._next()
-
-
-    def _jsmin(self):
-        """Copy the input to the output, deleting the characters which are
-           insignificant to JavaScript. Comments will be removed. Tabs will be
-           replaced with spaces. Carriage returns will be replaced with linefeeds.
-           Most spaces and linefeeds will be removed.
-        """
-        self.theA = '\n'
-        self._action(3)
-
-        while self.theA != '\000':
-            if self.theA == ' ':
-                if isAlphanum(self.theB):
-                    self._action(1)
-                else:
-                    self._action(2)
-            elif self.theA == '\n':
-                if self.theB in ['{', '[', '(', '+', '-']:
-                    self._action(1)
-                elif self.theB == ' ':
-                    self._action(3)
-                else:
-                    if isAlphanum(self.theB):
-                        self._action(1)
-                    else:
-                        self._action(2)
-            else:
-                if self.theB == ' ':
-                    if isAlphanum(self.theA):
-                        self._action(1)
-                    else:
-                        self._action(3)
-                elif self.theB == '\n':
-                    if self.theA in ['}', ']', ')', '+', '-', '"', '\'']:
-                        self._action(1)
-                    else:
-                        if isAlphanum(self.theA):
-                            self._action(1)
-                        else:
-                            self._action(3)
-                else:
-                    self._action(1)
-
-    def minify(self, instream, outstream):
-        self.instream = instream
-        self.outstream = outstream
-        self.theA = '\n'
-        self.theB = None
-        self.theLookahead = None
-
-        self._jsmin()
-        self.instream.close()
-
-if __name__ == '__main__':
-    import sys
-    jsm = JavascriptMinify()
-    jsm.minify(sys.stdin, sys.stdout)

=== removed file 'lib/lp/scripts/utilities/js/jsmin_all.py'
--- lib/lp/scripts/utilities/js/jsmin_all.py	2012-01-13 14:53:03 +0000
+++ lib/lp/scripts/utilities/js/jsmin_all.py	1970-01-01 00:00:00 +0000
@@ -1,46 +0,0 @@
-#!/usr/bin/env python
-
-"""Handle minifying all javascript files in the build directory by walking
-
-$ jsmin_all.py $lp_js_root
-
-"""
-import os
-import re
-import sys
-from jsmin import JavascriptMinify
-
-
-def dirwalk(dir):
-    "walk a directory tree, using a generator"
-    for f in os.listdir(dir):
-        fullpath = os.path.join(dir,f)
-        if os.path.isdir(fullpath) and not os.path.islink(fullpath):
-            for x in dirwalk(fullpath):  # recurse into subdir
-                yield x
-        else:
-            yield fullpath
-
-
-def is_min(filename):
-    """Check if this file is alrady a minified file"""
-    return re.search("^(min).js$", filename)
-
-def minify(filename):
-    """Given a filename, handle minifying it as -min.js"""
-    if not is_min(filename):
-        new_filename = re.sub(".js$", "-min.js", filename)
-
-        with open(filename) as shrink_me:
-            with open(new_filename, 'w') as tobemin:
-                jsm = JavascriptMinify()
-                jsm.minify(shrink_me, tobemin)
-
-
-if __name__ == '__main__':
-    root = sys.argv[1]
-
-    if os.path.isfile(root):
-        minify(root)
-    else:
-        [minify(f) for f in dirwalk(root)]

=== modified file 'setup.py' (properties changed: -x to +x)
--- setup.py	2012-01-13 14:31:46 +0000
+++ setup.py	2012-02-21 20:59:18 +0000
@@ -50,6 +50,7 @@
         'lazr.smtptest',
         'lazr.testing',
         'lazr.uri',
+        'lpjsmin',
         # Required for launchpadlib
         'keyring',
         'manuel',

=== modified file 'versions.cfg'
--- versions.cfg	2012-02-20 22:26:24 +0000
+++ versions.cfg	2012-02-21 20:59:18 +0000
@@ -6,6 +6,7 @@
 
 ampoule = 0.2.0
 amqplib = 0.6.1
+argparse = 1.2.1
 BeautifulSoup = 3.1.0.1
 bson = 0.3.2
 bzr = 2.5.0dev2-r6152
@@ -42,6 +43,7 @@
 lazr.smtptest = 1.3
 lazr.testing = 0.1.1
 lazr.uri = 1.0.2
+lpjsmin = 0.4
 manuel = 1.1.1
 Markdown = 2.1.0
 martian = 0.11