Skip to content

Place file-based app artifacts into repo's artifacts dir if used #49863

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+126 −34
Draft

Place file-based app artifacts into repo's artifacts dir if used #49863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion documentation/general/dotnet-run-file.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ have the shared `.cs` files source-included via `<Compile Include="../Shared/**/

## Build outputs

Build outputs are placed under a subdirectory whose name is hashed file path of the entry point
If [artifacts output layout][artifacts-output] is enabled, build outputs of the file-based app are placed there
(except caching markers which are placed in the global temp directory described next).
Comment on lines +181 to +182
Copy link
Member

Choose a reason for hiding this comment

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

I admit I'm not understanding the motivation of this change here. Do we expect users to then be looking at these outputs, or is this just to leverage the existing things that exclude artifact outputs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Leveraging existing default item excludes is one motivation. Another is that all repo artifacts are in one place - so the same motivations should apply as to why artifacts layout exists in the first place, e.g., you can clean all repo artifacts together.

Copy link
Member

Choose a reason for hiding this comment

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

Will the artifacts at least be placed under a runfile subfolder or some such? Otherwise I worry a bit that the artifacts directory will be polluted.

In particular, if a file is not part of a loaded project (e.g. it was not loaded yet or it was deliberately unloaded), the IDE currently will just treat it as a file-based program in many cases, it will do a design time build on it in order to get basic set of references and so on. It could be confusing if the artifacts folder contains many such directories simply due to the way the user was browsing.

Copy link
Member Author

Choose a reason for hiding this comment

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

In particular, if a file is not part of a loaded project (e.g. it was not loaded yet or it was deliberately unloaded), the IDE currently will just treat it as a file-based program in many cases, it will do a design time build on it in order to get basic set of references and so on.

Hm, that's unfortunate, I haven't realized that.

But I've also noticed that artifacts layout currently seems to assume that projects have unique names. So each project gets a folder like artifacts/[ProjectName]. Script's "project name" is the script's file name. That can lead to two scripts sharing the same output location (even more likely given the IDE behavior you described). So I guess we need to create some unique names or directories for scripts.

