Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

[release/3.0] Double publish artifacts in cases of stable builds #8443

Merged
merged 1 commit into from Oct 10, 2019

Conversation

mmitche
Copy link
Member

@mmitche mmitche commented Oct 3, 2019

Stable assets do not publish to stable directories to avoid overwrite issues. But, the dotnet-install script expects the runtime version and directory to match, which they wouldn't. To deal with this, in cases of stable builds, copy all the stable branded bits to files with non-stable names. Thus, the suffixed directory will contain the stable file names (e.g. 3.0.1) as well as suffixed file names that work prior to release day. On release day, the directory is copied or renamed and the dotnet-install script will work with the official versions.

@mmitche mmitche requested a review from dagood October 3, 2019 15:40
Copy link
Member

@dagood dagood left a comment

Choose a reason for hiding this comment

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

I don't understand, doesn't this defeat the point of the original change? I thought release infra (or release people) would perform this copy as part of the release?

/cc @MichaelSimons @mthalman

(Also, it seems like it would be a better implementation to add a new project that doesn't have SuppressFinalPackageVersion set so it can calculate the stable version, rather than using a property function find replace which may unintentionally stabilize nonstable asset filenames.)

@dagood
Copy link
Member

dagood commented Oct 3, 2019

Copy link
Member

@dagood dagood left a comment

Choose a reason for hiding this comment

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

Gave it a bit more reading and I see I misunderstood. This seems fine.

@mmitche
Copy link
Member Author

mmitche commented Oct 3, 2019

(Also, it seems like it would be a better implementation to add a new project that doesn't have SuppressFinalPackageVersion set so it can calculate the stable version, rather than using a property function find replace which may unintentionally stabilize nonstable asset filenames.)

This will only turn the stable asset file names into non-stable (if they don't already exist). I also don't like setting SuppressFinalPackageVersion. I am thinking I'll tweak the version props so that it provides both suffixed and non-suffixed versions for use where needed.

@dagood
Copy link
Member

dagood commented Oct 3, 2019

Ignore that, it was based on the misunderstanding that this was publishing to Runtime/3.0.1/foo and Runtime/3.0.1-etc-1234/foo.

@mmitche
Copy link
Member Author

mmitche commented Oct 3, 2019

Ignore that, it was based on the misunderstanding that this was publishing to Runtime/3.0.1/foo and Runtime/3.0.1-etc-1234/foo.

Nope:

  • Runtime/3.0-etc/foo-3.0
  • Runtime/3.0-etc/foo-3.0-etc

@dagood
Copy link
Member

dagood commented Oct 3, 2019

Yeah, my mistake was pretty obvious after actually reading the code comment. 😄

@mmitche
Copy link
Member Author

mmitche commented Oct 4, 2019

@dagood I tweaked this a bit to avoid some corner cases and reworked the version props a bit so I don't have to override the IncludePrereleaseLabel... property.

<ProductVersionSuffix Condition="'$(IncludePreReleaseLabelInPackageVersion)' == 'true'">-$(VersionSuffix)</ProductVersionSuffix>
<ProductBandVersion Condition="'$(ProductBandVersion)' == ''">$(MajorVersion).$(MinorVersion)</ProductBandVersion>
<ProductionVersion Condition="'$(ProductionVersion)' == ''">$(ProductBandVersion).$(PatchVersion)</ProductionVersion>
<ProductVersion>$(ProductionVersion)$(ProductVersionSuffix)</ProductVersion>
<ProductVersionWithSuffix>$(ProductionVersion)</ProductVersionWithSuffix>
<ProductVersionWithSuffix Condition="'$(IncludePreReleaseLabelInPackageVersion)' != 'true'">$(ProductionVersion)-$(VersionSuffix)</ProductVersionWithSuffix>
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little confused this works, I would expect Arcade to unset VersionSuffix based on https://github.com/dotnet/arcade/blob/ca919f224b58b17e48d8786dd555ae9a1f6cdfa3/src/Microsoft.DotNet.Arcade.Sdk/tools/Version.BeforeCommonTargets.targets#L103-L112, and this would end up as 3.0.0-.

Copy link
Member

Choose a reason for hiding this comment

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

And I think Arcade would have to introduce a new property to let us do this, since IIRC VersionSuffix is used directly by the NuGet pack targets (and that's why Arcade needs to clear it).

Stable assets do not publish to stable directories to avoid overwrite issues. But, the dotnet-install script expects the runtime version and directory to match, which they wouldn't. To deal with this, in cases of stable builds, copy all the stable branded bits to files with non-stable names.  Thus, the suffixed directory will contain the stable file names (e.g. 3.0.1) as well as suffixed file names that work prior to release day. On release day, the directory is copied or renamed and the dotnet-install script will work with the official versions.
@mmitche
Copy link
Member Author

mmitche commented Oct 9, 2019

@dagood Finally working and doing what it is supposed to.

@mmitche mmitche merged commit 34a7a30 into dotnet:release/3.0 Oct 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants