Last Comment Bug 725199 - include build machine name in about:buildconfig
: include build machine name in about:buildconfig
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla13
Assigned To: John Ford [:jhford]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-07 19:48 PST by John Ford [:jhford]
Modified: 2012-02-09 10:16 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add build machine hostname to the about:config box (792 bytes, patch)
2012-02-07 21:17 PST, John Ford [:jhford]
no flags Details | Diff | Splinter Review
silly typos (791 bytes, patch)
2012-02-07 21:23 PST, John Ford [:jhford]
ted: review+
Details | Diff | Splinter Review

Description John Ford [:jhford] 2012-02-07 19:48:55 PST
It'd be cool, at least for me, to have the non-qualified hostname for the machine that built firefox available in about:buildconfig.  I'll try to write up a patch at some point in the future.
Comment 1 John Ford [:jhford] 2012-02-07 21:17:37 PST
Created attachment 595310 [details] [diff] [review]
add build machine hostname to the about:config box

Adding this information makes life easier for me, especially when dealing with updates on new platforms.
Comment 2 John Ford [:jhford] 2012-02-07 21:23:20 PST
Created attachment 595311 [details] [diff] [review]
silly typos

I had originally had just "<p>Built by "@BUILD_HOSTNAME@"", but forgot to clean up a quote when switching to an h2.

this results in:

<body class="aboutPageWideContainer">
<h1>about:buildconfig</h1>
<h2>Build Machine</h2>
<p>jhford-wifi</p>
<h2>Source</h2>
<p>Built from <a href="http://hg.mozilla.org/mozilla-central/rev/b45785802731">http://hg.mozilla.org/mozilla-central/rev/b45785802731</a></p>
<h2>Build platform</h2>
Comment 3 Justin Wood (:Callek) 2012-02-07 22:34:26 PST
Comment on attachment 595311 [details] [diff] [review]
silly typos

A few reasons for not doing this:
* Its unlikely useful for 99.999% of our users, while at least about:buildconfig has actual use elsewhere.
* A given build can happen on multiple machines, (build on one, l10n repack on another, etc.)
* Local developers who wish to share a build may expose their local hostname to people, which could be a security risk in some circumstances (imo.)

Opinion on the approach:
* I think if we do it, we should use ifdef MOZILLA_OFFICIAL for even showing this
* also change it from "Build Machine" to "Built on @VENDOR_NAME@'s Machine"
* Instead of doing BUILD_HOSTNAME as a shell expansion in this makefile (which might get called multiple times per build) how about generating the value in configure and using it directly (it is unlikely to change from one build to another to make caching of autoconf.mk still work fine)

I'm r-'ing for the actual error of not closing your ifdef in toolkit/content/buildconfig.html

I'll leave the other points up to you/ted though, since I'm not technically a reviewer here.
Comment 4 Ted Mielczarek [:ted.mielczarek] 2012-02-08 04:56:31 PST
Comment on attachment 595311 [details] [diff] [review]
silly typos

Review of attachment 595311 [details] [diff] [review]:
-----------------------------------------------------------------

This seems useful.

::: toolkit/content/Makefile.in
@@ +77,4 @@
>  DEFINES += -DSOURCE_REPO="$(SOURCE_REPO)"
>  endif
>  
> +BUILD_HOSTNAME := $(shell hostname)

I'd actually just use = here, so this only gets evaluated on-demand. (:= is better when you're going to use the variable value multiple times, but here you're only using it once anyway.)

::: toolkit/content/buildconfig.html
@@ +58,5 @@
>  <body class="aboutPageWideContainer">
>  <h1>about:buildconfig</h1>
> +#ifdef BUILD_HOSTNAME
> +<h2>Build Machine</h2>
> +<p>@BUILD_HOSTNAME@</p>

You forgot an #endif here.
Comment 5 Ed Morley [:emorley] 2012-02-09 10:16:35 PST
https://hg.mozilla.org/mozilla-central/rev/8d88f07c034f

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