Skip to content

Commit 4eb99cb

Browse files
author
Girish Konda
committed
Include Copilot CLI runtime assets in PackAsTool packages
Tool packages are produced from the publish layout, but the SDK registered its Copilot CLI runtime assets only for build output. `dotnet pack --no-build` therefore produced a tool package with no `runtimes/<rid>/native` payload, and the installed tool failed with "Copilot runtime wrapper not found". Register the existing runtime asset items before publish item collection as well and give them `CopyToPublishDirectory`, so the assets already staged for build output also reach the publish layout the tool package is built from.
1 parent d5c9d06 commit 4eb99cb

3 files changed

Lines changed: 117 additions & 15 deletions

File tree

‎dotnet/src/build/GitHub.Copilot.SDK.targets‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,11 @@
215215
<Delete Files="$(_CopilotOutputDir)\$(_CopilotRuntimeLib)" Condition="!Exists('$(_CopilotRuntimeNodePath)')" />
216216
</Target>
217217

218-
<!-- Register the compatibility entrypoint as content so it flows through project references.
218+
<!-- Register the compatibility entrypoint as content so it flows through project references
219+
and publish output. Tool packages (PackAsTool) are built from the publish layout, so the
220+
runtime assets must be registered for publish as well as build output.
219221
Same gating semantics as _CopyCopilotCliToOutput. -->
220-
<Target Name="_RegisterCopilotCliForCopy" BeforeTargets="GetCopyToOutputDirectoryItems" DependsOnTargets="_DownloadCopilotCli" Condition="'$(TargetFramework)' != '' And ('$(CopilotCliBinaryPath)' != '' Or '$(CopilotSkipCliDownload)' != 'true') And '$(_CopilotPlatform)' != ''">
222+
<Target Name="_RegisterCopilotCliForCopy" BeforeTargets="GetCopyToOutputDirectoryItems;GetCopyToPublishDirectoryItems" DependsOnTargets="_DownloadCopilotCli" Condition="'$(TargetFramework)' != '' And ('$(CopilotCliBinaryPath)' != '' Or '$(CopilotSkipCliDownload)' != 'true') And '$(_CopilotPlatform)' != ''">
221223
<PropertyGroup>
222224
<_CopilotCacheDir Condition="'$(_CopilotCacheDir)' == ''">$(IntermediateOutputPath)copilot-cli\$(CopilotCliVersion)\$(_CopilotPlatform)</_CopilotCacheDir>
223225
<_CopilotCliBinaryPath Condition="'$(_CopilotCliBinaryPath)' == ''">$(_CopilotCacheDir)\prebuilds\$(_CopilotPlatform)\$(_CopilotRuntimeWrapper)</_CopilotCliBinaryPath>
@@ -239,22 +241,27 @@
239241
Condition="Exists('$(_CopilotRuntimeWrapperPath)') And Exists('$(_CopilotRuntimeNodePath)')" />
240242
<ContentWithTargetPath Include="$(_CopilotCliBinaryPath)"
241243
TargetPath="runtimes\$(_CopilotRid)\native\$(_CopilotBinary)"
242-
CopyToOutputDirectory="PreserveNewest" />
244+
CopyToOutputDirectory="PreserveNewest"
245+
CopyToPublishDirectory="PreserveNewest" />
243246
<ContentWithTargetPath Include="$(_CopilotExplicitCliMarker)"
244247
TargetPath="runtimes\$(_CopilotRid)\native\.copilot-explicit-cli"
245248
CopyToOutputDirectory="PreserveNewest"
249+
CopyToPublishDirectory="PreserveNewest"
246250
Condition="'$(CopilotCliBinaryPath)' != ''" />
247251
<!-- FFI cdylib (only when the tarball ships it); see _CopyCopilotCliToOutput. -->
248252
<ContentWithTargetPath Include="$(_CopilotRuntimeNodePath)"
249253
TargetPath="runtimes\$(_CopilotRid)\native\$(_CopilotRuntimeLib)"
250254
CopyToOutputDirectory="PreserveNewest"
255+
CopyToPublishDirectory="PreserveNewest"
251256
Condition="Exists('$(_CopilotRuntimeNodePath)')" />
252257
<ContentWithTargetPath Include="@(_CopilotRuntimeRootAsset)"
253258
TargetPath="runtimes\$(_CopilotRid)\native\%(RecursiveDir)%(Filename)%(Extension)"
254-
CopyToOutputDirectory="PreserveNewest" />
259+
CopyToOutputDirectory="PreserveNewest"
260+
CopyToPublishDirectory="PreserveNewest" />
255261
<ContentWithTargetPath Include="@(_CopilotRuntimePrebuildAsset)"
256262
TargetPath="runtimes\$(_CopilotRid)\native\%(RecursiveDir)%(Filename)%(Extension)"
257-
CopyToOutputDirectory="PreserveNewest" />
263+
CopyToOutputDirectory="PreserveNewest"
264+
CopyToPublishDirectory="PreserveNewest" />
258265
</ItemGroup>
259266
</Target>
260267
</Project>

