RESOLVED FIXED Bug 154271
run-webkit-httpd should use webkitpy to run httpd.
https://bugs.webkit.org/show_bug.cgi?id=154271
Summary run-webkit-httpd should use webkitpy to run httpd.
Jer Noble
Reported 2016-02-15 17:15:18 PST
run-webkit-httpd should use webkitpy to run httpd.
Attachments
Patch (10.14 KB, patch)
2016-02-15 17:17 PST, Jer Noble
no flags
Patch (11.59 KB, patch)
2016-02-15 22:52 PST, Jer Noble
ap: review+
ap: commit-queue-
Patch for landing (11.16 KB, patch)
2016-02-16 11:24 PST, Jer Noble
no flags
Jer Noble
Comment 1 2016-02-15 17:17:33 PST
Alexey Proskuryakov
Comment 2 2016-02-15 19:04:28 PST
Comment on attachment 271395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271395&action=review > Tools/ChangeLog:3 > + run-webkit-httpd should use webkitpy to run httpd. Will we still be getting a standard output log with this? It is quite helpful for debugging.
Jer Noble
Comment 3 2016-02-15 21:19:08 PST
In my testing, we still were getting access and error logs to stdout.
Jer Noble
Comment 4 2016-02-15 21:23:53 PST
And now that I try it on another machine, it doesn't print to stdout. I'll add that to the patch.
Jer Noble
Comment 5 2016-02-15 22:52:52 PST
Alexey Proskuryakov
Comment 6 2016-02-16 09:15:23 PST
Comment on attachment 271419 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271419&action=review Should new-run-webkit-httpd be deleted? If bug 95256 is to be trusted, it should have been deleted as soon as chromium forked. > Tools/Scripts/run-webkit-httpd:70 > + # FIXME: is this the best way to handle unsupported port names? This comment is not clear to me. What are the downsides, and what are the alternatives? > Tools/Scripts/run-webkit-httpd:78 > + http_port = options.http_port if options.http_port is not None else "8000" > + if options.http_all_interfaces is not None: > + print "Starting httpd on port %s (all interfaces)" % http_port > + else: > + print "Starting httpd on <http://127.0.0.1:%s>" % http_port I don't think that this is accurate any more - when there is no explicitly specified port, we seem to listen on 8000, 8080 and 8443 (which is a good thing, as we can finally debug https tests with run-webkit-httpd).
Jer Noble
Comment 7 2016-02-16 10:52:56 PST
(In reply to comment #6) > Comment on attachment 271419 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271419&action=review > > Should new-run-webkit-httpd be deleted? If bug 95256 is to be trusted, it > should have been deleted as soon as chromium forked. I had no idea that was a thing. Probably? > > Tools/Scripts/run-webkit-httpd:70 > > + # FIXME: is this the best way to handle unsupported port names? > > This comment is not clear to me. What are the downsides, and what are the > alternatives? Copypasta from run-webkit-tests. Perhaps we should just use the default port, and not provide an option to run as another port. > > Tools/Scripts/run-webkit-httpd:78 > > + http_port = options.http_port if options.http_port is not None else "8000" > > + if options.http_all_interfaces is not None: > > + print "Starting httpd on port %s (all interfaces)" % http_port > > + else: > > + print "Starting httpd on <http://127.0.0.1:%s>" % http_port > > I don't think that this is accurate any more - when there is no explicitly > specified port, we seem to listen on 8000, 8080 and 8443 (which is a good > thing, as we can finally debug https tests with run-webkit-httpd). That's true, but there's no current mechanism to report back from the web server what interfaces and ports are currently bound. And for the option to bind to all interfaces, there may not be any possible mechanism for that. So, I'll add a fixme and a bug to improve the reporting about what ports the server is reachable at.
Jer Noble
Comment 8 2016-02-16 11:24:58 PST
Created attachment 271456 [details] Patch for landing
WebKit Commit Bot
Comment 9 2016-02-18 14:31:47 PST
Comment on attachment 271456 [details] Patch for landing Clearing flags on attachment: 271456 Committed r196778: <http://trac.webkit.org/changeset/196778>
Note You need to log in before you can comment on or make changes to this bug.