skratchdot / open-golang

Open a file, directory, or URI using the OS's default application for that object type. Optionally, you can specify an application to use.
MIT License
783 stars 65 forks source link

Can't find xdg-open in production #2

Open grymoire7 opened 10 years ago

grymoire7 commented 10 years ago

The open method references xdg-open relative to package source. So the tests pass, but this doesn't work in a shipping product where the executable may be moved or not even be shipped with the source.

open.go:66> app := path.Join(path.Dir(file), "..", "vendor", "xdg-open")

In shipping code, I use:

open.go:66> app := "./xdg-open"

and ship xdg-open alongside the executable.

A different solution would be to default to "xdg-open" (assume it's in the path) and allow it to be settable by the user to another location.

skratchdot commented 10 years ago

@grymoire7 - can you retest this? I'm not entirely sure the fix works, but there's a blog post suggesting it will. If not, you can re-open, and I'll try to think of a better way to fix it.

grymoire7 commented 10 years ago

This suffers from what is essentially the same issue. If only an executable is shipped (no source) then it will be looking for xdg-open as "../Vendor/xdg-open". Now, with runtime.Caller(1) this could still work IF the open-golang package is installed on the client machine, though I didn't actually try that. The problem is that when I ship an executable I can't expect 1) that the open-golang package (or even Go) is installed on the client machine, or 2) that I can place xdg-open in "../Vendor" relative to the executable location.

I think it might be reasonable to expect xdg-open to be shipped in the same directory as the executable, as shown in the original issue comments. Alternately, it would be reasonable to allow the location to be specified via an API call. I think the API call option would be the most general solution. It would allow me (or anyone) to specify the location I ship it in, and allow you to leave "../Vendor/xdg-open" as the default which would likely be ideal for testing in place.

I apologize for any confusion. Please let me if there's anything else I can do.

Thanks, -Tracy

skratchdot commented 10 years ago

Ah. Thanks for that explanation. I'll re-open and fix. An API call makes sense in this case...

Byron commented 9 years ago

Something I find problematic in the current implementation is that even if xdg-open is present in the system, this implementation will fail unless the executable using open-golang ships its own in the expected spot.

As my executable is supposed to remain standalone, I will have to make a fix. If you like, I can submit a pull request which tries to find xdg-open in the system first. If that fails, it would create it's own in a temporary location and use it. That way, executables wouldn't have to rely on side-by-side binaries.

What do you think about this approach ?

skratchdot commented 9 years ago

That would be great. I've actually meant to look into this bug, but don't have any experience shipping golang code. I've been meaning to setup a vagrant box to test the best way to make this package 'shipable'. Please submit a PR if you fix it! Thanks for the help...

skratchdot commented 9 years ago

I had meant to look into including the xdg-open package as an embedded string perhaps, so *nix boxes could run the script without needing it on the file system- but I think that approach might be flawed b/c the xdg-open script doesn't appear to work that way (it expects arguments, etc).

Whatever the fix, I should definitely be checking if xdg-open exists in the current path before trying to find the packaged version that is included in this lib (like you were saying).

Byron commented 9 years ago

Alright, will do ! I am inexperienced with go myself, but found the distribution process especially nice. You might want to look into gox to aid in cross-platform compilation.

skratchdot commented 9 years ago

I was also just reading this to see if there's a good way to publish static files / executables in a golang package: https://groups.google.com/forum/#!topic/golang-nuts/rdk-HdpxQps

I'd like to know if there's a good way to include xdg-open as a string, and execute it via eval or bash -c (or something similar).

Byron commented 9 years ago

I think you can use verbatim go literal strings for that.

const script := `<all script code goes here
and here>
No need to escape anything.
`
skratchdot commented 9 years ago

I see @Byron and @slspeek both made the same change. I'm gonna go ahead and pull it in. I'd still like to include the script as a constant, and execute it that way (for linux distro's that don't come with xdg-open), but I'll pull this in for now until I get a bit of time to test the inclusion of xdg-open via a variable.

Byron commented 9 years ago

Thanks, great to see ! I feel much better now that I don't have to use my fork anymore.

slspeek commented 9 years ago

Thanks.

2014-09-05 23:36 GMT+02:00 skratchdot notifications@github.com:

I see @Byron https://github.com/Byron and @slspeek https://github.com/slspeek both made the same change. I'm gonna go ahead and pull it in. I'd still like to include the script as a constant, and execute it that way (for linux distro's that don't come with xdg-open), but I'll pull this in for now until I get a bit of time to test the inclusion of xdg-open via a variable.

— Reply to this email directly or view it on GitHub https://github.com/skratchdot/open-golang/issues/2#issuecomment-54684762 .