Otherwise, build outputs are placed under a subdirectory whose name is hashed file path of the entry point
inside a temp or app data directory which should be owned by and unique to the current user per [runtime guidelines][temp-guidelines].
The subdirectory is created by the SDK CLI with permissions restricting access to it to the current user (`0700`) and the run fails if that is not possible.
Note that it is possible for multiple users to run the same file-based program, however each user's run uses different build artifacts since the base directory is unique per user.
Expand Down
3 changes: 1 addition & 2 deletions src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,8 +650,7 @@ public static void WriteProjectFile(
<Project>

<PropertyGroup>
<IncludeProjectNameInArtifactsPaths>false</IncludeProjectNameInArtifactsPaths>
<ArtifactsPath>{EscapeValue(artifactsPath)}</ArtifactsPath>
<FileBasedAppArtifactsPath>{EscapeValue(artifactsPath)}</FileBasedAppArtifactsPath>
<PublishDir>artifacts/$(MSBuildProjectName)</PublishDir>
</PropertyGroup>

Expand Down
2 changes: 1 addition & 1 deletion src/Tasks/Microsoft.NET.Build.Tasks/sdk/UseArtifactsOutputPath.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Copyright (c) .NET Foundation. All rights reserved.
If the .props file is not imported here, it will be imported from Microsoft.NET.DefaultOutputPaths.targets, so that artifacts output
properties can be set directly in the project file too (only in that case they won't affect the intermediate output). -->
<Import Project="$(MSBuildThisFileDirectory)..\targets\Microsoft.NET.DefaultArtifactsPath.props"
Condition="'$(UseArtifactsOutput)' == 'true' Or '$(ArtifactsPath)' != ''"/>
Condition="'$(UseArtifactsOutput)' == 'true' Or '$(ArtifactsPath)' != '' Or '$(FileBasedAppArtifactsPath)' != ''"/>

<PropertyGroup Condition="'$(UseArtifactsOutput)' == 'true'">
<UseArtifactsIntermediateOutput Condition="'$(UseArtifactsIntermediateOutput)' == ''">true</UseArtifactsIntermediateOutput>
Expand Down
8 changes: 8 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.DefaultArtifactsPath.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,12 @@ Copyright (c) .NET Foundation. All rights reserved.
<ArtifactsPath>$(MSBuildProjectDirectory)\artifacts</ArtifactsPath>
<_ArtifactsPathLocationType>ProjectFolder</_ArtifactsPathLocationType>
</PropertyGroup>

<!-- Use FileBasedAppArtifactsPath if set -->
<PropertyGroup Condition="'$(ArtifactsPath)' == '' And '$(FileBasedAppArtifactsPath)' != ''">
<UseArtifactsOutput Condition="'$(UseArtifactsOutput)' == '' And '$(UsingMicrosoftArtifactsSdk)' != 'true'">true</UseArtifactsOutput>
<ArtifactsPath>$(FileBasedAppArtifactsPath)</ArtifactsPath>
<IncludeProjectNameInArtifactsPaths Condition="'$(IncludeProjectNameInArtifactsPaths)' == ''">false</IncludeProjectNameInArtifactsPaths>
<_ArtifactsPathLocationType>FileBasedApp</_ArtifactsPathLocationType>
</PropertyGroup>
</Project>
143 changes: 113 additions & 30 deletions test/dotnet.Tests/CommandTests/Run/RunFileTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.DotNet.Cli.Run.Tests;

public sealed class RunFileTests(ITestOutputHelper log) : SdkTest(log)
{
private static readonly string s_program = """
private static readonly string s_program = /* lang=C#-Test */ """
if (args.Length > 0)
{
Console.WriteLine("echo args:" + string.Join(";", args));
Expand All @@ -27,15 +27,15 @@ public sealed class RunFileTests(ITestOutputHelper log) : SdkTest(log)
#endif
""";

private static readonly string s_programDependingOnUtil = """
private static readonly string s_programDependingOnUtil = /* lang=C#-Test */ """
if (args.Length > 0)
{
Console.WriteLine("echo args:" + string.Join(";", args));
}
Console.WriteLine("Hello, " + Util.GetMessage());
""";

private static readonly string s_util = """
private static readonly string s_util = /* lang=C#-Test */ """
static class Util
{
public static string GetMessage()
Expand All @@ -45,6 +45,29 @@ public static string GetMessage()
}
""";

private static readonly string s_programReadingEmbeddedResource = /* lang=C#-Test */ """
var assembly = System.Reflection.Assembly.GetExecutingAssembly();
var resourceName = assembly.GetManifestResourceNames().SingleOrDefault();

if (resourceName is null)
{
Console.WriteLine("Resource not found");
return;
}

using var stream = assembly.GetManifestResourceStream(resourceName)!;
using var reader = new System.Resources.ResourceReader(stream);
Console.WriteLine(reader.Cast<System.Collections.DictionaryEntry>().Single());
""";

private static readonly string s_resx = """
<root>
<data name="MyString">
<value>TestValue</value>
</data>
</root>
""";

private static readonly string s_consoleProject = $"""
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
Expand Down Expand Up @@ -950,26 +973,8 @@ public void BinaryLog_EvaluationData()
public void EmbeddedResource()
{
var testInstance = _testAssetsManager.CreateTestDirectory();
string code = """
using var stream = System.Reflection.Assembly.GetExecutingAssembly().GetManifestResourceStream("Program.Resources.resources");

if (stream is null)
{
Console.WriteLine("Resource not found");
return;
}

using var reader = new System.Resources.ResourceReader(stream);
Console.WriteLine(reader.Cast<System.Collections.DictionaryEntry>().Single());
""";
File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), code);
File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), """
<root>
<data name="MyString">
<value>TestValue</value>
</data>
</root>
""");
File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), s_programReadingEmbeddedResource);
File.WriteAllText(Path.Join(testInstance.Path, "Resources.resx"), s_resx);

new DotnetCommand(Log, "run", "Program.cs")
.WithWorkingDirectory(testInstance.Path)
Expand All @@ -982,7 +987,7 @@ public void EmbeddedResource()
// This behavior can be overridden.
File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), $"""
#:property EnableDefaultEmbeddedResourceItems=false
{code}
{s_programReadingEmbeddedResource}
""");

new DotnetCommand(Log, "run", "Program.cs")
Expand All @@ -994,6 +999,35 @@ Resource not found
""");
}

/// <summary>
/// <c>.resx</c> files in <c>./artifacts/</c> should not be included.
/// Part of <see href="https://github.com/dotnet/sdk/issues/49826"/>.
/// </summary>
[Fact]
public void EmbeddedResource_InRepoArtifacts()
{
var testInstance = _testAssetsManager.CreateTestDirectory();
File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), s_programReadingEmbeddedResource);
File.WriteAllText(Path.Join(testInstance.Path, "Directory.Build.props"), """
<Project>
<PropertyGroup>
<UseArtifactsOutput>true</UseArtifactsOutput>
</PropertyGroup>
</Project>
""");
var dir = Path.Join(testInstance.Path, "artifacts", "obj", "AnotherApp");
Directory.CreateDirectory(dir);
File.WriteAllText(Path.Join(dir, "Resources.resx"), s_resx);

new DotnetCommand(Log, "run", "Program.cs", "-bl")
.WithWorkingDirectory(testInstance.Path)
.Execute()
.Should().Pass()
.And.HaveStdOut("""
Resource not found
""");
}

[Fact]
public void NoRestore_01()
{
Expand Down Expand Up @@ -1408,6 +1442,58 @@ public void ArtifactsDirectory_Permissions()
.Should().Be(actualMode, artifactsDir);
}

[Fact]
public void ArtifactsPath()
{
var testInstance = _testAssetsManager.CreateTestDirectory();
var programPath = Path.Join(testInstance.Path, "Program.cs");
File.WriteAllText(programPath, s_program);

var globalArtifactsDir = VirtualProjectBuildingCommand.GetArtifactsPath(programPath);
if (Directory.Exists(globalArtifactsDir)) Directory.Delete(globalArtifactsDir, recursive: true);

new DirectoryInfo(Path.Join(testInstance.Path, "artifacts")).Should().NotExist();
new DirectoryInfo(Path.Join(testInstance.Path, "bin")).Should().NotExist();

new DotnetCommand(Log, "build", "Program.cs")
.WithWorkingDirectory(testInstance.Path)
.Execute()
.Should().Pass();

new DirectoryInfo(globalArtifactsDir).EnumerateDirectories().Should().NotBeEmpty();
new DirectoryInfo(Path.Join(testInstance.Path, "artifacts")).Should().NotExist();
new DirectoryInfo(Path.Join(testInstance.Path, "bin")).Should().NotExist();
}

/// <summary>
/// When the surrounding repo uses artifacts layout, file-based apps place their artifacts there.
/// </summary>
[Fact]
public void ArtifactsPath_ReusedFromRepo()
{
var testInstance = _testAssetsManager.CreateTestDirectory();
var programPath = Path.Join(testInstance.Path, "Program.cs");
File.WriteAllText(programPath, s_program);
File.WriteAllText(Path.Join(testInstance.Path, "Directory.Build.props"), """
<Project>
<PropertyGroup>
<UseArtifactsOutput>true</UseArtifactsOutput>
</PropertyGroup>
</Project>
""");

new DirectoryInfo(Path.Join(testInstance.Path, "artifacts")).Should().NotExist();

new DotnetCommand(Log, "build", "Program.cs")
.WithWorkingDirectory(testInstance.Path)
.Execute()
.Should().Pass();

// We still put our marker files into the global artifacts directory, but it should not contain any subdirectories.
new DirectoryInfo(VirtualProjectBuildingCommand.GetArtifactsPath(programPath)).EnumerateDirectories().Should().BeEmpty();
new FileInfo(Path.Join(testInstance.Path, "artifacts", "bin", "Program", "debug", "Program.dll")).Should().Exist();
}

[Fact]
public void LaunchProfile()
{
Expand Down Expand Up @@ -1843,8 +1929,7 @@ public void Api()
<Project>

<PropertyGroup>
<IncludeProjectNameInArtifactsPaths>false</IncludeProjectNameInArtifactsPaths>
<ArtifactsPath>/artifacts</ArtifactsPath>
<FileBasedAppArtifactsPath>/artifacts</FileBasedAppArtifactsPath>
<PublishDir>artifacts/$(MSBuildProjectName)</PublishDir>
</PropertyGroup>

Expand Down Expand Up @@ -1922,8 +2007,7 @@ public void Api_Diagnostic_01()
<Project>

<PropertyGroup>
<IncludeProjectNameInArtifactsPaths>false</IncludeProjectNameInArtifactsPaths>
<ArtifactsPath>/artifacts</ArtifactsPath>
<FileBasedAppArtifactsPath>/artifacts</FileBasedAppArtifactsPath>
<PublishDir>artifacts/$(MSBuildProjectName)</PublishDir>
</PropertyGroup>

Expand Down Expand Up @@ -1994,8 +2078,7 @@ public void Api_Diagnostic_02()
<Project>

<PropertyGroup>
<IncludeProjectNameInArtifactsPaths>false</IncludeProjectNameInArtifactsPaths>
<ArtifactsPath>/artifacts</ArtifactsPath>
<FileBasedAppArtifactsPath>/artifacts</FileBasedAppArtifactsPath>
<PublishDir>artifacts/$(MSBuildProjectName)</PublishDir>
</PropertyGroup>

Expand Down
Loading