663180 - Make configure write mozinfo JSON data about build configuration for test harnesses to use

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

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

[(not currently active) Ted Mielczarek]

Attachment: 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]

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 (Exited; not receiving bugmail, old account, do not use)]

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

[(not currently active) Ted Mielczarek]

(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

[(not currently active) Ted Mielczarek]

Attachment: 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

[(not currently active) Ted Mielczarek]

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]

Comment on attachment 538538 [details] [diff] [review]
write $objdir/mozinfo.json during configure, rev 2

looks great!

Comment 8

[(not currently active) Ted Mielczarek]

http://hg.mozilla.org/projects/build-system/rev/6d19baaa339f

Comment 9

[(not currently active) Ted Mielczarek]

http://hg.mozilla.org/mozilla-central/rev/6d19baaa339f