From fdc1bb50da16208d67119c3327bd7039c4cc14f2 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 24 Feb 2016 15:37:41 +0100 Subject: [PATCH 1/4] simplify js build steps - run npm, bower install every time (except from sdist). Removes need to check npm, bower sources. - only check `built/index.js` for build target since webpack is only a single step now --- package.json | 2 +- setup.py | 4 +- setupbase.py | 101 +++++++++++++++------------------------------------ 3 files changed, 33 insertions(+), 74 deletions(-) diff --git a/package.json b/package.json index f99a80407..2ba8b0f5f 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "url": "https://github.com/jupyter/notebook.git" }, "scripts": { - "bower": "bower install", + "bower": "bower install --allow-root --config.interactive=false", "build:watch": "concurrent \"npm run build:css:watch\" \"npm run build:js:watch\"", "build": "npm run build:css && npm run build:js", "build:css": "python setup.py css", diff --git a/setup.py b/setup.py index 451ff9e56..ffe7cb3a3 100755 --- a/setup.py +++ b/setup.py @@ -55,7 +55,7 @@ from setupbase import ( check_package_data_first, CompileCSS, CompileJS, - Bower, + JavascriptDependencies, JavascriptVersion, css_js_prerelease, ) @@ -119,7 +119,7 @@ setup_args['cmdclass'] = { 'sdist' : css_js_prerelease(sdist, strict=True), 'css' : CompileCSS, 'js' : CompileJS, - 'jsdeps' : Bower, + 'jsdeps' : JavascriptDependencies, 'jsversion' : JavascriptVersion, } diff --git a/setupbase.py b/setupbase.py index 06068dee7..18a00e0d5 100644 --- a/setupbase.py +++ b/setupbase.py @@ -326,8 +326,8 @@ def run(cmd, *args, **kwargs): return check_call(cmd, *args, **kwargs) -class Bower(Command): - description = "fetch static client-side components with bower" +class JavascriptDependencies(Command): + description = "fetch static client-side components with npm and bower" user_options = [ ('force', 'f', "force fetching of bower dependencies"), @@ -342,45 +342,21 @@ class Bower(Command): bower_dir = pjoin(static, 'components') node_modules = pjoin(repo_root, 'node_modules') - def should_run(self): - if self.force: - return True - if not os.path.exists(self.bower_dir): - return True - return mtime(self.bower_dir) < mtime(pjoin(repo_root, 'bower.json')) - - def should_run_npm(self): - if not which('npm'): - print("npm unavailable", file=sys.stderr) - return False - if not os.path.exists(self.node_modules): - return True - return mtime(self.node_modules) < mtime(pjoin(repo_root, 'package.json')) - def run(self): - if not self.should_run(): - print("bower dependencies up to date") - return - - if self.should_run_npm(): - print("installing build dependencies with npm") - run(['npm', 'install','--progress=false'], cwd=repo_root) - os.utime(self.node_modules, None) - - env = os.environ.copy() - env['PATH'] = npm_path + try: + run(['npm', 'install', '--progress=false'], cwd=repo_root) + except OSError as e: + print("Failed to run `npm install`: %s" % e, file=sys.stderr) + print("npm is required to build a development version of the notebook.", file=sys.stderr) + raise try: - run( - ['bower', 'install', '--allow-root', '--config.interactive=false'], - cwd=repo_root, - env=env - ) - except OSError as e: - print("Failed to run bower: %s" % e, file=sys.stderr) + run(['npm', 'run', 'bower'], cwd=repo_root) + except Exception as e: + print("Failed to run `npm run bower`: %s" % e, file=sys.stderr) print("You can install js dependencies with `npm install`", file=sys.stderr) raise - os.utime(self.bower_dir, None) + # update package data in case this created new files update_package_data(self.distribution) @@ -444,56 +420,39 @@ class CompileJS(Command): def finalize_options(self): self.force = bool(self.force) - apps = ['notebook', 'tree', 'edit', 'terminal', 'auth'] - targets = [ pjoin(static, app, 'js', 'built', 'main.min.js') for app in apps ] + target = pjoin(static, 'built', 'index.js') + targets = [target] - def sources(self, name): + def sources(self): """Generator yielding .js sources that an application depends on""" - yield pjoin(static, name, 'js', 'built', 'main.min.js') - - for sec in [name, 'base', 'auth']: - for f in glob(pjoin(static, sec, 'js', '*.js')): - if not f.endswith('.min.js'): - yield f - yield pjoin(static, 'services', 'built', 'config.js') - yield pjoin(static, 'built', 'index.js') - if name == 'notebook': - for f in glob(pjoin(static, 'services', '*', '*.js')): - yield f - for parent, dirs, files in os.walk(pjoin(static, 'components')): - if os.path.basename(parent) == 'MathJax': + yield pjoin(repo_root, 'package.json') + yield pjoin(repo_root, 'webpack.config.js') + for parent, dirs, files in os.walk(static): + if os.path.basename(parent) in {'MathJax', 'built'}: # don't look in MathJax, since it takes forever to walk it + # also don't look at build targets as sources dirs[:] = [] continue for f in files: + if not f.endswith('.js'): + continue yield pjoin(parent, f) - def should_run(self, name, target): - if self.force or not os.path.exists(target): + def should_run(self): + if self.force or not os.path.exists(self.target): + print("Missing %s" % self.target) return True - target_mtime = mtime(target) - for source in self.sources(name): + target_mtime = mtime(self.target) + for source in self.sources(): if mtime(source) > target_mtime: - print(source, target) + print('%s > %s' % (source, self.target)) return True return False - - def build_main(self, name): - """Build main.min.js""" - target = pjoin(static, name, 'js', 'built', 'main.min.js') - - if not self.should_run(name, target): - log.info("%s up to date" % target) - return - log.info("Rebuilding %s" % target) - run(['npm', 'run', 'build']) def run(self): self.run_command('jsdeps') - env = os.environ.copy() - env['PATH'] = npm_path - pool = ThreadPool() - pool.map(self.build_main, self.apps) + if self.should_run(): + run(['npm', 'run', 'build:js']) # update package data in case this created new files update_package_data(self.distribution) From d7a0aba84843c9fb61fa397af183e7b6470bc75a Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 24 Feb 2016 15:43:17 +0100 Subject: [PATCH 2/4] only write file in jsversion if there's a change to make avoids updating the file's mtime unnecessarily, forcing a rebuild --- setupbase.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/setupbase.py b/setupbase.py index 18a00e0d5..2bb9c6a5a 100644 --- a/setupbase.py +++ b/setupbase.py @@ -470,17 +470,27 @@ class JavascriptVersion(Command): def run(self): nsfile = pjoin(repo_root, "notebook", "static", "base", "js", "namespace.js") + lines = [] + found = False with open(nsfile) as f: - lines = f.readlines() - with open(nsfile, 'w') as f: - found = False - for line in lines: + for line in f.readlines(): if line.strip().startswith("Jupyter.version"): - line = ' Jupyter.version = "{0}";\n'.format(version) found = True + new_line = ' Jupyter.version = "{0}";\n'.format(version) + if new_line == line: + # no change, don't rewrite file + return + lines.append(new_line) + else: + lines.append(line) + + if not found: + raise RuntimeError("Didn't find Jupyter.version line in %s" % nsfile) + + print("Writing version=%s to %s" % (version, nsfile)) + with open(nsfile, 'w') as f: + for line in lines: f.write(line) - if not found: - raise RuntimeError("Didn't find Jupyter.version line in %s" % nsfile) def css_js_prerelease(command, strict=False): From 67b54ffbec65bf8c5922f66c095a867e32abfda3 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 24 Feb 2016 16:02:50 +0100 Subject: [PATCH 3/4] clarify jsdeps command description it's js dependencies (both build and run) --- setupbase.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setupbase.py b/setupbase.py index 2bb9c6a5a..90a2bab0f 100644 --- a/setupbase.py +++ b/setupbase.py @@ -327,10 +327,10 @@ def run(cmd, *args, **kwargs): class JavascriptDependencies(Command): - description = "fetch static client-side components with npm and bower" + description = "Fetch Javascript dependencies with npm and bower" user_options = [ - ('force', 'f', "force fetching of bower dependencies"), + ('force', 'f', "Force fetching of Javascript dependencies"), ] def initialize_options(self): From 70367e77e36d6b2924eef87ff00cd30d9b2c47eb Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 24 Feb 2016 16:05:09 +0100 Subject: [PATCH 4/4] remove unused force option from js dependencies --- setupbase.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/setupbase.py b/setupbase.py index 90a2bab0f..a4ad091d5 100644 --- a/setupbase.py +++ b/setupbase.py @@ -329,15 +329,11 @@ def run(cmd, *args, **kwargs): class JavascriptDependencies(Command): description = "Fetch Javascript dependencies with npm and bower" - user_options = [ - ('force', 'f', "Force fetching of Javascript dependencies"), - ] - def initialize_options(self): - self.force = False + pass def finalize_options(self): - self.force = bool(self.force) + pass bower_dir = pjoin(static, 'components') node_modules = pjoin(repo_root, 'node_modules') @@ -501,7 +497,7 @@ def css_js_prerelease(command, strict=False): jsdeps = self.distribution.get_command_obj('jsdeps') js = self.distribution.get_command_obj('js') css = self.distribution.get_command_obj('css') - jsdeps.force = js.force = strict + js.force = strict targets = [ jsdeps.bower_dir ] targets.extend(js.targets)