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.

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

Transitive dependency issues in single-component UWP head #169

Open
4 tasks
Arlodotexe opened this issue Aug 9, 2023 · 9 comments
Open
4 tasks

Transitive dependency issues in single-component UWP head #169

Arlodotexe opened this issue Aug 9, 2023 · 9 comments
Labels
bug Something isn't working dev loop ➰

Comments

@Arlodotexe
Copy link
Member

Arlodotexe commented Aug 9, 2023

Problem

Single-component solutions are facing a transitive dependency issue on UWP heads. It can be fixed by making the transitive dependency an explicit dependency, but not all transitive dependencies are problematic. This ticket serves to investigate why this is happening, and what we can do about it.

Findings

Behaviors.Uwp.csproj project requires an explicit reference to the Animations project to build successfully, even though Animations is a transitive dependency. This behavior is not observed in the SettingsControls.Uwp.csproj project.

  1. Dependency Chains:
    • Behaviors: Uwp -> Behaviors -> Animations -> Extensions. Had to manually add a direct reference from Uwp to Animations (but not Extensions for some reason) to fix the build issue.
    • SettingsControls: Uwp -> SettingsControl -> Triggers -> Helpers -> Extensions. No issue building.

image

  1. Task Analysis:

    • Comprehensive comparison of all tasks in both binlogs was performed.
    • The sequence of task execution was analyzed for both projects.
    • Minor discrepancies such as the presence of the Touch task (related to file timestamp updates) in SettingsControls.Uwp.csproj and not in Behaviors.Uwp.csproj were noted, but aren't relevant to the issue.
  2. MSBuild Task Parameters:

    • Parameters for MSBuild tasks were extracted and compared between the two components.
    • No significant discrepancies were found in the parameters.
  3. Hypotheses and Proposed Experiments:

    • Depth of Dependency Chain: The additional intermediate project layer (Helpers) in the SettingsControls chain might influence transitive dependency resolution.
    • Multiple transitive references: The Behaviors source project has two dependencies that should be available transiently to the Uwp head, while the SettingsControls source project only has one. It's possible that the reference from Behaviors.Uwp to Animations was needed but the reference to Extensions wasn't because it can only resolve one transient reference.
    • Explicit vs. Transitive References: Making all dependencies explicit in the Behaviors component resolves the issue. As one possible workaround, we could build a way to detect and add these to the UWP head automatically. Not recommended.

Recommendations

We'll need to finish investigation to find out why the transitive dependencies are working fine in SettingsControls but not Behaviors. If we adjust the Behaviors project reference structure to match certain characteristics of the SettingsControls project graph, it may yield the information needed to file a proper bug report and create a workaround.

Possible next steps:

Preview Give feedback
@Arlodotexe Arlodotexe added bug Something isn't working dev loop ➰ labels Aug 9, 2023
@michael-hawker
Copy link
Member

Wondering if it's because Behaviors directly references Extensions (which is also needed by the app) vs. the transitive inclusion of it elsewhere? That could be something to investigate with the all the issues we've had with duplicate ProjectReferences...

I also saw that if I only added Animations as a reference in Behaviors, instead of also re-adding Extensions and the HeaderedControls in the VS Project Reference dialog that I got this message:

3>"H:\code\CommunityToolkitWindows\components\Behaviors\heads\Uwp\Behaviors.Uwp.csproj" (Build;BuiltProjectOutputGroup;BuiltProjectOutputGroupDependencies;DebugSymbolsProjectOutputGroup;DebugSymbolsProjectOutputGroupDependencies;DocumentationProjectOutputGroup;DocumentationProjectOutputGroupDependencies;SatelliteDllsProjectOutputGroup;SatelliteDllsProjectOutputGroupDependencies;SGenFilesOutputGroup;SGenFilesOutputGroupDependencies target) (1) ->
3>(_WireUpCoreRuntime target) -> 
3>  C:\Users\mhawker\.nuget\packages\microsoft.net.uwpcoreruntimesdk\2.2.12\tools\CoreRuntime\Microsoft.Net.CoreRuntime.targets(238,5): error : CopyWin32Resources failed with exit code 701

Which I hadn't seen before, so not sure if that's also something related or not to maybe the underlying issue of what's happening here? It definitely seems related to UWP and the old-project style vs. new.

Maybe we should investigate upgrading MSBuildExtras?

Or at least if we can repro it outside as a minimal repro we can compare with and without it to see if that's a starting point?

@michael-hawker
Copy link
Member

michael-hawker commented Aug 9, 2023

The test UWP head has the same issue as well... however, adding the references manually doesn't seem to be helping...

5>"H:\code\CommunityToolkitWindows\components\Behaviors\heads\Tests.Uwp\Behaviors.Tests.Uwp.csproj" (Build;BuiltProjectOutputGroup;BuiltProjectOutputGroupDependencies;DebugSymbolsProjectOutputGroup;DebugSymbolsProjectOutputGroupDependencies;DocumentationProjectOutputGroup;DocumentationProjectOutputGroupDependencies;SatelliteDllsProjectOutputGroup;SatelliteDllsProjectOutputGroupDependencies;SGenFilesOutputGroup;SGenFilesOutputGroupDependencies target) (1) ->
5>(_WireUpCoreRuntime target) -> 
5>  C:\Users\mhawker\.nuget\packages\microsoft.net.uwpcoreruntimesdk\2.2.12\tools\CoreRuntime\Microsoft.Net.CoreRuntime.targets(238,5): error : CopyWin32Resources failed with exit code 500

@niels9001
Copy link
Collaborator

Hitting this exact problem with SettingsControls/UWP as well, caused by the Triggers reference

@Arlodotexe
Copy link
Member Author

Arlodotexe commented Jun 6, 2024

Additional information found during testing of the latest 8.1-rc.

It's worth noting that when consuming the nuget packages, installing the Media package doesn't automatically reference the transitive dependency on Animations, and this behavior is unexpected.

In order to reproduce any sample code from the Media component in our gallery, an end user application needs to install both Media and Behaviors, with Behaviors providing a transient dependency on Animations, but not Media. The transient dependency in Media on Animations is not picked up when Media is used via the NuGet package.

This is doubly unexpected because in the OP, we found that installing only Behaviors doesn't provide a transient dependency to Animations, contradicting what happened here (installing Behaviors did add a transient dependency to Animations). Possibly the fault of the Animations component and not the component that references it?

@Arlodotexe
Copy link
Member Author

Worth noting our use of InternalsVisibleTo are using or being used by same set of components that keep appearing in these issues-- Behaviors, Animations, Media and Extensions.

If our use of InternalsVisibleTo is indeed related to our AoT issues on Wasdk as well as what we've seen here regarding transient dependencies, it might be highlighting a deeper issue with how we've set up these libraries.

We might want to go through and see if we can remove the need for these InternalsVisibleTo, it looks like at least a few are just to avoid helper duplication. If it fixes any of the issues we've listed throughout this ticket, we've narrowed the scope of our search down considerably.

@Arlodotexe Arlodotexe moved this from 🛑 Blocked to 🏗 In progress in Toolkit 8.x Jun 24, 2024
@Arlodotexe
Copy link
Member Author

Arlodotexe commented Jun 27, 2024

Considering InternalsVisibleTo being involved, possibly related to #404

@Arlodotexe
Copy link
Member Author

This seems to be a general issue with non-sdk style projects, which is what our UWP head uses. The issue was reported by a member of the Windows App Community here and a full, minimal repro (without the toolkit) was uploaded to GitHub here.

@Arlodotexe
Copy link
Member Author

Until there's a clear path forward for resolving this for non-sdk style projects, I'm moving this back to "blocked".

@Arlodotexe Arlodotexe moved this from 🔖 Ready to 🛑 Blocked in Toolkit 8.x Jul 11, 2024
@michael-hawker
Copy link
Member

I feel like something similar was preventing me from building the Sample Gallery in release mode for WinUI 3 as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dev loop ➰
Projects
Status: 🛑 Blocked
Development

No branches or pull requests

3 participants