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

Do not reset WSL permissions metadata on save #49021

Closed
drujd opened this issue May 1, 2018 · 20 comments

Comments

@drujd
Copy link

commented May 1, 2018

The April Windows 10 update brings many improvements to WSL. One of them is the possibility to save unix permissions as NTFS file metadata:

https://blogs.msdn.microsoft.com/commandline/2018/01/12/chmod-chown-wsl-improvements/

One of the caveats is: "Editing a file using a Windows editor may remove the file's Linux metadata. In this case, the file will revert to its default permissions"

As of now, VSCode unfortunately does exactly that - when you save the file, the metadata (containing i.e. the execution permission bit) is deleted.

It would be nice to keep the file's metadata on save.

@nickjj

This comment has been minimized.

Copy link

commented May 7, 2018

I also just upgraded to 18.03 and am noticing the same behavior.

Files get reset back to 755 as soon as you save them with VSCode (running in Windows).

I'm seeing that other editors are avoiding this problem by turning off safe writes. I don't personally use phpStorm but I did read about that here https://github.com/andrewmackrodt/docker-sync-wsl-native#use-wsl-as-the-default-terminal-in-phpstorm.

@andrewmackrodt

This comment has been minimized.

Copy link

commented May 7, 2018

Unfortunately PhpStorm non-safe writes lose linux metadata for NTFS files (but not those mounted via SSH which v1 of that project was using).

From previously looking at this problem I believe it's a node.js issue. #28927 (comment); more specifically I think it related to nodejs/node#18549. WSL does not use ADS IIRC, I'm not sure how it stores its metadata.

@WSLUser

This comment has been minimized.

Copy link

commented May 7, 2018

This is applicable for anything done in VSCode, not just nodejs just as mentioned in the OP. They need to do work on their end but what about everything else? I don't think we can or should expect package maintainers to update for this behavior since it's a Windows specific issue. IDEs need to be able to handle the behavior, which from your guys point of view is a feature request, but a necessary one in order to maintain security of our data.

@drujd

This comment has been minimized.

Copy link
Author

commented May 8, 2018

@DarthSpock VSCode is written in Node.js/Electron. It utilizes Node.js methods for file operations.

@WSLUser

This comment has been minimized.

Copy link

commented May 8, 2018

And the referenced issue in nodejs was closed when trying to address this saying "use this workaround". I pinged them about it to see what they say about it. But if it gets a "won't fix", then the team here will need to fork off from them and implement it themselves. Thank you for the enlightment though.

@drujd

This comment has been minimized.

Copy link
Author

commented May 26, 2018

This might help: #42899

david50407 added a commit to david50407/voting_booth that referenced this issue Jun 27, 2018

@onomatopellan

This comment has been minimized.

Copy link

commented Jun 27, 2018

I just couldn't wait and compiled VScode myself with PR #42899 and indeed it fixes this issue and unix permission are conserved when CTRL+S a file on DrvFs.
I hope they merge it soon.

@WSLUser

This comment has been minimized.

Copy link

commented Jun 27, 2018

Problem is something with AppVeyor is blocking the merge and none of the MS devs appear to be troubleshooting the issue. For now the only workaround that preserves Linux permissions when working in WSL is to use the native Linux VS Code and doing so means suffering performance drops. Until the perf issues are resolved in WSL, Windows VS Code is the best route so this issue should be shown some serious attention over other new features since this is fundamental to workflow.

sampsyo added a commit to cucapra/linguine that referenced this issue Jul 24, 2018

Fix lots of permissions bits
Lots of text files keep getting their "execute" bit set because of
editing on Windows? I think the underlying problem is as described here:
microsoft/vscode#49021

For @Checkmate50, can you please set this git configuration option?
https://stackoverflow.com/a/1580644
@Oxfos

This comment has been minimized.

Copy link

commented Aug 19, 2018

I'm also using WSL and I start VS Code (Windows) from Bash by simply typing code. But I experience the same problem, i.e. files being stripped all permissions when modified in VS Code (I see that when I start a new Bash session).
I was thinking to install the Linux VS Code but I read above that performance would be reduced. So I'd better wait. It would be nice if in a new VS Code version Linux permissions are not stripped when files are modified.

SOLVED (20/8/2018):
I read here that one should use the /mnt/drive/path directory in the Windows file system of WSL to work with files that will be created/modified with Windows tools (like VS Code). I so restarted my project in that directory (c drive) and now I do not experience the problems with permissions I describe above. (Before I was simply working in my home directory ~/).
I was not aware of that. Sorry for any inconvenience.

@bpasero bpasero self-assigned this Aug 20, 2018

@bpasero bpasero added bug and removed feature-request upstream labels Aug 20, 2018

@bpasero bpasero added this to the August 2018 milestone Aug 20, 2018

@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 20, 2018

#42899 is merged which claims to fix this. Alternate data streams will no longer be removed when saving a file.

(I still think it is not advised to save files from Windows that are owned by WSL though, so do this at your own risk)

@nickjj

This comment has been minimized.

Copy link

commented Aug 20, 2018

(I still think it is not advised to save files from Windows that are owned by WSL though, so do this at your own risk)

These aren't necessarily files owned by WSL. They are files that happen to be mounted into WSL.

Insiders 1.27 as of commit 6e559 (August 20th 2018's build) still resets files back to 755 on edit btw, not sure if that PR has made it into an insiders release yet.

@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 21, 2018

@nickjj can you verify with today's insider build

@nickjj

This comment has been minimized.

Copy link

commented Aug 21, 2018

@bpasero Seems to be working great now.

Here's what I did to test it:

  1. touch hi.txt
  2. chmod 644 hi.txt
  3. stat -c '%a %n' hi.txt (to verify it is 644)
  4. code hi.txt
  5. Edit file in VSCode by adding text to it and then save it with VSCode
  6. Repeat step 3, and it was still 644.

For clarity, all of the 6 steps are ran from inside of a ConEmu driven WSL Ubuntu terminal. hi.txt lives on a Windows drive that's available as a mount inside of WSL. VSCode is installed directly in Windows.

@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 21, 2018

@nickjj how does that setup work exactly? How is hi.txt linked from Windows into Linux? Specifically, how does code hi.txt turn into an absolute file path for VSCode to open (I assume you call this from Linux)?

@VictorMazilu

This comment has been minimized.

Copy link

commented Aug 21, 2018

@nickjj

This comment has been minimized.

Copy link

commented Aug 21, 2018

@bpasero VSCode is installed directly within Windows (Windows installer) and hi.txt is a file that exists in a mounted drive that's mounted into WSL. The file and drive's content lives outside of the internal WSL file system.

The terminal I use (ConEmu) preloads wslpath to allow me to run code (which is a symlink of code-insiders btw) directly from within a WSL terminal. It's pretty rad!

@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 21, 2018

@nickjj makes sense, thanks

@bpasero bpasero added the verified label Aug 21, 2018

@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 21, 2018

Setting to verified based on #49021 (comment)

@drujd

This comment has been minimized.

Copy link
Author

commented Aug 21, 2018

Can also confirm that the Insiders build retains alternate data streams / WSL permissions! Thank you!

@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 21, 2018

Great. Also let me know if anyone is seeing weird issues when saving in general, the code change impacts every single save on Windows.

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
8 participants
You can’t perform that action at this time.