ci: add windows to test matrix #173

crazy-max · 2024-01-24T15:36:35Z

closes Fix file path separator #148
closes Add workflow that runs tests on Windows #149

crazy-max · 2024-01-24T15:52:31Z

Ok so a bunch of tests look to fail on Windows runner: https://github.com/tonistiigi/fsutil/actions/runs/7642673575/job/20822858671?pr=173#step:4:21

I'm more worried about:

--- FAIL: TestInternalReadFile (0.00s)
    followlinks_test.go:198: 
        	Error Trace:	D:/a/fsutil/fsutil/followlinks_test.go:198
        	Error:      	Received unexpected error:
        	            	expected single entry "\\" but got "dir"
        	            	github.com/tonistiigi/fsutil.statFile.func1
        	            		D:/a/fsutil/fsutil/followlinks.go:150
        	            	github.com/tonistiigi/fsutil.(*fs).Walk.func1
        	            		D:/a/fsutil/fsutil/fs.go:80
        	            	path/filepath.walkDir
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/path/filepath/path.go:438
        	            	path/filepath.walkDir
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/path/filepath/path.go:460
        	            	path/filepath.WalkDir
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/path/filepath/path.go:528
        	            	github.com/tonistiigi/fsutil.(*fs).Walk
        	            		D:/a/fsutil/fsutil/fs.go:49
        	            	github.com/tonistiigi/fsutil.statFile
        	            		D:/a/fsutil/fsutil/followlinks.go:145
        	            	github.com/tonistiigi/fsutil.TestInternalReadFile
        	            		D:/a/fsutil/fsutil/followlinks_test.go:197
        	            	testing.tRunner
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/testing/testing.go:1576
        	            	runtime.goexit
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/runtime/asm_amd64.s:1598
        	Test:       	TestInternalReadFile
--- FAIL: TestInternalReadDir (0.00s)
    followlinks_test.go:234: 
        	Error Trace:	D:/a/fsutil/fsutil/followlinks_test.go:234
        	Error:      	Received unexpected error:
        	            	expected to read parent entry "\\" before child "dir"
        	            	github.com/tonistiigi/fsutil.readDir.func1
        	            		D:/a/fsutil/fsutil/followlinks.go:189
        	            	github.com/tonistiigi/fsutil.(*fs).Walk.func1
        	            		D:/a/fsutil/fsutil/fs.go:80
        	            	path/filepath.walkDir
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/path/filepath/path.go:438
        	            	path/filepath.walkDir
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/path/filepath/path.go:460
        	            	path/filepath.WalkDir
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/path/filepath/path.go:528
        	            	github.com/tonistiigi/fsutil.(*fs).Walk
        	            		D:/a/fsutil/fsutil/fs.go:49
        	            	github.com/tonistiigi/fsutil.readDir
        	            		D:/a/fsutil/fsutil/followlinks.go:177
        	            	github.com/tonistiigi/fsutil.TestInternalReadDir
        	            		D:/a/fsutil/fsutil/followlinks_test.go:233
        	            	testing.tRunner
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/testing/testing.go:1576
        	            	runtime.goexit
        	            		C:/hostedtoolcache/windows/go/1.20.12/x64/src/runtime/asm_amd64.s:1598
        	Test:       	TestInternalReadDir

Which seems related to #167 (cc @jedevc)

crazy-max · 2024-01-29T08:05:34Z

followlinks.go

+	root = filepath.FromSlash(filepath.Clean(root))
+	if root == string(filepath.Separator) || root == "." {


It changes the behavior introduced in #174 related to docker/buildx#2207 to be aligned with os separator. cc @gabriel-samfira.

gabriel-samfira · 2024-01-29T08:12:04Z

Hi @crazy-max

Will have a look today. I had a couple of old PRs that tried to do this a while back 😄. Glad to see this happening. Apologies for the delay.

crazy-max · 2024-01-29T08:31:27Z

Hi @crazy-max

Will have a look today. I had a couple of old PRs that tried to do this a while back 😄. Glad to see this happening. Apologies for the delay.

No worries! I will carry your commit from #148 and add back my few changes so you can have a better look. 🙏

gabriel-samfira

Looks good. Just one tiny comment. Changes are mostly mechanic and deal with path separators.

validator.go

thaJeztah · 2024-01-29T11:42:53Z

diff_containerd.go

@@ -138,7 +139,7 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b walkerFn, fil
 					return err
 				}
 				if f1.stat.IsDir() && !f2copy.stat.IsDir() {
-					rmdir = f1.path + string(os.PathSeparator)
+					rmdir = f1.path + string(filepath.Separator)


(Just picked a random line); is all code always executed in the "native" environment, or are there also code-paths executed on non-matching platforms (client-side code running on Linux/mac on behalf of a daemon running on Windows, and vice-versa?)

Also note that / is accepted in various situations on Windows, so users may use / to specify paths on Windows, so in some cases both / and \ may have to be taken into account (if not normalised)

edit: e.g. https://github.com/docker/cli/blob/6dcf285ff155594711038668aa4f7a856435b050/cli/compose/loader/windows_path.go#L12-L27

which was taken from https://github.com/golang/go/blob/1d0e94b1e13d5e8a323a63cd1cc1ef95290c9c36/src/path/filepath/path_windows.go#L12-L65

tonistiigi

Looks like freebsd has actual test failures

crazy-max · 2024-02-07T08:12:40Z

Looks like freebsd has actual test failures

This is not related to changes here I think but #172 (comment). Looks flaky.

crazy-max · 2024-02-07T11:13:49Z

Looks like freebsd has actual test failures

This is not related to changes here I think but #172 (comment). Looks flaky.

Should be good now if you want to merge this one first: #172

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

more checks

receive.go

tonistiigi · 2024-02-07T18:18:29Z

filter.go

@@ -96,7 +96,7 @@ func NewFilterFS(fs FS, opt *FilterOpt) (FS, error) {
 	}

 	patternChars := "*[]?^"
-	if os.PathSeparator != '\\' {
+	if filepath.Separator != '\\' {


Not sure if this can be easily fixed but FS should not be platform-specific, so these filters should also not depend on the current os.

Hum I think this is based on user input if I'm not mistaken (cc @jedevc)

We can leave this for follow-up

This seems to come from #121.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

This was referenced Jan 24, 2024

fsutil error expected single entry docker/buildx#2207

Closed

followlinks: sanitize root path #174

Merged

crazy-max force-pushed the ci-windows branch 3 times, most recently from c595ae6 to 9de1b78 Compare January 26, 2024 06:56

This comment was marked as resolved.

Sign in to view

crazy-max force-pushed the ci-windows branch from 1f24f5c to 0e06527 Compare January 29, 2024 07:40

crazy-max requested a review from tonistiigi January 29, 2024 07:43

crazy-max marked this pull request as ready for review January 29, 2024 08:02

crazy-max commented Jan 29, 2024

View reviewed changes

crazy-max force-pushed the ci-windows branch 2 times, most recently from cb02be4 to b7a2e86 Compare January 29, 2024 09:26

gabriel-samfira approved these changes Jan 29, 2024

View reviewed changes

validator.go Outdated Show resolved Hide resolved

crazy-max force-pushed the ci-windows branch from b7a2e86 to c73d856 Compare January 29, 2024 10:49

thaJeztah reviewed Jan 29, 2024

View reviewed changes

crazy-max mentioned this pull request Jan 29, 2024

docker COPY command broken when running script inside windows environment moby/moby#47231

Closed

crazy-max force-pushed the ci-windows branch from c73d856 to 819ba84 Compare January 30, 2024 07:59

crazy-max mentioned this pull request Jan 30, 2024

ci(deps): replace buildkit to fix fsutil issues on Windows docker/compose#11426

Merged

tonistiigi requested changes Feb 7, 2024

View reviewed changes

crazy-max force-pushed the ci-windows branch from 819ba84 to cb06d2a Compare February 7, 2024 12:19

crazy-max added 2 commits February 7, 2024 18:35

ci: add windows to test matrix

330b1ff

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

diskwriter: fix test scope

b843da9

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

crazy-max force-pushed the ci-windows branch from cb06d2a to f94943e Compare February 7, 2024 17:35

crazy-max requested a review from tonistiigi February 7, 2024 17:44

tonistiigi previously approved these changes Feb 7, 2024

View reviewed changes

tonistiigi self-requested a review February 7, 2024 18:11

tonistiigi reviewed Feb 7, 2024

View reviewed changes

fix file path separator

3d01fa5

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>

crazy-max force-pushed the ci-windows branch from f94943e to 3d01fa5 Compare February 8, 2024 08:09

crazy-max requested a review from tonistiigi February 8, 2024 09:49

tonistiigi approved these changes Feb 8, 2024

View reviewed changes

tonistiigi merged commit d68510a into tonistiigi:master Feb 8, 2024

crazy-max deleted the ci-windows branch February 8, 2024 17:13

jedevc mentioned this pull request Apr 18, 2024

recv: translate linkname to wire format #197

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ci: add windows to test matrix #173

ci: add windows to test matrix #173

Uh oh!

crazy-max commented Jan 24, 2024 •

edited

Loading

Uh oh!

crazy-max commented Jan 24, 2024

Uh oh!

This comment was marked as resolved.

crazy-max Jan 29, 2024

Uh oh!

gabriel-samfira commented Jan 29, 2024

Uh oh!

crazy-max commented Jan 29, 2024

Uh oh!

gabriel-samfira left a comment

Uh oh!

Uh oh!

thaJeztah Jan 29, 2024 •

edited

Loading

Uh oh!

tonistiigi left a comment

Uh oh!

crazy-max commented Feb 7, 2024 •

edited

Loading

Uh oh!

crazy-max commented Feb 7, 2024

Uh oh!

Uh oh!

tonistiigi Feb 7, 2024

Uh oh!

crazy-max Feb 8, 2024

Uh oh!

tonistiigi Feb 8, 2024

Uh oh!

jedevc Feb 8, 2024

Uh oh!

Uh oh!

		root = filepath.FromSlash(filepath.Clean(root))
		if root == string(filepath.Separator) \|\| root == "." {

ci: add windows to test matrix #173

ci: add windows to test matrix #173

Uh oh!

Conversation

crazy-max commented Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crazy-max commented Jan 24, 2024

Uh oh!

This comment was marked as resolved.

crazy-max Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

gabriel-samfira commented Jan 29, 2024

Uh oh!

crazy-max commented Jan 29, 2024

Uh oh!

gabriel-samfira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thaJeztah Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonistiigi left a comment

Choose a reason for hiding this comment

Uh oh!

crazy-max commented Feb 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crazy-max commented Feb 7, 2024

Uh oh!

Uh oh!

tonistiigi Feb 7, 2024

Choose a reason for hiding this comment

Uh oh!

crazy-max Feb 8, 2024

Choose a reason for hiding this comment

Uh oh!

tonistiigi Feb 8, 2024

Choose a reason for hiding this comment

Uh oh!

jedevc Feb 8, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crazy-max commented Jan 24, 2024 •

edited

Loading

thaJeztah Jan 29, 2024 •

edited

Loading

crazy-max commented Feb 7, 2024 •

edited

Loading