making it easier to use system jinja, ply

[Paweł Hajdan, Jr.]

Hello Blink developers,

I'd like to ask for your opinion about a change that would simplify using system python libraries e.g. for Linux packaging, and shouldn't have measurable impact on you or any other Chrome developers.

This is the patch that is needed to build with system jinja and ply: http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/www-client/chromium/files/chromium-system-jinja-r6.patch?revision=1.1

The entire patch is not really upstreamable, but I'd like you to consider the following part:

--- third_party/WebKit/Source/bindings/core/v8/generated.gyp.orig	2014-08-19 10:15:53.874850750 +0000
+++ third_party/WebKit/Source/bindings/core/v8/generated.gyp	2014-08-19 10:16:04.163050746 +0000
@@ -80,7 +80,6 @@
       # Update that regex if command line changes (other than changing flags)
       'action': [
         'python',
-        '-S',  # skip 'import site' to speed up startup
         '<(bindings_scripts_dir)/idl_compiler.py',
         '--cache-dir',
         '<(bindings_scripts_output_dir)',
--- third_party/WebKit/Source/bindings/modules/v8/generated.gyp.orig	2014-08-19 10:17:07.340279760 +0000
+++ third_party/WebKit/Source/bindings/modules/v8/generated.gyp	2014-08-19 10:17:13.556400768 +0000
@@ -68,7 +68,6 @@
       # Update that regex if command line changes (other than changing flags)
       'action': [
         'python',
-        '-S',  # skip 'import site' to speed up startup
         '<(bindings_scripts_dir)/idl_compiler.py',
         '--cache-dir',
         '<(bindings_scripts_output_dir)',

I'm not aware of python -S usage in other places, and it doesn't seem to have measurable impact on build time. On my z620 workstation with ssd, clobber build times are:

for no changes (src@291031; time ninja -C out/Release chrome)

real 15m11.854s

user 464m18.860s

sys 22m9.609s

real 15m0.480s

user 465m30.646s

sys 22m31.609s

real 15m4.285s

user 464m41.670s

sys 22m27.069s

-S removed:

real 14m47.716s

user 464m48.964s

sys 22m38.165s

real 15m24.748s

user 463m40.079s

sys 22m24.327s

real 15m0.118s

user 465m15.053s

sys 22m17.451s

Please let me know if you'd like to see some other type of build benchmark for this. It's not a big deal, distros can have these two additional chunks in the patch, I'm just concern they increase chance of hitting a conflict at some point.

The bottom line is I think removing -S from python invocation should be no-op for Blink developers, help packagers and also help standardize the codebase (I'm not aware of -S being used elsewhere).

Paweł

[Adam Barth]

As we've discussed before, we're not willing to make any changes to Blink to facilitate Linux packaging.

Adam

[Paweł Hajdan, Jr.]

Adam, could you explain rationale in this specific case?

I could find past discussion about WebSQL, https://groups.google.com/a/chromium.org/d/msg/blink-dev/5M2eX538AB8/fQZfuDR31aUJ . There are specific reasons listed there, and I accepted these reasons. I couldn't find a blanket rejection of any changes related to Linux packaging. In fact, for Chromium a solution that works for everyone has been found, i.e. build/linux/unbundle. This was made possible by using specific technical reasons to guide the discussion.

What's wrong with changing the way python is invoked during build process from "python -S" to "python" if it doesn't have measurable impact on build time? These are the only two places where -S is used inside Blink:

$ git grep '\-S' -- '*gyp*'

Source/bindings/core/v8/generated.gyp:        '-S',  # skip 'import site' to speed up startup

Source/bindings/modules/v8/generated.gyp:        '-S',  # skip 'import site' to speed up startup

Paweł

[Adam Barth]

As a project, we need to decide what configuration matrix we support.  The Blink project does not support Linux packaging as a configuration.  For that reason, we're not willing to make any changes to the codebase to support that configuration.

Blink has an open source license, which means you're welcome to take the code and do with it as you please, but that doesn't mean we need to support arbitrary configurations in trunk.

Adam

[Dirk Pranke]

While I agree with Adam that supporting system python libraries for linux packaging is not a particular goal of Blink, and we shouldn't go out of our way to support that configuration, I'd actually support this particular change and would approve a CL to remove it.

It looks like an unnecessary optimization to me, and frankly I didn't even know what the -S flag did, so it's not very common :). In addition it actually reduces the variety of ways we invoke Python (as Paweł says, these are the only two invocations that use -S). So the change seems to make sense on general code health grounds alone.

Adam, does that make sense?

-- Dirk

[Adam Barth]

On Thu Aug 28 2014 at 10:12:38 AM Dirk Pranke <dpr...@chromium.org> wrote:

While I agree with Adam that supporting system python libraries for linux packaging is not a particular goal of Blink, and we shouldn't go out of our way to support that configuration, I'd actually support this particular change and would approve a CL to remove it.

It looks like an unnecessary optimization to me, and frankly I didn't even know what the -S flag did, so it's not very common :). In addition it actually reduces the variety of ways we invoke Python (as Paweł says, these are the only two invocations that use -S). So the change seems to make sense on general code health grounds alone.

Adam, does that make sense?

If we're trying to achieve consistency, I would argue that we should add -S to all our invocations of Python instead.

Adam

[Dirk Pranke]

On Thu, Aug 28, 2014 at 10:17 AM, Adam Barth <aba...@chromium.org> wrote:

On Thu Aug 28 2014 at 10:12:38 AM Dirk Pranke <dpr...@chromium.org> wrote:

While I agree with Adam that supporting system python libraries for linux packaging is not a particular goal of Blink, and we shouldn't go out of our way to support that configuration, I'd actually support this particular change and would approve a CL to remove it.

It looks like an unnecessary optimization to me, and frankly I didn't even know what the -S flag did, so it's not very common :). In addition it actually reduces the variety of ways we invoke Python (as Paweł says, these are the only two invocations that use -S). So the change seems to make sense on general code health grounds alone.

