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

[Generation] AdditionalFiles should not be removed if not added by uno #8176

Open
jeromelaban opened this issue Feb 23, 2022 Discussed in #8175 · 3 comments · May be fixed by #8561
Open

[Generation] AdditionalFiles should not be removed if not added by uno #8176

jeromelaban opened this issue Feb 23, 2022 Discussed in #8175 · 3 comments · May be fixed by #8561
Assignees
Labels
area/code-generation Categorizes an issue or PR as relevant to code generation difficulty/challenging 🤯 Categorizes an issue for which the difficulty level is reachable with internals understanding kind/bug Something isn't working

Comments

@jeromelaban
Copy link
Member

Discussed in #8175

Originally posted by HavenDV February 23, 2022
I noticed that the Uno package started blocking the use of third-party generators. In my case, I was generating View code behind constructor code in my test project.
Is this a bug or is it a special behavior to limit the possible problems of using other generators in most cases? In second case, just need to create a library that will call these generators and use it already in Uno

I assume it is caused by this code

<_AdditionalFilesCleanup Include="@(AdditionalFiles)" />
<AdditionalFiles Remove="@(_AdditionalFilesCleanup)" />

P.S. I did not find any changes in the code that would explicitly prohibit the use of other generators. But I still can't achieve using another generator in a project with Uno.UI.WebAssembly. This may be related to dotnet updates to 6.0.2 or VS to 17.2 preview

Repro:
ratbuddyssey-master.zip

  1. Build Ratbuddyssey.Apps.Uno.WebAssembly. The generator will not show the generated file (it should appear if at least one AdditionalFile exists at the time of generation)
image 2. This is not a bug related to the generation order. If you remove Visible="False" then you can see that AdditionalFile exists before the start of build. But it seems then some MSBuild code removes AdditionalFile at one of the stages of preparation. 3. ViewBase code is generated in another library (Ratbuddyssey.Apps.Uno.Core.Standard) and available for Uno generator
@jeromelaban jeromelaban added kind/bug Something isn't working area/code-generation Categorizes an issue or PR as relevant to code generation difficulty/challenging 🤯 Categorizes an issue for which the difficulty level is reachable with internals understanding labels Feb 23, 2022
@francoistanguay francoistanguay removed their assignment Apr 6, 2022
@HavenDV
Copy link
Member

HavenDV commented Apr 12, 2022

<_AdditionalFilesCleanup Include="@(AdditionalFiles)" />
<AdditionalFiles Remove="@(_AdditionalFilesCleanup)" />
<AdditionalFiles Condition="'@(_AdditionalFilesCleanup)'!=''" Include="@(_AdditionalFilesCleanup->'$([System.IO.Path]::GetFullPath('%(_AdditionalFilesCleanup.Identity)'))')" />

The problem is because this code clears the metadata for AdditionalFiles. The fix is either by removing this code (I need to know why the full path is being used) or by moving the metadata on cleanup

@HavenDV
Copy link
Member

HavenDV commented Apr 12, 2022

I guess this can be solved with something like

<AdditionalFiles>
        <Identity>$([System.IO.Path]::GetFullPath('%(AdditionalFiles.Identity)'))</Identity>
</TestItemGroup>

But I need to check if this will update the FullPath and other metadata.

@HavenDV
Copy link
Member

HavenDV commented Apr 13, 2022

Everything turned out to be not so simple. Even if these three lines are removed, msbuild cannot merge the metadata for AdditionalFiles that were specified with a relative path and the metadata for AdditionalFiles with an absolute path, even if the end result is the same. Only the metadata for the AdditionalFiles that were added last will remain.
Therefore, in order to preserve the metadata for existing AdditionalFiles, we need to check for existence and separate behavior for each case.

By testing on a real project with _InjectAdditionalFiles override so far I've come to this solution. I need to look at it with fresh eyes to make sure that everything is normal (this works correctly)

<Project>

  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk.Web" />
  
  <!-- Project code -->

  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk.Web" />
  
  <Target Name="_InjectAdditionalFiles" BeforeTargets="GenerateMSBuildEditorConfigFileShouldRun;GenerateMSBuildEditorConfigFileCore" DependsOnTargets="_FillSourceGeneratorItemGroups">

    <ItemGroup Label="Include new additional files">
      <_ExistingAdditionalFiles Include="%(AdditionalFiles.FullPath)" />
      <AdditionalFiles Include="%(Page.FullPath)" SourceItemGroup="Page" Exclude="@(_ExistingAdditionalFiles)" />
      <AdditionalFiles Include="%(ApplicationDefinition.FullPath)" SourceItemGroup="ApplicationDefinition" Exclude="@(_ExistingAdditionalFiles)" />
      <AdditionalFiles Include="%(PRIResource.FullPath)" SourceItemGroup="PRIResource" Exclude="@(_ExistingAdditionalFiles)" />
      <AdditionalFiles Include="%(TSBindingAssemblySource.FullPath)" SourceItemGroup="TSBindingAssemblySource" Exclude="@(_ExistingAdditionalFiles)" />
    </ItemGroup>
    
    <PropertyGroup>
      <_Pages>@(Page)</_Pages>
      <_ApplicationDefinitions>@(ApplicationDefinition)</_ApplicationDefinitions>
      <_PRIResources>@(PRIResource)</_PRIResources>
      <_TSBindingAssemblySources>@(TSBindingAssemblySource)</_TSBindingAssemblySources>
    </PropertyGroup>
    
    <ItemGroup Label="Add SourceItemGroup metadata to existing additional files">
      <AdditionalFiles SourceItemGroup="Page" Condition="$([System.String]::new('$(_Pages)').Contains('%(AdditionalFiles.FullPath)'))" />
      <AdditionalFiles SourceItemGroup="ApplicationDefinition" Condition="$([System.String]::new('$(_ApplicationDefinitions)').Contains('%(AdditionalFiles.FullPath)'))" />
      <AdditionalFiles SourceItemGroup="PRIResource" Condition="$([System.String]::new('$(_PRIResources)').Contains('%(AdditionalFiles.FullPath)'))" />
      <AdditionalFiles SourceItemGroup="TSBindingAssemblySource" Condition="$([System.String]::new('$(_TSBindingAssemblySources)').Contains('%(AdditionalFiles.FullPath)'))" />
    </ItemGroup>

  </Target>

</Project>

@HavenDV HavenDV linked a pull request Apr 17, 2022 that will close this issue
6 tasks
@Youssef1313 Youssef1313 self-assigned this Apr 13, 2023
@MartinZikmund MartinZikmund changed the title [Generation] AdditionalFiles should not be removed if not added by uno [Generation] AdditionalFiles should not be removed if not added by uno Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code-generation Categorizes an issue or PR as relevant to code generation difficulty/challenging 🤯 Categorizes an issue for which the difficulty level is reachable with internals understanding kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants