Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/cmd/godoc: start server before scanning $GOPATH #13278

Closed
thewhitetulip opened this issue Nov 16, 2015 · 14 comments

Comments

Projects
None yet
10 participants
@thewhitetulip
Copy link

commented Nov 16, 2015

When we run godoc, it doesn't show any notification that the server has started, typically when I need godoc up and running I am in a middle of a programming crisis and only the doc can save me, but sadly it doesn't print when it is ready, I have to wait for some time or keep refreshing the page in my browser 100 times before it actually is started. So it would be great if we print this message before the call to HandleAndServe function, a way to notify that the server is up and running

@bradfitz bradfitz changed the title godoc should print "starting on localhost:8000" x/tools/cmd/godoc: should print "starting on localhost:8000" Nov 16, 2015

@jnjackins

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2015

On my laptop, the server finishes starting faster than I can switch to a browser window and press return. Maybe there is another issue here?

@adg

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2015

@jnjackins

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2015

Is this not not covered by the -v flag?

$ godoc -v -http :8000
2015/11/17 16:08:58 Go Documentation Server
2015/11/17 16:08:58 version = devel +babdb38 Tue Nov 17 04:06:32 2015 +0000
2015/11/17 16:08:58 address = :8000
2015/11/17 16:08:58 goroot = /Users/jnj/src/go
2015/11/17 16:08:58 tabwidth = 4
2015/11/17 16:08:58 search index disabled
name space {
    /:
        gated(os(/Users/jnj/src/go), 20) /
    /lib/godoc:
        mapfs /
    /src:
        gated(os(/Users/jnj/src/go), 20) /src
        gated(os(/Users/jnj), 20) /src
}

There are a couple function calls between these logs and http.ListenAndServe, but they are in new goroutines.

@thewhitetulip

This comment has been minimized.

Copy link
Author

commented Nov 17, 2015

@jnjackins I guess there you misunderstood what I asked, i do not want to know each and every thing as in the -v flag, I just want to know when the server has started! just one line, starting server on localhost

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 28, 2015

I don't think godoc shoud print chatter unless asked (with -v). But it would be reasonable for godoc to start the server before scanning the entire $GOPATH. Then at least you can go to the page and get a "we're still indexing" or whatever the status is.

@rsc rsc changed the title x/tools/cmd/godoc: should print "starting on localhost:8000" x/tools/cmd/godoc: start server before scanning $GOPATH Dec 28, 2015

@rsc rsc added this to the Go1.7 milestone Dec 28, 2015

@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016

@quentinmit quentinmit modified the milestones: Unreleased, Go1.8 Oct 10, 2016

@quentinmit quentinmit added the NeedsFix label Oct 10, 2016

@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 11, 2016

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jun 28, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Aug 15, 2017

Change https://golang.org/cl/55930 mentions this issue: cmd/godoc: start http server before scanning packages

@gopherbot

This comment has been minimized.

Copy link

commented Dec 14, 2017

Change https://golang.org/cl/83975 mentions this issue: cmd/godoc: listen TCP port earlier

@agnivade

This comment has been minimized.

Copy link
Member

commented Jan 19, 2018

Looks like both the CLs were abandoned. I have a CL prepared which returns a proper message before the scan is complete, so that the webserver is responsive and the scan can continue in the background.

Will send it across shortly.

@gopherbot

This comment has been minimized.

Copy link

commented Jan 20, 2018

Change https://golang.org/cl/88695 mentions this issue: godoc: init corpus in a separate goroutine in http mode

@andybons

This comment has been minimized.

Copy link
Member

commented Feb 9, 2018

Reverted in golang.org/cl/93115

@andybons andybons reopened this Feb 9, 2018

@agnivade

This comment has been minimized.

Copy link
Member

commented Feb 9, 2018

Oops, so sorry about this. I can't remember why I did not run the tests. My bad.

Anyways, so the TestWeb is failing because the http server has started in the background and it is replying with the "Scan in progress. Please try after sometime" message. So the strings which the test is looking for is not there. Hence the failure.

I will send a CL with the proper fix.

On a side note, should we start running tests in try bots for this repo ?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 9, 2018

Trybots need to be kicked off by a human. They just weren't kicked off.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 10, 2018

Change https://golang.org/cl/93215 mentions this issue: Revert "Revert "godoc: init corpus in a separate goroutine in http mode""

@ALTree ALTree modified the milestones: Go1.10, Gccgo, Go1.11 Feb 10, 2018

@gopherbot

This comment has been minimized.

Copy link

commented Mar 27, 2018

Change https://golang.org/cl/102799 mentions this issue: [release-branch.go1.10] godoc: init corpus in a separate goroutine in http mode

gopherbot pushed a commit to golang/tools that referenced this issue Mar 27, 2018

[release-branch.go1.10] godoc: init corpus in a separate goroutine in…
… http mode

Currently, in http mode the server blocks until the corpus
has been initialized. This can cause considerable delay
if the user workspace is significantly large and the files
are not present in the buffer cache.

This CL spawns off the initialization in a separate goroutine
if httpMode is set and turns on a flag when it's done.
The http handler checks the flag and returns an error response
if it has not been set.

The check is only performed for the path prefixes handled by the
handlerServer struct. Other paths do not call the GetPageInfo() function
and hence can return immediately. This preserves maximum responsiveness
of the server.

Also adds an additional print statement in verbose mode

Note: This is a re-do of a previous CL golang.org/cl/88695 which was
incorrect committed without running tests. This CL fixes that test.

Fixes golang/go#13278

Change-Id: I80c801f32af007312090d3783a2ea2c6f92cad66
Reviewed-on: https://go-review.googlesource.com/93215
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 006ac43)
Reviewed-on: https://go-review.googlesource.com/102799
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>

@golang golang locked and limited conversation to collaborators Mar 27, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.