Skip to content

Conversation

sharwell
Copy link
Contributor

@sharwell sharwell commented Jun 19, 2024

  • Only hook up VSIX up-to-date check when VSToolsPath is defined
  • Ensure globalPackagesFolder ends with a directory separator
  • Update Sign.proj to use a NuGet-defined package cache location instead of assuming a prior step placed it at a hard-coded location

In other cases, the VSIX container properties are not defined, leading
to failures in early design time build scenarios.
Make sure to include information about inputs and outputs to CreateVsixContainer for fast up-to-date check.
-->
<PropertyGroup>
<PropertyGroup Condition="'$(VSToolsPath)' != '' and ('$(IsVsixProject)' == 'true' or '$(GeneratePkgDefFile)' == 'true')">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 This is the same condition used above for conditional import of VisualStudio.ImportSdk.targets

if ($env:NUGET_PACKAGES -ne $null) {
# Join-Path with an empty child path effectively adds a trailing directory separator only if one does not
# already exist
$env:NUGET_PACKAGES = Join-Path $env:NUGET_PACKAGES ''
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 This makes me a bit uncomfortable; let me know if it's acceptable or if there is a better way to do it.

@sharwell sharwell marked this pull request as ready for review June 19, 2024 16:31
if ($env:NUGET_PACKAGES -ne $null) {
# Join-Path with an empty child path effectively adds a trailing directory separator only if one does not
# already exist
$env:NUGET_PACKAGES = Join-Path $env:NUGET_PACKAGES ''
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$env:NUGET_PACKAGES = Join-Path $env:NUGET_PACKAGES ''
$env:NUGET_PACKAGES = Join-Path $env:NUGET_PACKAGES $([IO.Path]::DirectorySeparatorChar)

This appears to do the correct thing locally.

@sharwell sharwell marked this pull request as draft June 19, 2024 19:10
Condition="'$(Restore)' == 'true'"/>

<!--
Restore built-in tools for signing.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 Restoring Sign.proj is the step that defines NuGetPackageRoot for use within it.

@premun
Copy link
Member

premun commented Jun 21, 2024

@sharwell a quick note - we squash merge in Arcade instead of merging only

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.

3 participants