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

Normalize paths coming from ls tree #891

Merged
merged 1 commit into from
Mar 16, 2022

Conversation

gsoltis
Copy link
Contributor

@gsoltis gsoltis commented Mar 16, 2022

Note:

@vercel
Copy link

vercel bot commented Mar 16, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/turbo-site/FAh9Mg25SWbUEWQiMqxN2VhsfhCE
✅ Preview: https://turbo-site-git-gsoltis-normalizelstree.vercel.sh

@jaredpalmer
Copy link
Contributor

Not sure if error is related

@thebanjomatic
Copy link
Contributor

Will this also address the root cause of Issue #801?

@jaredpalmer
Copy link
Contributor

@thebanjomatic i believe so

@thebanjomatic
Copy link
Contributor

thebanjomatic commented Mar 16, 2022

Thanks, I pulled down and built the branch locally and can confirm that this does indeed appear to be the case if you want to tag the issue. That being said, it might still be a good idea to add the ToSlash call in cache_http.go when adding files to the tar writer to ensure that it is generating archive files consistent with what is generated on linux in terms of preserving the directory structure in the file. Those file paths appear to come from globby which generates platform dependent paths.

filesToBeCached := globby.GlobFiles(pt.pkg.Dir, outputs, ignore)

Maybe httpCache.storeFile could just normalize the name that gets passed in to posix

@gsoltis
Copy link
Contributor Author

gsoltis commented Mar 16, 2022

Github Actions seems to be degraded at the moment. Will investigate test failures once it sorts itself out.

@gsoltis
Copy link
Contributor Author

gsoltis commented Mar 16, 2022

Looks like test failures were due to Github Actions issues.

@gsoltis gsoltis merged commit 6ed3122 into gsoltis/fix_global_deps_path Mar 16, 2022
@gsoltis gsoltis deleted the gsoltis/normalize_lstree branch March 16, 2022 20:10
kodiakhq bot pushed a commit that referenced this pull request Mar 17, 2022
This should ensure that archives generated for remote caching have the directory
structure preserved in a consistant manner to what is generated on linux/mac

Fixes: #801

PR #891 I believe fixes the hash calculation so that a remote-cache generated on linux or mac can be consumed on windows and no longer results in a cache miss, but I still have concerns about consuming archives generated on windows from the linux environment. In particular, these paths are coming from globby and don't pass through the normalization from the linked PR, so they are still os-dependent.

I have encountered some problems with builds failing in GH Actions because dependencies that should have been built already were not found when I had previously built those projects on my windows machine. I believe this PR should fix the problem but I haven't had the chance to try reproducing the existing issue to verify that this fixes it. I might be able to get that done later today, but I wanted to get a PR up sooner rather than later
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants