MS Windows fixes #310
MS Windows fixes #310
Conversation
|
I think I can come up with a slightly cleaner fix so I don't have to patch every single backend method. |
|
@muesli if you don't entirely dislike this patch, we could merge as is to fix the windows situation and I'll work on a new PR, to clean things up a bit. |
| @@ -106,7 +105,7 @@ func main() { | |||
| log.Fatalf("Error creating the configuration %s", err) | |||
| } | |||
|
|
|||
| if config.URL().String() != cfg.DefaultPath() { // the user specified a custom config path or URI | |||
There was a problem hiding this comment.
String() does some encoding when using Windows paths which is not what we want here.
|
This is looking good now. Ended up wrapping Also took the chance to add some test helpers and clean things up a bit. |
Not yet, just found one more windows niggle when executing the binary. |
|
Fixed in 602668a |
|
sounds good! |
There was a problem hiding this comment.
Hey @rubiojr o/
Sorry for responding this late - I've somehow completely missed your PR on this.
I implemented a configuration system in knoxite based on your awesome work in beehive. I'm also about to contribute back some tweaks and enhancements I made in knoxite to beehives config system. As @muesli already addressed I also think it's best to somehow combine both systems - or at least don't let them diverge that much - so we can push improvements and fixes to both programs.
In knoxite the same issues regarding windows paths occurred, however we've used another approach to resolve them. We wrapped a method called PathToUrl around url.Parse(), called by the configs setURL() method. This method should be able to parse all kind of different formatted config file paths (unix/windows) without converting backslashes with a fixWindowsPath method.
| if runtime.GOOS == "windows" { | ||
| u.Path = filepath.Join(u.Host, u.Path) | ||
| u.Host = "" | ||
| } |
There was a problem hiding this comment.
The URL Host probably also needs to be joined for other operating systems too, e.g
crypto://pw@local/folder/beehive.conf gets saved at /folder/beehive.conf
|
Awesome @penguwin, let me have a look at the implementation there and I'll get back to you ASAP. |
|
cool, if there's anything I can help/assist you with let me know |
* Handle Windows path arguments like `c:/path/to/beehive.conf` when using --config. * Handle crypto URLs correctly * Test fixes, making sure we always use forward slashes in paths under Windows. For some extra context, Go URL parsing in Windows can be a bit surprising. Parsing file:///c:/beehive.conf, the URI format documented [here][1], will set the URL path to "/c:/beehive.conf". This behaviour is captured in golang/go#6027. If we use a URL like file://c:/beehive.conf, the drive name will be parsed as the host and the path will miss the drive name. ```go package main import ( "fmt" "net/url" ) func main() { u, _ := url.Parse(`file://c:/beehive.conf`) fmt.Println(u.Host) fmt.Println(u.Path) } ``` This prints: ``` c: /beehive.conf ``` We workaround it patching the URL path and host so URLs like `crypt://sercret@c:/beehive.conf` behave as expected. [1]: https://docs.microsoft.com/en-us/archive/blogs/ie/file-uris-in-windows
Wraps platform-specific workarounds.
rubiojr
force-pushed
the
windows-fixes
branch
from
602668a
to
d097b0f
Compare
|
@penguwin: rebased the branch but not without some 😅, since I forgot what I did here in May 😄 . It'll need some extra polish before asking for a reviewing it again, so I'm taking care of that first. If y'all haven't started it, I'll also be looking into extracting the configuration system into it's own repo, see if we can make that work for both beehive and knoxite, to avoid duplicated work and the tedious maintenance task of cherry picking fixes for it from one project to the other. |
c:/path/to/beehive.confwhen using --config.c:\foo\barwindows paths aren't accepted (use forward slash instead)For some extra context, Go URL parsing in Windows can be a bit surprising.
Parsing
file:///c:/beehive.conf, the URI format documented here,will set the URL path to
/c:/beehive.conf.This behaviour is captured in golang/go#6027.
If we use a URL like file://c:/beehive.conf, the drive name will be parsed
as the host and the path will miss the drive name.
This prints:
We workaround it patching the URL path and host so URLs like
crypt://sercret@c:/beehive.confbehave as expected.