-
Notifications
You must be signed in to change notification settings - Fork 219
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
Generate project.json in %TEMP% rather than in install location #332
Generate project.json in %TEMP% rather than in install location #332
Conversation
# create the temp folder if it does not exist | ||
$TempPath = Join-Path -Path $env:TEMP -ChildPath "getclrdbg" | ||
if (-not (Test-Path -Path $TempPath -PathType Container)) { | ||
New-Item -ItemType Directory -Force -Path $TempPath | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you delete the folder if it exists? I'm a little concerned that the file gets cached somewhere I don't know about and may not realize the need to clean to get an update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way project.json is generated, it will be recreated each time this script is run, nothing is cached.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, I see above it will override the contents regardless. Is there any other file that may be left behind that could be a problem? If not then no need to delete the folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gregg-miskelly yes. |
New-Item -ItemType Directory -Force -Path $InstallPath | ||
# create the temp folder if it does not exist | ||
$TempPath = Join-Path -Path $env:TEMP -ChildPath "getclrdbg" | ||
if (-not (Test-Path -Path $TempPath -PathType Container)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no point to having -PathType Container
if you don't deal with having a file named 'getclrdbg'
lgtm |
77c55ee
to
f750cf8
Compare
Updated this to created a directory named from a new Guid under %temp% and also fixed an issue with giving the script a relative path as an install location. This should fix any caching/simultaneous use issues. |
New-Item -ItemType Directory -Force -Path $InstallPath | ||
# create the temp folder if it does not exist | ||
$GuidString = [System.Guid]::NewGuid() | ||
$TempPath = Join-Path -Path $env:TEMP -ChildPath $GuidString |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are going to do it this way (which sounds reasonable to me) you should probably also delete the directory at the end of the script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change we generate the project.json in %temp%\<guid> and restore/publish from this directory into the final install directory. This avoids having the project.json in the final install directory.
f750cf8
to
237affa
Compare
With this change we generate the project.json in %temp%\getclrdbg and
restore/publish from this directory into the final install directory. This
avoids having the project.json in the final install directory.