Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove CoreCrossTargetingTargetsPath import when SDK provides them via package mechanism #1062

Closed
nguerrera opened this issue Sep 21, 2016 · 7 comments · Fixed by #6668
Closed

Remove CoreCrossTargetingTargetsPath import when SDK provides them via package mechanism #1062

nguerrera opened this issue Sep 21, 2016 · 7 comments · Fixed by #6668
Labels
triaged
Milestone
VS 17.0

Comments

@nguerrera
Copy link
Contributor

Microsoft.Commmon.CrossTargeting.targets imported $(CoreCrossTargetingTargetsPath) as a temporary bootstrapping mechanism before there was packaging extensibility available for cross-targeting builds. Once the SDK targets are successfully wired in via the packaging mechanism, remove this import.
@nguerrera
Copy link
Contributor Author

@rainersigwald @AndyGerlicher Please assign this to me. Any chance I could get permission to do that myself?

@rainersigwald
Copy link
Member

@nguerrera I think you might have permissions now. Maybe.

@rainersigwald
Copy link
Member

Do we still need this?

@nguerrera
Copy link
Contributor Author

Technically, we could remove this:

https://github.com/microsoft/msbuild/blob/c70d6ded175c0a7da632018b856daff9f9cd392e/src/Tasks/Microsoft.Common.CrossTargeting.targets#L189-L191

But it is a breaking change if anyone besides dotnet/sdk started using it. I think we could also leave it and remove the TODO.

@nguerrera nguerrera removed their assignment Oct 21, 2019
@dsplaisted
Copy link
Member

Looks like this is probably not used, at least in public code: https://github.com/search?q=CoreCrossTargetingTargetsPath+-filename%3AMicrosoft.Common.CrossTargeting.targets&type=code

@rainersigwald
Copy link
Member

We'll, now's the time then.

@Nirmal4G
Copy link
Contributor

I already have it in my patch. I'll separate this into a new PR!

@Nirmal4G Nirmal4G mentioned this issue Jul 13, 2021
Merged
ladipro pushed a commit that referenced this issue Jul 20, 2021
@Nirmal4G
Remove import using 'CoreCrossTargetingTargetsPath' property (#6668)
fd23477 
Fixes #1062

### Context

The import based on this property was introduced as a temporary bootstrapping
mechanism before there was packaging extensibility available for multi-targeting.

The packaging mechanism (aka NuGet) now uses `buildCrossTargeting`/`buildMultiTargeting`,
similar to `build` package folder to hold and import multi-targeting logic via NuGet's Restore.

Thus, we don't need this workaround anymore.


### Changes Made

Removed the temporary import based on `CoreCrossTargetingTargetsPath`


### Testing

Since there are no tests for this property, as it is with all the workarounds… We'll see if we break anyone during the self-hosting period.


### Notes

This patch was already a part of #6161. Since, the change was approved independently, I have separated this into a new PR.
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
triaged
Projects
None yet
Milestone
VS 17.0
Development

Successfully merging a pull request may close this issue.

Remove import using CoreCrossTargetingTargetsPath property
5 participants
@nguerrera @dsplaisted @rainersigwald @Nirmal4G @AR-May