‎dotnet/test/GitHub.Copilot.SDK.Test.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
<Compile Include="..\src\Polyfills\*.cs" Link="Polyfills\%(Filename)%(Extension)" />
3939
<PackageReference Include="Microsoft.Bcl.Memory" />
4040
<PackageReference Include="System.Net.Http.Json" />
41+
<Reference Include="System.IO.Compression" />
4142
<Reference Include="System.Net.Http" />
4243
</ItemGroup>
4344

‎dotnet/test/Unit/MSBuildTargetsTests.cs‎

Lines changed: 104 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
using System.Collections.Concurrent;
66
using System.Diagnostics;
7+
using System.IO.Compression;
78
using System.Net;
89
using System.Net.Sockets;
910
using System.Runtime.CompilerServices;
@@ -31,6 +32,11 @@ public class MSBuildTargetsTests
3132
private static readonly string RuntimeWrapperName =
3233
OperatingSystem.IsWindows() ? "copilot-runtime.exe" : "copilot-runtime";
3334

35+
private static readonly string RuntimeLibraryName =
36+
OperatingSystem.IsWindows() ? "copilot_runtime.dll"
37+
: OperatingSystem.IsMacOS() ? "libcopilot_runtime.dylib"
38+
: "libcopilot_runtime.so";
39+
3440
[Fact]
3541
public async Task PreinstalledCliBinaryPath_IsHonored_DownloadSkipped_AndCopiedToOutput()
3642
{
@@ -236,6 +242,32 @@ public async Task RuntimePackageAssets_AreFilteredAndCopiedToOutput()
236242
Assert.False(File.Exists(sandbox.ExpectedRuntimeAsset("obsolete", "tool")));
237243
}
238244

245+
[Fact]
246+
public async Task PackAsTool_NoBuild_IncludesRuntimeAssetsInToolPackage()
247+
{
248+
using var sandbox = MSBuildSandbox.Create(packAsTool: true);
249+
var preinstalled = sandbox.WritePreinstalledBinary("fake-cli-contents");
250+
sandbox.WriteRuntimeCacheAsset("prebuilds", GetReleasePlatform(), "runtime.node", "runtime");
251+
sandbox.WriteRuntimeCacheAsset("prebuilds", GetReleasePlatform(), RuntimeWrapperName, "wrapper");
252+
sandbox.WriteRuntimeCacheAsset("ripgrep", "bin", GetReleasePlatform(), "rg", "ripgrep");
253+
254+
var result = await sandbox.PackNoBuildAsync(new Dictionary<string, string>
255+
{
256+
["CopilotCliBinaryPath"] = preinstalled,
257+
});
258+
259+
Assert.True(result.Succeeded, result.FailureMessage());
260+
261+
var rid = MSBuildSandbox.GetPortableRid();
262+
var entries = sandbox.GetPackageEntries();
263+
var nativePath = $"tools/net8.0/{rid}/runtimes/{rid}/native";
264+
Assert.Contains($"{nativePath}/{BinaryName}", entries);
265+
Assert.Contains($"{nativePath}/{RuntimeWrapperName}", entries);
266+
Assert.Contains($"{nativePath}/runtime.node", entries);
267+
Assert.Contains($"{nativePath}/{RuntimeLibraryName}", entries);
268+
Assert.Contains($"{nativePath}/ripgrep/bin/{GetReleasePlatform()}/rg", entries);
269+
}
270+
239271
[Fact]
240272
public async Task PreinstalledCliBinaryPath_NonExistentFile_FailsWithActionableError()
241273
{
@@ -327,25 +359,46 @@ private MSBuildSandbox(string projectDir)
327359
ProjectDir = projectDir;
328360
}
329361