Adam, does that make sense?

If we're trying to achieve consistency, I would argue that we should add -S to all our invocations of Python instead.

Why? Assuming Paweł's benchmarks are right, it appears to be an obscure feature that makes no difference in our build times? 

I could see any argument that it gets us better build hermeticity, but I'd want to explore that goal a lot more before agreeing (especially since most of the rest of the world seems to want to get python hermeticity through things like virtualenv instead).

(Though at this point even debating this further probably costs more than removing the flag is worth in improved code health; I don't feel that strongly about it ...).

-- Dirk

[Torne (Richard Coles)]

Given the ways that site is used by Linux distros and the ways that packaging of python and add-on python modules differ between systems, my experience is that using -S means that instead of getting a user's random potentially weird/broken python installation, you get an arbitrary *subset* of the user's random potentially weird/broken python installation instead, which may well be considerably more broken than what you started with :)

(Since anything that is on the system's default sys.path will still be there, but things added dynamically by site will be gone)

If we actually wanted to deal with python install nonuniformity we should have a virtualenv or a completely hermetic python. I don't believe that -S actually helps anything other than giving you a potential startup time improvement if site.py happens to be slow on a given system.

[Eric Seidel]

We have a working hermetic python solution in the infra repository and should be using that approach in all of our chromium repositories. Please file bugs about any modules were missing and we can add the proper wheel/env support to chromium/blink.

[Nico Weber]

On Fri, Aug 29, 2014 at 10:04 PM, 'Eric Seidel' via blink-dev <blin...@chromium.org> wrote:

We have a working hermetic python solution in the infra repository and should be using that approach in all of our chromium repositories.

I'm not sure if we should. On OS X, I think the hermetic python doesn't support importing frameworks for system libraries (at least that used to be true), and doing that is required by the build.

On the other hand, on Windows I need to delete some python2.6 that's installed by a system service and automatically added to PATH all the time.