Skip to content

Conversation

@directhex
Copy link
Contributor

Update branch used for maui previews and get it building

@ghost
Copy link

ghost commented Nov 18, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@directhex directhex marked this pull request as ready for review November 22, 2021 12:27
@akoeplinger akoeplinger requested a review from mmitche November 22, 2021 17:22
Copy link
Member

@mmitche mmitche left a comment

Choose a reason for hiding this comment

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

Aside from the StabilizePackageVersions issue, I think this is correct.

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

You also want to remove all instances of ServicingVersion and GeneratePackageOnBuild

<ServicingVersion>1</ServicingVersion>
those should be added until you want to service that specific package.

@safern
Copy link
Member

safern commented Nov 22, 2021

GeneratePackageOnBuild

Well the transport packages should keep that if you need to service those.

@directhex
Copy link
Contributor Author

directhex commented Nov 23, 2021

Failing because _PackageVersion is empty but I don't see why - it should be populated by $(PackageVersion) in https://github.com/directhex/runtime/blob/brand-maui-branch/src/libraries/workloads-testing.targets#L5 as far as I can see, and PackageVersion is not empty as I read the binlogs

@steveisok
Copy link
Member

steveisok commented Nov 23, 2021

Failing because _PackageVersion is empty but I don't see why - it should be populated by $(PackageVersion) in https://github.com/directhex/runtime/blob/brand-maui-branch/src/libraries/workloads-testing.targets#L5 as far as I can see, and PackageVersion is not empty as I read the binlogs

@radical can you provide any insight?

@directhex
Copy link
Contributor Author

Seems more likely related to changes by @mmitche to the version handling in that file

@safern
Copy link
Member

safern commented Nov 23, 2021

I see what's going on and I hit this on main when porting these changes to main. PackageVersion is not set at this point when this property group is evaluated, so that it is why its value is empty. In order to fix this, we need to move the PropertyGroup that sets _PackageVersion inside the target, so that the property value is assigned when the target is executed and therefore PackageVersion will already be evaluated and assigned. See: https://github.com/dotnet/runtime/blob/main/eng/testing/workloads-testing.targets#L96-L97

@directhex
Copy link
Contributor Author

@safern @mmitche any objections? We're green

@steveisok steveisok merged commit 30c3091 into dotnet:release/6.0-maui Nov 24, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants