-
Notifications
You must be signed in to change notification settings - Fork 403
DevCheck: Creating nuget directory if it does not exist #6039
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR fixes an issue where the DevCheck script fails when attempting to download nuget.exe if the target directory doesn't exist. The script now creates the necessary directory structure before downloading the file.
- Adds directory existence check before downloading nuget.exe
- Creates the directory if it doesn't exist to prevent curl download failures
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
DrusTheAxe
left a comment
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.
I assume
tools/DevCheck/DevCheck.ps1
Outdated
| $fileDir = [IO.Path]::GetDirectoryName($file) | ||
| if (-not(Test-Path -Path $fileDir -PathType Container)) | ||
| { | ||
| $null = New-Item -ItemType Directory -Path $fileDir |
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.
...\.user directory's created by DevCheck but could be a timing thing, or simply one tells DevCheck to only download nuget.exe and not do other things (which happen to create .user)
function Get-UserPath creates the .user directory but as $NugetExe could be anywhere (not just there) we have 2 choices
- Create the target dir for the caller
- Expect the target dir exists (caller's job to ensure it exists) AND ensure
.user(and.temp) are created before doing any Nuget processing
I'm kinda partial to #2. Helps avoid human error if you typo your desired target dir DevCheck won't know any better and create it for you. That could have unintentional undesirable side effects.
RECOMMEND: Change line 1972 to
Write-Host "...ERROR: $fileDir doesn't exist" -ForegroundColor Red -BackgroundColor Black
$global:issues++
return $false
and add at line 2050.5
# Ensure .temp and .user exist
$null = Get-TempPath
$null = Get-UserPath
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.
...\.userdirectory's created by DevCheck but could be a timing thing, or simply one tells DevCheck to only download nuget.exe and not do other things (which happen to create .user)
The nuget.exe is not put on the same ..\.user directory that is created by DevCheck.
Get-UserPath returns {Project-Root}\.user. But nuget.exe is set to be downloaded to .user from the global root path: C:\.user\nuget.exe. This is the file that ends up being created when I run .\tools\DevCheck\DevCheck.ps1 -NugetExeUpdate.
Should this be fixed to use the same .user directory from the project root that is created by DevCheck?
If I don't have the nuget.exe directory created, running
DevCheck.ps1 -NugetExeUpdatefails as the curl command cannot create the file in the download process (by default, if I don't have the.userdirectory).This change makes the script create the directory in case of a first download of the nuget in the computer.
A microsoft employee must use /azp run to validate using the pipelines below.
WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.
For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.