330-
public static MSBuildSandbox Create()
362+
public static MSBuildSandbox Create(bool packAsTool = false)
331363
{
332364
var dir = Path.Combine(Path.GetTempPath(), "copilot-sdk-targets-test-" + Guid.NewGuid().ToString("N"));
333365
Directory.CreateDirectory(dir);
334366

367+
// Tool packages are produced from the publish layout, so the tool variant
368+
// exercises the packaging path reported in issue #2067.
369+
var toolProperties = packAsTool
370+
? $"""
371+
372+
<OutputType>Exe</OutputType>
373+
<PackAsTool>true</PackAsTool>
374+
<PackageId>CopilotSdkTargetsTest.Tool</PackageId>
375+
<Version>1.0.0</Version>
376+
<ToolCommandName>copilot-sdk-targets-test</ToolCommandName>
377+
<RuntimeIdentifiers>{GetPortableRid()}</RuntimeIdentifiers>
378+
<AppendRuntimeIdentifierToOutputPath>false</AppendRuntimeIdentifierToOutputPath>
379+
<SelfContained>false</SelfContained>
380+
<UseAppHost>false</UseAppHost>
381+
"""
382+
: string.Empty;
383+
335384
// Minimal class library that imports the SDK targets with a pinned fake
336385
// CopilotCliVersion so the targets do not need the generated props file.
337386
var csproj = $"""
338387
<Project Sdk="Microsoft.NET.Sdk">
339388
<PropertyGroup>
340389
<TargetFramework>net8.0</TargetFramework>
341390
<CopilotCliVersion>0.0.0-test</CopilotCliVersion>
342-
<EnableDefaultCompileItems>true</EnableDefaultCompileItems>
391+
<EnableDefaultCompileItems>true</EnableDefaultCompileItems>{toolProperties}
343392
</PropertyGroup>
344393
<Import Project="{TargetsFilePath}" />
345394
</Project>
346395
""";
347396
File.WriteAllText(Path.Combine(dir, "App.csproj"), csproj);
348-
File.WriteAllText(Path.Combine(dir, "Stub.cs"), "namespace CopilotSdkTargetsTest { internal static class Stub { } }\n");
397+
File.WriteAllText(
398+
Path.Combine(dir, "Stub.cs"),
399+
packAsTool
400+
? "namespace CopilotSdkTargetsTest { internal static class Stub { private static void Main() { } } }\n"
401+
: "namespace CopilotSdkTargetsTest { internal static class Stub { } }\n");
349402

350403
return new MSBuildSandbox(dir);
351404
}
@@ -439,9 +492,46 @@ public void WriteStaleOutputRuntimeAsset(params string[] pathAndContents)
439492
string.Join(Path.DirectorySeparatorChar.ToString(), relativeParts) + Environment.NewLine);
440493
}
441494

