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

Delete ExternalTools directory on Npm Cache Clear #1053

Merged
merged 5 commits into from
Jun 24, 2016

Conversation

mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Jun 14, 2016

This change updates the npm cache clear button to also delete our ExternalTools directory. This directory contains node packages used internally by NTVS, so we want to provide a way to delete it if users get in a bad state.

Also extracted the localappdata paths for NTVS to constants.

closes #1044

This change updates the npm cache clear button to also delete our ExternalTools directory. This directory contains node packages used internally by NTVS, so we want to provide a way to delete it if users get in a bad state.

Also extracted the localappdata paths for NTVS to constants.

closes microsoft#1044
} catch (DirectoryNotFoundException) {
// Directory has already been deleted. Do nothing.
_cacheClearedSuccessfully.Visible = true;
return true;
} catch (IOException exception) {
// files are in use or path is too long
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, just now realizing this case might be pretty likely in the case of typings... what's the max path length of typings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here was the script I used:

C:\Users\matb.REDMOND\AppData\Local\Microsoft\Node.js Tools> Get-ChildItem -Recurse . | % { $_.fullname } | sort -Property Length

Longest path is 150 characters with Node 6 and my relatively short user name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On < Node 6, the max path was 273 :sad:

I changed the logic to use robocopy to nuke the directory instead. This was tested with Node 6 and Node 0.12, and both seem to work fine now.

For reference, another way to delete long path files is with the Windows DeleteFile function:

In the ANSI version of this function, the name is limited to MAX_PATH characters. To extend this limit to 32,767 wide characters, call the Unicode version of the function and prepend "\?" to the path.

@mousetraps
Copy link
Contributor

Left some comments... we may want to reconsider our approach here if we think that we are significantly more likely to fail to delete typings than we are to delete the existing cache directory.

// Directory has already been deleted. Do nothing.
_cacheClearedSuccessfully.Visible = true;
} catch (IOException exception) {
// To handle long paths, nuke the directory contents with robocopy
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: nuke -> exterminate 🤖

@mousetraps
Copy link
Contributor

👍

@mjbvz mjbvz merged commit 29bd5ec into microsoft:master Jun 24, 2016
@mjbvz mjbvz removed the in-progress label Jun 24, 2016
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.

Npm Options delete cache button should delete AppData typings install
3 participants