Last Comment Bug 663180 - Make configure write mozinfo JSON data about build configuration for test harnesses to use
: Make configure write mozinfo JSON data about build configuration for test har...
Status: RESOLVED FIXED
fixed-in-bs
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Ted Mielczarek [:ted.mielczarek]
:
Mentors:
Depends on:
Blocks: 606524 664197
  Show dependency treegraph
 
Reported: 2011-06-09 12:03 PDT by Ted Mielczarek [:ted.mielczarek]
Modified: 2011-06-14 10:40 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
write $objdir/mozinfo.json during configure (7.31 KB, patch)
2011-06-09 12:06 PDT, Ted Mielczarek [:ted.mielczarek]
khuey: review+
k0scist: review+
Details | Diff | Review
write $objdir/mozinfo.json during configure, rev 2 (10.69 KB, patch)
2011-06-10 09:33 PDT, Ted Mielczarek [:ted.mielczarek]
k0scist: review+
Details | Diff | Review

Description Ted Mielczarek [:ted.mielczarek] 2011-06-09 12:03:30 PDT
configure should write out a JSON data file that test harnesses can pass to mozinfo to accurately reflect info about the current build configuration.
Comment 1 Ted Mielczarek [:ted.mielczarek] 2011-06-09 12:06:26 PDT
Created attachment 538310 [details] [diff] [review]
write $objdir/mozinfo.json during configure

This is going to require us to bump PYTHON_VERSION to 2.6, I'll file that later.

r?jhammel on the Python bits, and r?khuey on the build bits.
Comment 2 Jeff Hammel 2011-06-09 13:50:05 PDT
Comment on attachment 538310 [details] [diff] [review]
write $objdir/mozinfo.json during configure

+  def setUp(self):
+    self.tmpdir = mkdtemp()
+    self.f = os.path.join(self.tmpdir, "f")
+
+  def tearDown(self):
+    rmtree(self.tmpdir)

Since you only have one file, why make a temporary directory?
Shouldn't a temporary file suffice?

+      with open(self.f, 'r') as f:

The 'r' is implied

+    for r in required:
+        if r not in env:
+            raise Exception("Missing required environment variable
'%s'" % r)

You could report all of these at the same time, albeit there are only
two:

missing = [r for r in required if r not in env]
if missing:
    raise Exception("Missing...: %s" % ', '.join(missing))

+    if p in ["x86_64", "ppc64", "sparc64"]:
+        d["bits"] = 64
+    else:
+        d["bits"] = 32
Is this dangerous?  I guess not, mostly. I'd feel more comfortable if
we only set to 32 for known values, but maybe its not worth the
trouble.

+    with open(filename, "w") as f:

While `with` is a better pattern, iirc it doesn't work with older
python. I'd prefer to be as backwards-compatible as possible in case
we need to backport

+def write_json(filename, env=os.environ):
<snip/>
It might be nice to check if the filename is a file object, then just
use that.  Similarly
+        write_json(sys.argv[1])
could just use sys.stdout if len(sys.argv) == 1

+sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
Not a fan :(

Overall, looks good! r+

I'd prefer if, for the simple values we have, we just kept a template
(or templates) and populated that and dropped the json/python 2.6 dependency
entirely.
Comment 3 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2011-06-09 16:58:36 PDT
Comment on attachment 538310 [details] [diff] [review]
write $objdir/mozinfo.json during configure

>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>@@ -9251,6 +9251,12 @@
> 
> AC_OUTPUT($MAKEFILES)
> 
>+# Generate a JSON config file for unittest harnesses etc to read
>+# build configuration details from in a standardized way.
>+rm -f ./mozinfo.json
>+export OS_TARGET TARGET_CPU MOZ_DEBUG
>+$PYTHON ${_topsrcdir}/config/writemozinfo.py ./mozinfo.json
>+
> dnl Prevent the regeneration of cairo-features.h forcing rebuilds of gfx stuff
> if test "$CAIRO_FEATURES_H"; then
>   if cmp -s $CAIRO_FEATURES_H "$CAIRO_FEATURES_H".orig; then

I'm not thrilled about exporting the variables.  Can we pass them on the shell cmd?

OS_TARGET=$OS_TARGET ... $PYTHON ...

Also, if we're going to unconditionally rm -f mozinfo.json don't make anything depend on it in the build system!
Comment 4 Ted Mielczarek [:ted.mielczarek] 2011-06-10 04:41:08 PDT
(In reply to comment #2)
> +    if p in ["x86_64", "ppc64", "sparc64"]:
> +        d["bits"] = 64
> +    else:
> +        d["bits"] = 32
> Is this dangerous?  I guess not, mostly. I'd feel more comfortable if
> we only set to 32 for known values, but maybe its not worth the
> trouble.

Probably not, but I can make it a whitelist for both 32 and 64 so we'd just get unknown otherwise.

> 
> +    with open(filename, "w") as f:
> 
> While `with` is a better pattern, iirc it doesn't work with older
> python. I'd prefer to be as backwards-compatible as possible in case
> we need to backport

You can use with in 2.5, which we already require in the build system, so I don't think that's a problem.

> +sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
> Not a fan :(

Yeah, it's crappy, but it's just a unit test so I feel okay about it.

> I'd prefer if, for the simple values we have, we just kept a template
> (or templates) and populated that and dropped the json/python 2.6 dependency
> entirely.

Okay, I can do that, it's just slightly more of a pain.

(In reply to comment #3)
> I'm not thrilled about exporting the variables.  Can we pass them on the
> shell cmd?

Yeah, I'll fix that.

> Also, if we're going to unconditionally rm -f mozinfo.json don't make
> anything depend on it in the build system!

Right, I won't. If we need to, we can do the cmp trick like we do elsewhere to not modify it if it's unchanged.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2011-06-10 09:33:00 PDT
Created attachment 538538 [details] [diff] [review]
write $objdir/mozinfo.json during configure, rev 2

I ditched the json module here in favor of some pretty simplistic Python -> JSON serialization for the small set of value types I care about. I kept the with statements, since you can import them into Python 2.5. We require 2.5 in the build system already, so there's no point in worrying about older backwards compat there.

I added a few tests for some other things, I think I addressed all of your comments except for things I explicitly replied to above. jhammel: can you give this one more once-over?
Comment 6 Ted Mielczarek [:ted.mielczarek] 2011-06-10 09:35:28 PDT
Also I went ahead and addressed khuey's comment, doing the right thing with cmp in configure so we don't overwrite the file when it's unchanged.
Comment 7 Jeff Hammel 2011-06-10 09:36:12 PDT
Comment on attachment 538538 [details] [diff] [review]
write $objdir/mozinfo.json during configure, rev 2

looks great!
Comment 8 Ted Mielczarek [:ted.mielczarek] 2011-06-10 10:18:22 PDT
http://hg.mozilla.org/projects/build-system/rev/6d19baaa339f
Comment 9 Ted Mielczarek [:ted.mielczarek] 2011-06-13 06:17:17 PDT
http://hg.mozilla.org/mozilla-central/rev/6d19baaa339f

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