Skip to content

Conversation

@joeloff
Copy link
Member

@joeloff joeloff commented Aug 22, 2022

This update contains multiple fixes for issues that were identified during the last servicing release.

  • ItemDefinitionGroup is evaluated globally while the FileVersion property is only set after the GetAssemblyVersion target runs. This causes the VS components to assume the default workload version, which doesn't play well with upgrades in VS. The assembly file version value is preferred because it can change between builds, allow consecutive insertions into VS that can be upgraded.
  • The updated task now provides metadata that will allow us to split the VSDROP generation into two separate archives. One which only contains workload packs (needed to share DROPs across VS versions to support multi-targeting) and another that only contains the manifest installers and workload components.
  • The ZIP archive names have been updated to disambiguate its contents and support automatic drop creation during staging.

@ghost
Copy link

ghost commented Aug 22, 2022

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

ItemDefinitionGroup is evaluated globally while the FileVersion property is only set after the GetAssemblyVersion target runs. This causes the VS components to assume the default workload version, which doesn't play well with upgrades in VS. The assembly file version value is preferred because it can change between builds, allow consecutive insertions into VS that can be upgraded.

Author: joeloff
Assignees: joeloff
Labels:

area-Infrastructure-libraries

Milestone: -

@joeloff joeloff requested a review from hoyosjs August 22, 2022 04:11
@joeloff
Copy link
Member Author

joeloff commented Aug 22, 2022

We don't need this until the next servicing release as we can fix this up manually when we publish to VS.

@hoyosjs hoyosjs added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 22, 2022
@hoyosjs
Copy link
Member

hoyosjs commented Aug 22, 2022

@carlossanlop Let's hold this one for 6.0.10

@lewing lewing added this to the 6.0.x milestone Aug 22, 2022
@lewing lewing changed the title Fix VS component versions [release/6.0] Fix VS component versions Aug 22, 2022
@ghost
Copy link

ghost commented Aug 22, 2022

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

ItemDefinitionGroup is evaluated globally while the FileVersion property is only set after the GetAssemblyVersion target runs. This causes the VS components to assume the default workload version, which doesn't play well with upgrades in VS. The assembly file version value is preferred because it can change between builds, allow consecutive insertions into VS that can be upgraded.

Author: joeloff
Assignees: joeloff
Labels:

NO-MERGE, area-Infrastructure-libraries, area-Infrastructure-mono

Milestone: 6.0.x

@lewing
Copy link
Member

lewing commented Aug 22, 2022

Is there any reason not to do the 7.0 version of this now?

@joeloff
Copy link
Member Author

joeloff commented Aug 22, 2022

Is there any reason not to do the 7.0 version of this now?

None that I can think of

@joeloff
Copy link
Member Author

joeloff commented Aug 31, 2022

I'm going to continue pushing more of the 6.0 changes here so we can get everything ready in one go

@carlossanlop
Copy link
Contributor

I'm going to continue pushing more of the 6.0 changes here so we can get everything ready in one go

Hope you don't mind if I convert it to draft.

@carlossanlop carlossanlop marked this pull request as draft August 31, 2022 19:07
@joeloff
Copy link
Member Author

joeloff commented Sep 7, 2022

This PR is blocked on dotnet/arcade#10582 We likely need to update it to include the Arcade changes once we have a validation build.

Below is an example of what the new archives will look like

image

@joeloff
Copy link
Member Author

joeloff commented Sep 8, 2022

@hoyosjs can you take another look at the PR. The darc update dependencies touched a bunch of Arcade related files.

@hoyosjs
Copy link
Member

hoyosjs commented Sep 8, 2022

The changes LGTM.

@lewing
Copy link
Member

lewing commented Sep 8, 2022

ci is blocked on #75294

@hoyosjs
Copy link
Member

hoyosjs commented Sep 8, 2022

And it broke :( This will likely need #68847

@joeloff joeloff marked this pull request as ready for review September 9, 2022 04:25
@joeloff joeloff removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Sep 9, 2022
@lewing lewing requested a review from carlossanlop September 9, 2022 18:12
@lewing
Copy link
Member

lewing commented Sep 9, 2022

tell mode packaging fix

@lewing lewing merged commit f2112e3 into dotnet:release/6.0 Sep 9, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants