Making arcade self buildable

[jcagme]

Solves: #917

Test build: https://dnceng.visualstudio.com/internal/_build/results?buildId=49482

[mmitche]

Is this Windows only?

[tmat]

🕐

[tmat]

Why is this needed? You can just pass the local directory path directly to RestoreSources, no?

[jcagme]

I tried just passing /p:RestoreResouces and the initial Arcade SDK is not restored. Thing like Versions.props have $(RestoreResources) and that is how the value is propagated. NuGet.config doesn't have this, also, a source in NuGet.config is not just the path but also a key name so I wonder how would /p:RestoreResouces work for NuGet.props

[chcosta]

I don't think the source for a custom SDK can come from a props file or property today, I think only NuGet.config is supported as the source for a custom SDK. At least that was my understanding like 6 months ago, perhaps this limitation has been fixed?

[jcagme]

I think that is the case still

[tmat]

How about setting /p:RestoreConfigFile=Local.NuGet.config, where Local.NuGet.config would be one that points to the local package (you can generate the file to temp dir in the PS script).

[jcagme]

Test build with new Validation leg here: https://dnceng.visualstudio.com/internal/_build/results?buildId=52048

[jcagme]

@tmat @chcosta after our several chats and conversations the following validates itself by using packages built locally. These packages are located in artifacts/validatesdk and will be restored from there by the official part of the build. PTAL

Test build: https://dnceng.visualstudio.com/internal/_build/results?buildId=53341

[jcagme]

@tmat I've added eng/Tools.props as suggested. Also, addressed the other minor details. Can you please confirm this is good to merge?

[jcagme]

@chcosta can you take a final look please?

[chcosta]

You might just do something like this for "_Script"

variables:
- name: _Script:
  value: eng\common\cibuild.cmd
- name: _ValidateSdkArgs
  value: ''
strategy:
  matrix:
    Build_Debug:
      _BuildConfig: Debug
      _PublishType: none
      _SignType: test
      _DotNetPublishToBlobFeed: false
    Build_Release:
      _BuildConfig: Release
      ${{ if or(eq(variables['System.TeamProject'], 'public'), in(variables['Build.Reason'], 'PullRequest')) }}:
        _PublishType: none
        _SignType: test
        _DotNetPublishToBlobFeed : false
      ${{ if and(ne(variables['System.TeamProject'], 'public'), notin(variables['Build.Reason'], 'PullRequest')) }}:
        _PublishType: blob
        _SignType: real
        _DotNetPublishToBlobFeed : true
        _Script: eng\validate-sdk.cmd
        _ValidateSdkArgs: -gitHubPat $(BotAccount-dotnet-maestro-bot-PAT) -barToken $(MaestroAccessToken)

I think that will resolve correctly.

[jcagme]

Yeah, way better!

[chcosta]

This feels like a strange and fragile coupling. Couldn't this just be something like...

<PropertyGroup>
  <ResourceSources>
    $(RestoreSources);
    $(AdditionalRestoreSources);
  </ResourceSources>
</PropertyGroup>

and then this line in validate-sdk.ps1...

  . .\common\build.ps1 -configuration $configuration @Args  /p:ArcadeSelfTest=true

… becomes...

  . .\common\build.ps1 -configuration $configuration @Args /p:AdditionalRestoreSources=$packagesSource 

[chcosta]

why the double backslash?

[jcagme]

It was originally a typo but when fixing it I realized that somewhere in the process last \ is removed ending up in folders like "validatesdktoolset". I searched the source and could not find a place where this could happen. $properties in build.ps1 does receive the correct value so is probably something MSBuild does is stripping that last '' from the path. @tmat do you know what might be causing this?

[chcosta]

Use Join-Path

[jcagme]

Using Join-Path had no effect. Again, I think this is been removed in MSBuild rather than at the PS1 level

[chcosta]

You may need to do a check at https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Arcade.Sdk/tools/RepoLayout.props#L54 and ensure the path provided properly ends with a \

[jcagme]

Actually based on the last change we won't need to set the ArtifactDir anymore so this is a no-op now

[tmat]

This is not needed anymore.

[tmat]

[tmat]