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

Jump to bottom

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

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

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

thewhitetulip opened this issue Nov 16, 2015 · 14 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@thewhitetulip
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

adg commented Nov 17, 2015 via email

@jnjackins
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

rsc 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 The path to resolution is known, but the work has not been done. 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
Copy link

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

@gopherbot
Copy link

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

@agnivade
Copy link
Contributor

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
Copy link

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

@andybons
Copy link
Member

andybons commented Feb 9, 2018

Reverted in golang.org/cl/93115

@andybons andybons reopened this Feb 9, 2018
@agnivade
Copy link
Contributor

agnivade 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
Copy link
Contributor

bradfitz commented Feb 9, 2018

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

@gopherbot
Copy link

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
Copy link

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
@agnivade @andybons
[release-branch.go1.10] godoc: init corpus in a separate goroutine in…
1396c68 
… 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.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
10 participants
@bradfitz @rsc @quentinmit @andybons @jnjackins @agnivade @ALTree @thewhitetulip @adg @gopherbot