Bug 154271 - run-webkit-httpd should use webkitpy to run httpd.
Summary: run-webkit-httpd should use webkitpy to run httpd.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks: 154294
  Show dependency treegraph
 
Reported: 2016-02-15 17:15 PST by Jer Noble
Modified: 2016-02-19 18:35 PST (History)
5 users (show)

See Also:


Attachments
Patch (10.14 KB, patch)
2016-02-15 17:17 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (11.59 KB, patch)
2016-02-15 22:52 PST, Jer Noble
ap: review+
ap: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (11.16 KB, patch)
2016-02-16 11:24 PST, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2016-02-15 17:15:18 PST
run-webkit-httpd should use webkitpy to run httpd.
Comment 1 Jer Noble 2016-02-15 17:17:33 PST
Created attachment 271395 [details]
Patch
Comment 2 Alexey Proskuryakov 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.
Comment 3 Jer Noble 2016-02-15 21:19:08 PST
In my testing, we still were getting access and error logs to stdout.
Comment 4 Jer Noble 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.
Comment 5 Jer Noble 2016-02-15 22:52:52 PST
Created attachment 271419 [details]
Patch
Comment 6 Alexey Proskuryakov 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).
Comment 7 Jer Noble 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.
Comment 8 Jer Noble 2016-02-16 11:24:58 PST
Created attachment 271456 [details]
Patch for landing
Comment 9 WebKit Commit Bot 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>