Support non-top level imports #14

prashantv · 2016-10-01T20:32:57Z

I have a very simply dummy program,

package main

import _ "go.uber.org/yarpc/transport"

func main() {
}

I have this in a completely clean GOPATH, and I run go get ., which should download all imports. This fails with:

> go get .
package go.uber.org/yarpc/transport: unrecognized import path "go.uber.org/yarpc/transport" (parse https://go.uber.org/yarpc/transport?go-get=1: no go-import meta tags)

This works fine for non-vanity imports.

The text was updated successfully, but these errors were encountered:

HelloGrayson · 2016-10-01T20:38:52Z

With the current approach, you must install an entire library before you can use subpackages.

I can think of 4 ways to add this type of support:

Explicitly list subpackages of a package in the sally.yaml - of course you'd only need to do this for subpackages you'd like to support installing directly. In most cases, I imagine the entire library should be installed before using subpackages.
Dynamically determine subpackages when generating static site - this has the benefit of being "complete", but can get stale just like item 1.
Change sally to a Go server that can accept dynamic requests that always serves the right html regardless of the URL. This is probably the biggest challenge since it means changing from Github Pages to something that can serve Go.
Update the go tool to resolve the root package to determine if its a vanity import or not, then crawl to the actual location of the subpackage

This isn't really an issue with YARPC or ThriftRW since we want you installing the root package in order to use the library. Is that different with Zap or UberFX? Trying to think of what type of libraries want you to start from a subpackage only. Perhaps item 1 is sufficient in that case.

prashantv · 2016-10-01T20:43:27Z

I think the solution is much simpler than that, we just need to return the same page for all URLs under the root package. The only difference should be the godoc page we direct to.

E.g., for grpc, the root:

$ curl -s google.golang.org/grpc?go-get=1
<!DOCTYPE html>
<html>
<head>
<meta name="go-import" content="google.golang.org/grpc git https://github.com/grpc/grpc-go">

<meta name="go-source" content="google.golang.org/grpc https://github.com/grpc/grpc-go https://github.com/grpc/grpc-go/tree/master{/dir} https://github.com/grpc/grpc-go/tree/master{/dir}/{file}#L{line}">

<meta http-equiv="refresh" content="0; url=https://godoc.org/google.golang.org/grpc">
</head>
<body>
Nothing to see here. Please <a href="https://godoc.org/google.golang.org/grpc">move along</a>.
</body>
</html>

For a subpackage that exists:

$ curl -s google.golang.org/grpc/transport?go-get=1
<!DOCTYPE html>
<html>
<head>
<meta name="go-import" content="google.golang.org/grpc git https://github.com/grpc/grpc-go">

<meta name="go-source" content="google.golang.org/grpc https://github.com/grpc/grpc-go https://github.com/grpc/grpc-go/tree/master{/dir} https://github.com/grpc/grpc-go/tree/master{/dir}/{file}#L{line}">

<meta http-equiv="refresh" content="0; url=https://godoc.org/google.golang.org/grpc/transport">
</head>
<body>
Nothing to see here. Please <a href="https://godoc.org/google.golang.org/grpc/transport">move along</a>.
</body>
</html>

And a subpackage that does not exist:

$ curl -s google.golang.org/grpc/transportdoesnotexist?go-get=1
<!DOCTYPE html>
<html>
<head>
<meta name="go-import" content="google.golang.org/grpc git https://github.com/grpc/grpc-go">

<meta name="go-source" content="google.golang.org/grpc https://github.com/grpc/grpc-go https://github.com/grpc/grpc-go/tree/master{/dir} https://github.com/grpc/grpc-go/tree/master{/dir}/{file}#L{line}">

<meta http-equiv="refresh" content="0; url=https://godoc.org/google.golang.org/grpc/transportdoesnotexist">
</head>
<body>
Nothing to see here. Please <a href="https://godoc.org/google.golang.org/grpc/transportdoesnotexist">move along</a>.
</body>
</html>

Note how the go-import and go-source meta tags are exactly the same in all cases.

HelloGrayson · 2016-10-01T20:45:48Z

@prashantv nice, except that Sally is a static site generator.

It might be that item 3 is the only way - let me research a bit.

Worse case I just rework sally and get go.uber.org on AWS - shouldn't be too big a deal.

HelloGrayson · 2016-10-01T22:59:40Z

OK - I've reworked Sally into an HTTP server in #15

This makes Sally behave the same as google.golang.org/grpc.

Non-existent package:

$ curl localhost:8080/nonexistent

404 page not found

Existent package:

$ curl localhost:8080/yarpc

<!DOCTYPE html>
<html>
    <head>
        <meta name="go-import" content="go.uber.org/yarpc git https://github.com/yarpc/yarpc-go">
        <meta name="go-source" content="go.uber.org/yarpc https://github.com/yarpc/yarpc-go https://github.com/yarpc/yarpc-go/tree/master{/dir} https://github.com/yarpc/yarpc-go/tree/master{/dir}/{file}#L{line}">
        <meta http-equiv="refresh" content="0; url=https://godoc.org/go.uber.org/yarpc">
    </head>
    <body>
        Nothing to see here. Please <a href="https://godoc.org/go.uber.org/yarpc">move along</a>.
    </body>
</html>

With trailing slash (#9):

$ curl localhost:8080/yarpc/

<!DOCTYPE html>
<html>
    <head>
        <meta name="go-import" content="go.uber.org/yarpc git https://github.com/yarpc/yarpc-go">
        <meta name="go-source" content="go.uber.org/yarpc https://github.com/yarpc/yarpc-go https://github.com/yarpc/yarpc-go/tree/master{/dir} https://github.com/yarpc/yarpc-go/tree/master{/dir}/{file}#L{line}">
        <meta http-equiv="refresh" content="0; url=https://godoc.org/go.uber.org/yarpc/">
    </head>
    <body>
        Nothing to see here. Please <a href="https://godoc.org/go.uber.org/yarpc/">move along</a>.
    </body>
</html>

With deep path:

$ curl localhost:8080/yarpc/hahahaha

<!DOCTYPE html>
<html>
    <head>
        <meta name="go-import" content="go.uber.org/yarpc git https://github.com/yarpc/yarpc-go">
        <meta name="go-source" content="go.uber.org/yarpc https://github.com/yarpc/yarpc-go https://github.com/yarpc/yarpc-go/tree/master{/dir} https://github.com/yarpc/yarpc-go/tree/master{/dir}/{file}#L{line}">
        <meta http-equiv="refresh" content="0; url=https://godoc.org/go.uber.org/yarpc/hahahaha">
    </head>
    <body>
        Nothing to see here. Please <a href="https://godoc.org/go.uber.org/yarpc/hahahaha">move along</a>.
    </body>
</html>

With really deep path:

$ curl localhost:8080/yarpc/hahahaha/blahblah

<!DOCTYPE html>
<html>
    <head>
        <meta name="go-import" content="go.uber.org/yarpc git https://github.com/yarpc/yarpc-go">
        <meta name="go-source" content="go.uber.org/yarpc https://github.com/yarpc/yarpc-go https://github.com/yarpc/yarpc-go/tree/master{/dir} https://github.com/yarpc/yarpc-go/tree/master{/dir}/{file}#L{line}">
        <meta http-equiv="refresh" content="0; url=https://godoc.org/go.uber.org/yarpc/hahahaha/blahblah">
    </head>
    <body>
        Nothing to see here. Please <a href="https://godoc.org/go.uber.org/yarpc/hahahaha/blahblah">move along</a>.
    </body>
</html>

prashantv · 2016-10-02T17:31:31Z

I think the HTTP server is the only way to fix the issue correctly. However, the barrier to using sally goes up significantly with this approach. Is option #2 something we could still support? E.g., when you run sally, it figured out the subpackages and adds them automatically. Re-running is all that's needed if there's a new package.

HelloGrayson · 2016-10-02T17:46:43Z

@prashantv I had thought the same thing, but switching to a server was easy: #15

I think it's important we get Sally working seamlessly - I don't want a bunch of customers stumbling over confusing go tool errors, even if the current setup does work.

I'm going to spend a bit of time today on a Travis -> AWS continuous deployment flow using go.graysonkoonce.com - if that ends up being easy, then I think this is a viable path.

prashantv · 2016-10-02T17:49:51Z

@breerly Switching to a HTTP server is easy in Go, but it's also harder for more users to use since the overhead goes from: "Use this tool + GitHub pages" to "Use this tool, and run it on some hosting service" (which now means yet another service to deploy, monitor, pay for hosting, etc).

It really depends on the reach we'd like for sally. For my personal projects, I'd likely not use sally if it required hosting another binary somewhere, but would definitely consider it if it was super easy with GitHub pages. What do you think?

HelloGrayson · 2016-10-02T17:56:58Z

@prashantv I think what I'd like to do is support both modes. For go.uber.org, I want it to be completely seamless, and so I'm ok hooking it up as a server.

I think we should make it work as a static site generator for Github Pages at the same time. In that mode, the best we can likely do is dynamically determine the import paths and create the right static site structure from there.

prashantv · 2016-10-02T17:58:25Z

Yep, that'd be ideal, exactly what I was thinking :)

ascandella · 2016-10-12T20:09:43Z

Ok cool, sounds like we're set to move uberfx to go.uber.org/fx then?

prashantv mentioned this issue Oct 1, 2016

Glide 0.11 doesn't work with go.uber.org yarpc/yarpc-go#369

Closed

HelloGrayson added the enhancement label Oct 1, 2016

This was referenced Oct 1, 2016

Support trailing slash #9

Closed

Rework as HTTP server #15

Merged

HelloGrayson closed this as completed in #15 Oct 12, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support non-top level imports #14

Support non-top level imports #14

prashantv commented Oct 1, 2016

HelloGrayson commented Oct 1, 2016 •

edited

prashantv commented Oct 1, 2016

HelloGrayson commented Oct 1, 2016 •

edited

HelloGrayson commented Oct 1, 2016

prashantv commented Oct 2, 2016

HelloGrayson commented Oct 2, 2016

prashantv commented Oct 2, 2016

HelloGrayson commented Oct 2, 2016 •

edited

prashantv commented Oct 2, 2016

ascandella commented Oct 12, 2016

Support non-top level imports #14

Support non-top level imports #14

Comments

prashantv commented Oct 1, 2016

HelloGrayson commented Oct 1, 2016 • edited

prashantv commented Oct 1, 2016

HelloGrayson commented Oct 1, 2016 • edited

HelloGrayson commented Oct 1, 2016

prashantv commented Oct 2, 2016

HelloGrayson commented Oct 2, 2016

prashantv commented Oct 2, 2016

HelloGrayson commented Oct 2, 2016 • edited

prashantv commented Oct 2, 2016

ascandella commented Oct 12, 2016

HelloGrayson commented Oct 1, 2016 •

edited

HelloGrayson commented Oct 1, 2016 •

edited

HelloGrayson commented Oct 2, 2016 •

edited