Bug 47981 – [NRWT] Get child process number from environment variable

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Gabor Rapcsanyi]

Get the child process number from an environment variable if it exists.
It would be useful for buildbots.

[Gabor Rapcsanyi]

Created attachment 71284 [details]
proposed patch

[Csaba Osztrogonác]

(In reply to comment #1)
> Created an attachment (id=71284) [details]
> proposed patch

LGTM. ;)

[Ojan Vafai]

Comment on attachment 71284 [details]
proposed patch

I don't have a problem with this, but how is an environment variable easier than just passing something on the commandline?

[Csaba Osztrogonác]

(In reply to comment #3)
> (From update of attachment 71284 [details])
> I don't have a problem with this, but how is an environment variable easier than just passing something on the commandline?

If you would like to add/modify a commandline parameter on the buildbot, you have to modify the buildbot master config and ask Bill to update the buildbot master. But you can easily set an environment variable locally.

[Eric Seidel (no email)]

I guess.  Always feels strange to have environment variables for these sorts of things.

I assume the command-line wins out over the environment variable.

If you have concerns like this you could also have the buildbot run some wrapper script that you can control more easily.

[Gabor Rapcsanyi]

(In reply to comment #5)
> I guess.  Always feels strange to have environment variables for these sorts of things.
> 
> I assume the command-line wins out over the environment variable.
> 
> If you have concerns like this you could also have the buildbot run some wrapper script that you can control more easily.

Yes, the command-line is the primary.
Now we run all of our buildbots with different and custom environment and these kind of options are not port or platform specifics so we can't set them on the buildbot master.
The wrapper is not good because the buildbot master will "call" the original script not your wrapper. So I think that's the simplest way to set different child process number on buildbots like WEBKIT_WAIT_FOR_HTTPD in ORWT to set http lock and BUILD_WEBKIT_ARGS in build-webkit script.

[WebKit Commit Bot]

Comment on attachment 71284 [details]
proposed patch

Rejecting patch 71284 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 71284]" exit_code: 2
Cleaning working directory
Updating working directory
Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/4534020

[Eric Seidel (no email)]

Comment on attachment 71284 [details]
proposed patch

I think this was a false rejection from a run-away commit-node.

[Dirk Pranke]

Comment on attachment 71284 [details]
proposed patch

The patch looks good to me as well, although I'm not a reviewer. As with others, I'm not generally a fan of environmental variables, but it sounds like it is useful here. Ideally there'd be a comment next to this line in the code about *why* we're looking at the variable (since it's easily settable on the bots, unlike the command line)

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 71284 [details]:

transitions/transition-end-event-transform.html
http/tests/appcache/fail-on-update-2.html

Please file bugs against the tests.  The author(s) of the test(s) have been CCed on this bug.  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 71284 [details]
proposed patch

Clearing flags on attachment: 71284

Committed r70217: <http://trac.webkit.org/changeset/70217>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.