这是indexloc提供的服务，不要输入任何密码
Skip to content

fix: preserve file permission #362

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 21, 2021
Merged

fix: preserve file permission #362

merged 2 commits into from
Dec 21, 2021

Conversation

@rubiagatra
Copy link
Contributor

@rubiagatra rubiagatra commented Dec 19, 2021

Close #156

This is only draft if someone can have discussion with me it would be great

@vercel
Copy link
Contributor

vercel bot commented Dec 19, 2021 •
edited
Loading

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/FBiFEm5tcwgBgrfirxfTVP1Gn2aQ
✅ Preview: https://turbo-site-git-fork-rubiagatra-fix-preserved-file-permission.vercel.sh

@rubiagatra rubiagatra mentioned this pull request Dec 19, 2021
Closed
@jaredpalmer jaredpalmer added the kind: bug Something isn't working label Dec 20, 2021
jaredpalmer
jaredpalmer suggested changes Dec 20, 2021
View reviewed changes
Copy link
Contributor

@jaredpalmer jaredpalmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change the call sites to pass 0 to preserve permissions?

turborepo/cli/internal/cache/cache_fs.go

Line 32 in 11665a9

err := fs.RecursiveCopyOrLinkFile(cachedFolder, target, fs.DirPermissions, true, true)

turborepo/cli/internal/cache/cache_fs.go

Line 54 in 11665a9

if err := fs.CopyOrLinkFile(file, filepath.Join(f.cacheDirectory, hash, rel), fs.DirPermissions, fs.DirPermissions, true, true); err != nil {

and in all of this file

https://github.com/vercel/turborepo/blob/a229aaa1c4aaecaef054adf09a8800ca74e4c1c6/cli/internal/prune/prune.go#L175-L291

and lastly here:

turborepo/cli/internal/run/run_state.go

Line 289 in 2ee2c40

if err := fs.CopyFile(chrometracing.Path(), name, fs.DirPermissions); err != nil {

@rubiagatra
Copy link
Contributor Author

rubiagatra commented Dec 21, 2021 •
edited
Loading

I just did that you requested @jaredpalmer and undo my draft commit.

  1. Pass 0 instead of fs.DirPermissions
  2. Run go test ./...
?   	turbo/cmd/turbo	[no test files]
?   	turbo/internal/api	[no test files]
?   	turbo/internal/backends	[no test files]
?   	turbo/internal/backends/nodejs	[no test files]
?   	turbo/internal/cache	[no test files]
?   	turbo/internal/client	[no test files]
?   	turbo/internal/config	[no test files]
ok  	turbo/internal/context	1.608s
ok  	turbo/internal/core	0.533s
ok  	turbo/internal/fs	1.295s
?   	turbo/internal/globby	[no test files]
?   	turbo/internal/info	[no test files]
?   	turbo/internal/login	[no test files]
ok  	turbo/internal/logstreamer	0.841s
?   	turbo/internal/prune	[no test files]
ok  	turbo/internal/run	2.290s
?   	turbo/internal/scm	[no test files]
?   	turbo/internal/ui	[no test files]
ok  	turbo/internal/ui/term	1.980s
ok  	turbo/internal/util	1.140s
?   	turbo/internal/util/browser	[no test files]
?   	turbo/internal/xxhash	[no test files]

Is there anything I can help with my changes to not break something?. I see potential test/documentation/onboarding improvement in the future, I am happy to help.

@rubiagatra rubiagatra changed the title draft: preserved file permission bugfix: preserved file permission Dec 21, 2021
@jaredpalmer jaredpalmer changed the title bugfix: preserved file permission fix: preserved file permission Dec 21, 2021
@jaredpalmer jaredpalmer changed the title fix: preserved file permission fix: preserve file permission Dec 21, 2021
@jaredpalmer jaredpalmer self-requested a review December 21, 2021 14:29
@jaredpalmer jaredpalmer merged commit 67e64ab into vercel:main Dec 21, 2021
jaredpalmer added a commit that referenced this pull request Dec 22, 2021
@jaredpalmer
Revert "fix: preserve file permission (#362)"
7053451 
This reverts commit 67e64ab.
@jaredpalmer
Copy link
Contributor

jaredpalmer commented Dec 22, 2021

This unfortunately is more involved change. The current implementation breaks binary executables

sokra added a commit that referenced this pull request Oct 25, 2022
@sokra
increase benchmark timeouts to be able to benchmark larger module cou…
2bd92b3 
…nts (#362)
jridgewell pushed a commit to vercel/next.js that referenced this pull request Mar 10, 2023
@sokra
increase benchmark timeouts to be able to benchmark larger module cou…
1ab7e73 
…nts (vercel/turborepo#362)
sokra added a commit to vercel/next.js that referenced this pull request Mar 13, 2023
@sokra
increase benchmark timeouts to be able to benchmark larger module cou…
826998c 
…nts (vercel/turborepo#362)
@DuCanhGH DuCanhGH mentioned this pull request Mar 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaredpalmer jaredpalmer Awaiting requested review from jaredpalmer

Assignees

No one assigned

Labels

kind: bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

File Permissions not Preserved from Cache

2 participants

@rubiagatra @jaredpalmer