vercel / turborepo

Build system optimized for JavaScript and TypeScript, written in Rust
https://turbo.build/repo/docs
MIT License
26.39k stars 1.84k forks source link

error fetching from cache: error moving artifact from cache into package\lib-name - pattern contains path separator #195

Closed GiancarlosIO closed 2 years ago

GiancarlosIO commented 2 years ago

What version of Turborepo are you using?

1.0.6

Describe the Bug

I there! I'm trying Turborepo in a mono repository that works with Lerna. Currently, I'm getting a lot of these warnings in the console:

@my-org/icons:build: error fetching from cache: error moving artifact from cache into packages\icons: createtemp packages\next\.turbo\turbo-build.log: pattern contains path separator
@my-org/icons:build: cache miss, executing 496f71e70f091e41

Looks like the cache is not being used in the subsequent runs 🤔 At the end of the process I'm getting this output:

 Tasks:    30 successful, 30 total
 Cached:    0 cached, 30 total
 Time:    2m16.758s

As far I know, this warning comes from the io/iouti package. Maybe an extra validation is needed in windows 10?

Operating System: windows 10

Expected Behavior

The subsequent runs of a pipeline should use the cache from the .turbo folder.

To Reproduce

I'm getting the same warning in the basic example project https://github.com/vercel/turborepo/tree/main/examples/basic

bolshoytoster commented 2 years ago

The error seems to come from cli/internal/run/run.go/cli/internal/fs/fs.go

https://github.com/vercel/turborepo/blob/f6aa3d6300255c00bd1dd8b5667de407b1c51d4b/cli/internal/run/run.go#L448-L450

https://github.com/vercel/turborepo/blob/f6aa3d6300255c00bd1dd8b5667de407b1c51d4b/cli/internal/fs/fs.go#L76-L78

According to this issue from another repository, this error is caused by the second argument of ioutil.TempFile containing a slash. So we should be able to fix this by just stripping slashes in the WriteFile function.

bolshoytoster commented 2 years ago

Looking into the ioutil source, the error is thrown if the filename contains the os' path separator. This is probably why it only occurs on windows, some file must contain a backslash.

The TempFile function is 'dumb' as in it doesn't care about escaped slashes - it loops through each character and checks if it's the path separator - so we'd have to either remove/replace them or use a different method for creating temporary files.

ashfaqnisar commented 2 years ago

I happen to get this same issue with the lint of the turborepo starter pack.

docs:lint: error fetching from cache: error moving artifact from cache into apps\docs: createtemp apps\docs\.turbo\turbo-lint.log: pattern contains path separator
web:lint: error fetching from cache: error moving artifact from cache into apps\web: createtemp apps\web\.turbo\turbo-lint.log: pattern contains path separator

Would be happy to share the log file, if required

bolshoytoster commented 2 years ago

I think the source of this is the path.Split function. As said in this issue from the go repository:

The path package is for slash-separated paths, like URLs. The path/filepath package is for operating system specific path names. If you want to use os.PathSeparator--an operating system specific concept--then use path/filepath, not path.

path.Split only uses forward slashes, We should instead use filepath.Split to add compatibility for windows.

Also, looking into the WriteFile function, do we even need to create a temporary file?

https://github.com/vercel/turborepo/blob/f6aa3d6300255c00bd1dd8b5667de407b1c51d4b/cli/internal/fs/fs.go#L68-L96

All it does is create a temporary file, copy data to it, then rename the file to it's intended name. Could we not just create the destination file, io.copy the data from the source file and just delete the file if err != nil.

Additionally, WriteFile is only referenced in the CopyFile function just above, so we could just remove WriteFile altogether and put it in CopyFile.

I can open a pr for this if you want.