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

WIP: Windows Support #999

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ on:
jobs:

build:
runs-on: ubuntu-latest
runs-on: ${{ matrix.os }}
strategy:
matrix:
go: ["1.15.x", "1.16.x", "1.17.x"]
os: ["ubuntu-latest", "windows-latest"]
include:
- go: 1.17.x
os: ubuntu-latest
latest: true
# Lint only on Ubuntu with the latest Go.

steps:
- name: Setup Go
Expand Down
31 changes: 30 additions & 1 deletion sink.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"io"
"net/url"
"os"
"path/filepath"
"strings"
"sync"

Expand Down Expand Up @@ -96,16 +97,44 @@
}

func newSink(rawURL string) (Sink, error) {
u, err := url.Parse(rawURL)
// Before we attempt to parse this as a URL,
// ensure that file separators have been replaced with "/".
// Unix systems will be unaffected, and on Windows, this will turn,
// C:\foo\bar
// To,
// C:/foo/bar
// Which will parse as,
// URL{Scheme: "c", Path: "/foo/bar"}
// Note that Scheme is case-insensitive; it's always lower case.
u, err := url.Parse(filepath.ToSlash(rawURL))
if err != nil {
return nil, fmt.Errorf("can't parse %q as a URL: %v", rawURL, err)
}

if u.Scheme == "" {
// Unix:
// Naked paths like "/foo/bar" will
// parse to an empty scheme.
u.Scheme = schemeFile
}

_sinkMutex.RLock()
factory, ok := _sinkFactories[u.Scheme]
if !ok {
// On Windows, the scheme ("c") is part of the path
// ("c:\foo\bar"). Try again with the file scheme.

// Volume name is the "C:" of "C:\foo\bar" on Windows,
// and an empty string on other systems.
// Scheme is "c" (lower case).
volumeName := filepath.VolumeName(rawURL)
if u.Scheme+":" == strings.ToLower(volumeName) {
// Convert back to C:\foo\bar.
u.Path = filepath.Join(volumeName, filepath.FromSlash(u.Path))
u.Scheme = schemeFile
factory, ok = _sinkFactories[u.Scheme]

Check warning on line 135 in sink.go

View check run for this annotation

Codecov / codecov/patch

sink.go#L133-L135

Added lines #L133 - L135 were not covered by tests
}
}
_sinkMutex.RUnlock()
if !ok {
return nil, &errSinkNotFound{u.Scheme}
Expand Down
3 changes: 1 addition & 2 deletions writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"net/url"
"os"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -52,7 +51,7 @@ func TestOpenNoPaths(t *testing.T) {
func TestOpen(t *testing.T) {
tempName := tempFileName("", "zap-open-test")
assert.False(t, fileExists(tempName))
require.True(t, strings.HasPrefix(tempName, "/"), "Expected absolute temp file path.")
require.True(t, filepath.IsAbs(tempName), "Expected absolute temp file path.")

tests := []struct {
paths []string
Expand Down