Last Comment Bug 754469 - Import blessings Python package into mozilla-central
: Import blessings Python package into mozilla-central
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Gregory Szorc [:gps]
:
:
Mentors:
Depends on:
Blocks: 777068
  Show dependency treegraph
 
Reported: 2012-05-11 15:15 PDT by Gregory Szorc [:gps]
Modified: 2012-07-24 14:07 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add blessings 1.3 (27.18 KB, patch)
2012-05-14 11:43 PDT, Gregory Szorc [:gps]
no flags Details | Diff | Splinter Review
Add blessings 1.3 as a package (80.93 KB, patch)
2012-05-14 17:48 PDT, Gregory Szorc [:gps]
no flags Details | Diff | Splinter Review

Description Gregory Szorc [:gps] 2012-05-11 15:15:23 PDT
Not sure if this is the right component, but I'm requesting that the Python package 'blessings' (http://pypi.python.org/pypi/blessings/1.3) be imported into mozilla-central. This package provides high-level terminal control to enable things like color, positioning, etc.

The driving force behind this request is that I want more user-friendly tools in the repository. Pretty terminal interaction is a way of providing more user-friendly tools. One such tool is in the pipes at bug 751795.

I'm not sure if we need to get legal sign-off or what when it comes to importing external code. So, I figured I can get that sign-off first (from Gerv or whomever) and then submit a patch for review.
Comment 1 Gervase Markham [:gerv] 2012-05-14 02:01:13 PDT
gps: no problems from a licensing point of view.

Gerv
Comment 2 Gregory Szorc [:gps] 2012-05-14 11:19:25 PDT
Gerv: rock on!

Ted, etc: what's the appropriate location for this? other-licenses/blessings?
Comment 3 Ted Mielczarek [:ted.mielczarek] 2012-05-14 11:24:53 PDT
That seems fine, there's precedent for that location (virtualenv, simplejson, ply).
Comment 4 Gervase Markham [:gerv] 2012-05-14 11:32:34 PDT
NO! Not other-licenses!

Read the README in other-licenses.

Let's kill this strange idea that other-licenses is for non-MPL things. It's for non-MPL-_compatible_ things.

Gerv
Comment 5 Gregory Szorc [:gps] 2012-05-14 11:43:17 PDT
Created attachment 623745 [details] [diff] [review]
Add blessings 1.3

Patch includes the original LICENSE file.

Attempting to sneak in the creation of a new top-level directory, pylib/, as a site-packages like directory for all Python (in a similar vein of bug 661908). In my utopia world, all (MPL-compatible) code would exist in one unified tree and would be easily locatable. Ted: if you don't agree, please tell me where else to put this, as I have no clue.
Comment 6 Gervase Markham [:gerv] 2012-05-14 11:46:29 PDT
Brendan signs off on new top-level directory additions.

Gerv
Comment 7 Gregory Szorc [:gps] 2012-05-14 11:57:38 PDT
Brendan: The context here is that currently we have a lot of Python support code scattered throughout the tree. The intention with creating a new top-level directory is to eventually unify all this code to one central location. This would make it easier to find and hack on. It may also lead to better code cohesion and quality.

How bad is it currently? Just do a `find . -name '*.py'`. The following directories (and more) all need to be searched for Python modules at any given time:

* build
* build/pymake
* config
* dom
* editor/libeditor/html/tests/browserscope/lib/richtext2
* ipc/ipdl
* modules/freetype2/src/tools
* testing/marionette
* testing/mochitest
* testing/mozbase/* (8 directories)
* testing/peptest
* testing/...
* xpcom
Comment 8 Ted Mielczarek [:ted.mielczarek] 2012-05-14 11:59:28 PDT
If other-licenses is contentious and we don't want a new top-level directory, feel free to stash it in build/pylib or build/python or whatever. I don't really care. Also, you don't really need review from me if you're just landing a Python module there that's NPOTB.
Comment 9 Gregory Szorc [:gps] 2012-05-14 12:30:46 PDT
To complicate things even more, we probably need two Python trees: 1 containing packages with all their metadata as downloaded from PyPI and 1 for pure source (like what is in build/ config/ today). The sub-tree containing wholesale packages would need build system integration (like bug 661908).

pylib/package-sources and pyblic/sources?
Comment 10 Ted Mielczarek [:ted.mielczarek] 2012-05-14 12:46:21 PDT
Fine with me, although I'd probably just say /packages. We were just talking about single-file-module installation into the virtualenv, it seems fairly simple.
Comment 11 Gregory Szorc [:gps] 2012-05-14 12:51:57 PDT
(In reply to Ted Mielczarek [:ted] from comment #10)
> Fine with me, although I'd probably just say /packages. We were just talking
> about single-file-module installation into the virtualenv, it seems fairly
> simple.

For some things, yes. I /could/ resubmit this patch containing all of the package metadata files. I stripped what I thought was needless.
Comment 12 Gregory Szorc [:gps] 2012-05-14 17:48:42 PDT
Created attachment 623888 [details] [diff] [review]
Add blessings 1.3 as a package

Here is the version as a full package (literally uncompressed the tarball from upstream). We would need to hook this up with the build system somehow (likely through bug 661908).
Comment 13 Gregory Szorc [:gps] 2012-06-01 10:59:01 PDT
I'll just create build/pylib for now. We can always create a new top-level directory at some later date if we ever do consolidate the Python code.

Try at https://tbpl.mozilla.org/?tree=Try&rev=b31d597f3a0e

Per comment #8, I don't think I need a review to land this. So, once Try comes back green, it's landing in inbound.
Comment 15 Gregory Szorc [:gps] 2012-06-01 12:52:23 PDT
Backed out because I was stupid and pushed while the OS X builders were still broken.
Comment 16 Gregory Szorc [:gps] 2012-06-02 01:46:46 PDT
Turns out it really was something with this patch. Changing the virtualenv install order fixed things, interestingly. Try build at https://tbpl.mozilla.org/?tree=Try&rev=4ce135abbf76 proves it.

So, I landed in inbound.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e03a1f27ce24
https://hg.mozilla.org/integration/mozilla-inbound/rev/9abc60f44fd5
Comment 18 :Ehsan Akhgari 2012-06-02 12:08:14 PDT
Backed out:

https://hg.mozilla.org/mozilla-central/rev/f9d66e743b93
https://hg.mozilla.org/mozilla-central/rev/75312a3da3a6

Please include the bug number in the backout commit message.  Thanks!

Note You need to log in before you can comment on or make changes to this bug.