-
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
Conversation
…ng the `-ci` flag on the outer build.
- name: useDevVersions | ||
displayName: True when build output uses dev/CI versioning instead of official build versioning | ||
type: boolean | ||
default: false |
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 existing extraProperties
parameter
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.
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.
…ild. This follows the same pattern as Roslyn, and ensures that the binding redirects in MSBuild kick in on .NET Framework. This also happens to help unblock dotnet/installer#19491 by working around the downgrade problem.
646448d
to
aa8cb3b
Compare
aa8cb3b
to
503890d
Compare
f2b6dc5
to
bac04aa
Compare
…in the VMR (we can revert this once we have a join job)
…dist nupkgs for the components I disabled building
…ngth based on feedback from our sync.
<ItemGroup Condition="'$(UseOfficialBuildVersioning)' != 'false'"> | ||
<EnvironmentVariables Include="LatestCommit=$(GitCommitHash)" /> | ||
</ItemGroup> |
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.
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.
@@ -311,6 +311,7 @@ stages: | |||
container: ${{ variables.ubuntu2204Container }} | |||
targetOS: linux | |||
targetArchitecture: x64 | |||
useDevVersions: true |
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.
Please add comment explaining why this uses dev versions (I assume CI protection) and link to a tracking issue to eventually change that back when we produce our official bits from the VMR.
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.
Are we going to ship assets from this leg? I assumed that this leg came from before we had the Mariner cross build legs (that actually produce assets that we'd ship).
src/SourceBuild/content/build.sh
Outdated
@@ -175,6 +176,9 @@ while [[ $# > 0 ]]; do | |||
-use-mono-runtime) | |||
properties="$properties /p:SourceBuildUseMonoRuntime=true" | |||
;; | |||
-dev) | |||
properties="$properties /p:UseOfficialBuildVersioning=false" |
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.
I wonder why we need a new property for this. Can't we use OfficialBuildId
or OfficialBuild
for this instead? Both are already defined Arcade SDK properties.
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.
OfficialBuildId
and OfficialBuild
aren't the default experience, and we want to keep the VMR as official-build-branded by default at this time.
In the future (as we approach switching to the VMR, have validated testing experiences, etc.) we'll switch the defaults, at which point we can use the traditional Arcade SDK properties.
src/SourceBuild/content/build.sh
Outdated
@@ -39,6 +39,7 @@ usage() | |||
echo " --excludeCIBinarylog Don't output binary log (short: -nobl)" | |||
echo " --prepareMachine Prepare machine for CI run, clean up processes after build" | |||
echo " --use-mono-runtime Output uses the mono runtime" | |||
echo " --dev Use -dev or -ci versioning instead of Microsoft official build versions. Ignored when --source-only is specified." |
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.
Now that we have multiple non source-only settings, thoughts on moving these to a separate section much like we have the source-only settings?
...urceBuild/patches/aspnetcore/0001-Condition-RIDs-on-which-target-RIDs-are-available-wh.patch
Outdated
Show resolved
Hide resolved
@@ -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 comment
The 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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Define --dev argument and make passing CI conditional on actually using the
-ci
flag on the outer build.Fixes dotnet/source-build#3934