-
Notifications
You must be signed in to change notification settings - Fork 447
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
Enable building "non-official" VMR builds #19491
Changes from 15 commits
9c9fcde
2473cdf
d2a9114
cade20e
5b69a6c
503890d
bac04aa
8f1a12d
e6a7b11
42b338b
5743a1d
0100ee8
ed3d2df
bb20178
bcbbf10
570fb06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,9 +9,11 @@ | |
<RepositoryReference Include="source-build-reference-packages" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ItemGroup Condition="'$(UseOfficialBuildVersioning)' != 'false'"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @jkoritzinsky, do you recall the reason for adding this condition? Is there something that breaks if deployment-tools knows about the git hash in official building versioning mode? Any idea how I can observe this locally? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I did this because (at least to the best of my memory), I didn't want the VMR's git commit hash to be treated as the commit for deployment tools. Alternatively, I did this because the git commit hash info is only provided by the git-info folder in the VMR, which we don't import in dev-versioning mode (and eventually will cease to exist once we ship out of the VMR). Things might work with this condition removed, I'm not sure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! |
||
<EnvironmentVariables Include="LatestCommit=$(GitCommitHash)" /> | ||
</ItemGroup> | ||
Comment on lines
+12
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated but it would be nice if we could change this code path in deployment-tools to depend on GitCommitHash instead of LatestCommit. Then we wouldn't need this extra env var. |
||
|
||
<ItemGroup> | ||
<!-- https://github.com/dotnet/source-build/issues/4115. --> | ||
<EnvironmentVariables Include="PublishWindowsPdb=false" /> | ||
</ItemGroup> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Jeremy Koritzinsky <[email protected]> | ||
Date: Tue, 16 Apr 2024 15:56:07 -0700 | ||
Subject: [PATCH] Condition RIDs on which target RIDs are available when doing | ||
a VMR build | ||
|
||
Backport: https://github.com/dotnet/source-build/issues/3934 | ||
jkoritzinsky marked this conversation as resolved.
Show resolved
Hide resolved
|
||
--- | ||
src/Servers/IIS/IIS/test/testassets/TestTasks/TestTasks.csproj | 1 + | ||
src/Servers/IIS/build/testsite.props | 1 + | ||
.../ServerComparison.TestSites/ServerComparison.TestSites.csproj | 1 + | ||
3 files changed, 3 insertions(+) | ||
|
||
diff --git a/src/Servers/IIS/IIS/test/testassets/TestTasks/TestTasks.csproj b/src/Servers/IIS/IIS/test/testassets/TestTasks/TestTasks.csproj | ||
index f6157b9730..0e4cfec2ec 100644 | ||
--- a/src/Servers/IIS/IIS/test/testassets/TestTasks/TestTasks.csproj | ||
+++ b/src/Servers/IIS/IIS/test/testassets/TestTasks/TestTasks.csproj | ||
@@ -4,6 +4,7 @@ | ||
<OutputType>Exe</OutputType> | ||
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework> | ||
<RuntimeIdentifiers>win-x64;linux-x64;osx-x64</RuntimeIdentifiers> | ||
+ <RuntimeIdentifiers Condition="'$(DotNetBuild)' == 'true'">$(TargetRuntimeIdentifier)</RuntimeIdentifiers> | ||
|
||
<!-- | ||
This is used as a package by ASP.NET benchmarking infrastructure. It is meant for internal use only. See also | ||
diff --git a/src/Servers/IIS/build/testsite.props b/src/Servers/IIS/build/testsite.props | ||
index 8226200d1a..429d233d8e 100644 | ||
--- a/src/Servers/IIS/build/testsite.props | ||
+++ b/src/Servers/IIS/build/testsite.props | ||
@@ -1,6 +1,7 @@ | ||
<Project> | ||
<PropertyGroup> | ||
<RuntimeIdentifiers>$(RuntimeIdentifiers);win-x64;win-x86</RuntimeIdentifiers> | ||
+ <RuntimeIdentifiers Condition="'$(DotNetBuild)' == 'true'">$(TargetRuntimeIdentifier)</RuntimeIdentifiers> | ||
<Platforms>x64;x86</Platforms> | ||
<IISExpressAppHostConfig>$(MSBuildThisFileDirectory)applicationhost.config</IISExpressAppHostConfig> | ||
<IISAppHostConfig>$(MSBuildThisFileDirectory)applicationhost.iis.config</IISAppHostConfig> | ||
diff --git a/src/Servers/testassets/ServerComparison.TestSites/ServerComparison.TestSites.csproj b/src/Servers/testassets/ServerComparison.TestSites/ServerComparison.TestSites.csproj | ||
index a5a54f6c9d..2b3690af74 100644 | ||
--- a/src/Servers/testassets/ServerComparison.TestSites/ServerComparison.TestSites.csproj | ||
+++ b/src/Servers/testassets/ServerComparison.TestSites/ServerComparison.TestSites.csproj | ||
@@ -5,6 +5,7 @@ | ||
<PropertyGroup> | ||
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework> | ||
<RuntimeIdentifiers>win-x86;win-x64;linux-x64;osx-x64</RuntimeIdentifiers> | ||
+ <RuntimeIdentifiers Condition="'$(DotNetBuild)' == 'true'">$(TargetRuntimeIdentifier)</RuntimeIdentifiers> | ||
<InProcessTestSite>true</InProcessTestSite> | ||
</PropertyGroup> | ||
|
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.
is this supposed to be just for testing in the PR? we could also just pass
/p:UseOfficialBuildVersioning=false
through the existingextraProperties
parameterThere 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.
As we already expose this as a new build switch, we should use it here and not pass the msbuild property in. That said, given that this is temporary, I wonder if it's even worth introducing the --dev switch in the scripts.
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.
Given that we're pausing the Unified Build initiative for a bit, I'd rather have a well-documented switch for this important scenario until we decide to flip the default experience to be non-official-versioned instead of a hard-to-remember MSBuild property.