442-
public async Task<BuildResult> BuildAsync(IDictionary<string, string> properties)
495+
public async Task<BuildResult> BuildAsync(IDictionary<string, string> properties) =>
496+
await RunAsync("build --nologo -clp:NoSummary", properties);
497+
498+
/// <summary>
499+
/// Builds, publishes, then packs without rebuilding — the tool packaging sequence
500+
/// used by CI pipelines and reported in issue #2067.
501+
/// </summary>
502+
public async Task<BuildResult> PackNoBuildAsync(IDictionary<string, string> properties)
503+
{
504+
// Pin the configuration: build defaults to Debug while publish and pack default
505+
// to Release, which would otherwise stage and read different output trees.
506+
var rid = GetPortableRid();
507+
var build = await RunAsync($"build --nologo -clp:NoSummary -c Debug -r {rid}", properties);
508+
if (!build.Succeeded)
509+
{
510+
return build;
511+
}
512+
513+
var publish = await RunAsync($"publish --nologo -clp:NoSummary -c Debug -r {rid}", properties);
514+
if (!publish.Succeeded)
515+
{
516+
return publish;
517+
}
518+
519+
return await RunAsync(
520+
$"pack --nologo -clp:NoSummary -c Debug -r {rid} --no-build -o packages",
521+
properties);
522+
}
523+
524+
public List<string> GetPackageEntries()
525+
{
526+
var package = Directory.GetFiles(Path.Combine(ProjectDir, "packages"), "*.nupkg").Single();
527+
using var stream = File.OpenRead(package);
528+
using var archive = new ZipArchive(stream, ZipArchiveMode.Read);
529+
return archive.Entries.Select(entry => entry.FullName).ToList();
530+
}
531+
532+
private async Task<BuildResult> RunAsync(string command, IDictionary<string, string> properties)
443533
{
444-
var args = new StringBuilder("build --nologo -clp:NoSummary");
534+
var args = new StringBuilder(command);
445535
foreach (var (key, value) in properties)
446536
{
447537
// Quote the value so paths with spaces are preserved.
@@ -482,7 +572,7 @@ public async Task<BuildResult> BuildAsync(IDictionary<string, string> properties
482572
catch (InvalidOperationException) { /* process already exited */ }
483573
catch (NotSupportedException) { /* not supported on this platform */ }
484574
catch (System.ComponentModel.Win32Exception) { /* kill failed; best effort */ }
485-
throw new TimeoutException($"dotnet build did not complete within the timeout for args: {args}");
575+
throw new TimeoutException($"dotnet did not complete within the timeout for args: {args}");
486576
}
487577

488578
return new BuildResult(
@@ -499,26 +589,30 @@ public void Dispose()
499589
catch (UnauthorizedAccessException) { /* cleanup is best effort */ }
500590
}
501591

502-
private static string GetPortableRid()
592+
public static string GetPortableRid()
503593
{
594+
// Must agree with GetReleasePlatform(), which names the seeded runtime cache:
595+
// both describe the SDK actually running the build, so an emulated process
596+
// (x64 on arm64) pins the RID it seeded assets for.
597+
var arch = System.Runtime.InteropServices.RuntimeInformation.ProcessArchitecture;
504598
if (OperatingSystem.IsWindows())
505599
{
506-
return System.Runtime.InteropServices.RuntimeInformation.OSArchitecture switch
600+
return arch switch
507601
{
508602
System.Runtime.InteropServices.Architecture.Arm64 => "win-arm64",
509603
_ => "win-x64",
510604
};
511605
}
512606
if (OperatingSystem.IsMacOS())
513607
{
514-
return System.Runtime.InteropServices.RuntimeInformation.OSArchitecture switch
608+
return arch switch
515609
{
516610
System.Runtime.InteropServices.Architecture.Arm64 => "osx-arm64",
517611
_ => "osx-x64",
518612
};
519613
}
520614
var os = IsMusl() ? "linux-musl" : "linux";
521-
var architecture = System.Runtime.InteropServices.RuntimeInformation.OSArchitecture switch
615+
var architecture = arch switch
522616
{
523617
System.Runtime.InteropServices.Architecture.Arm64 => "arm64",
524618
_ => "x64",

0 commit comments

Comments
 (0)