Exclude platform-specific items using metadata #6681
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently the platform-specific files are included for every platform in the MSBuild evaluation, and then at build the _MauiRemovePlatformCompileItems target removes _all_ these items and then adds back the ones for the current platform. This works for an actual build but screws up VS, as the Project System doesn't properly handle a `Compile` item that is present in evaluation but removed during a build. The end result is that the platform-specific files are not actually treated as platform specific, leading to a number of user-visible issues: 1. Platform-specific files incorrectly show all target frameworks in the Nav Bar. 2. Spurious errors in the Error List. 3. Squiggles in the editor, unless the user changes the Nav Bar to the "correct" target framework. 4. Possibly they will not see actual errors in their code (e.g. if you use a Windows-only API in an Android-specific file, but the Nav Bar is set to Windows, you won't get an squiggle about that). 5. The spurious errors block Hot Reload as the Language Service thinks the project is very broken and will not attempt to apply changes. To work around this problem the Project System will now check items for special metadata indicating that they should be ignored. If an item is marked as a "LimitedConfigurationFile" but _not_ marked as "IncludeInThisConfiguration" the Project System will pretend it does not exist in the current build configuration. And as the file no longer exists in the evaluation data, it no longer matters that the Project System does not properly handle the removal in the target. Here we mark all items under the Platforms folder with "LimitedConfigurationFile", and then mark the items corresponding to the current platform with "IncludeInThisConfiguration". The _MauiRemovePlatformCompileItems is updated to remove items based on the metadata rather than the folder structure.
Rather than two properties, we can use one. First all platform-specific files are tagged with `ExcludeFromCurrentConfiguration=true`. Then we update the platform-specific files for the current platform with `ExcludeFromCurrentConfiguration=false`, and the _MauiRemovePlatformCompileItems target can just check the one property.
|
@tmeschter - could this also solve #6642 ? Or does this seem different? |
Eilon
added
the
area-tooling
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
tmeschter
added a commit
to tmeschter/roslyn-project-system
that referenced
this pull request
Apr 29, 2022
Related to [dotnet/maui dotnet#6681](dotnet/maui#6681). Related to AB#1525249. Currently MAUI includes platform-specific files are for every platform in the MSBuild evaluation, and then at build a target removes _all_ these items and then adds back the ones for the current platform. This works for an actual build but screws up VS, as the Project System doesn't properly handle a `Compile` item that is present in evaluation but removed during a build. The end result is that the platform-specific files are not actually treated as platform-specific, leading to a number of user-visible issues: 1. Platform-specific files incorrectly show all target frameworks in the Nav Bar 2. Spurious errors in the Error List 3. Squiggles in the editor, unless the user changes the Nav Bar to the "correct" target framework 4. Possibly they will not see actual errors in their code (e.g. if you use a Windows-only API in an Android-specific file, but the Nav Bar is set to Windows, you won't get an squiggle about that) 5. The spurious errors block Hot Reload as the Language Service thinks the project is very broken and will not attempt to apply changes With this change the Project System will now check items for special metadata indicating that they should be ignored. If an item is marked with `ExcludeFromCurrentConfiguration=true` the Project System will pretend it does not exist in the evaluation data for the current build configuration. And as the file no longer exists in the evaluation data, it no longer matters that the Project System does not properly handle the removal in the target. See [dotnet/maui dotnet#6681](dotnet/maui#6681) for the related MAUI SDK changes that add the metadata to the appropriate items.
@Eilon I haven't tried it out, but I think it will fix that bug as a side effect. Previously |
jonathanpeppers
approved these changes
Apr 29, 2022
There was a problem hiding this comment.
Changes look OK.
@mattleibow @Redth do we need to manually test what happens if you use this inside VS with & without Tom's fixes in the Project System?
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
mattleibow
approved these changes
May 2, 2022
|
Anything further I can do here? |
|
I tested this in an old IDE:
And it all appears to be working as it previously was (still has all the TFMs in the dropdown):
|
|
@ThatGuyMike7 can you please file a new issue with references to the previous similar issues/PRs? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Currently the platform-specific files are included for every platform in the MSBuild evaluation, and then at build the
_MauiRemovePlatformCompileItemstarget removes all these items and then adds back the ones for the current platform.This works for an actual build but screws up VS, as the Project System doesn't properly handle a
Compileitem that is present in evaluation but removed during a build. The end result is that the platform-specific files are not actually treated as platform-specific, leading to a number of user-visible issues:To work around this problem the Project System will now check items for special metadata indicating that they should be ignored. If an item is marked with
ExcludeFromCurrentConfiguration=truethe Project System will pretend it does not exist in the evaluation data for the current build configuration. And as the file no longer exists in the evaluation data, it no longer matters that the Project System does not properly handle the removal in the target.Here we mark all items under the Platforms folder with
ExcludeFromCurrentConfiguration=true, and then mark the items corresponding to the current platform withExcludeFromCurrentConfiguration=false. The_MauiRemovePlatformCompileItemstarget is also updated to remove items based on the metadata rather than the folder structure.Issues Fixed
Note that these issues also require a corresponding change in the dotnet/project-system repo.
Fixes #4726.
Fixes #5294.