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

'dotnet new android' produces invalid images #3325

Closed
jonathanpeppers opened this issue Jun 22, 2021 · 3 comments · Fixed by #3420
Closed

'dotnet new android' produces invalid images #3325

jonathanpeppers opened this issue Jun 22, 2021 · 3 comments · Fixed by #3420
Assignees
Labels
Priority:1 Work that is critical for the release, but we could probably ship without triaged The issue was evaluated by the triage team, placed on correct area, next action defined.
Milestone

Comments

@jonathanpeppers
Copy link
Member

jonathanpeppers commented Jun 22, 2021

Our templates: microsoft.android.templates.30.0.100-preview.6.56.nupkg.zip

If you dotnet new android you get a broken image that is coincidentally ~8kb:

image

This broken image causes dotnet build to fail when calling the underlying Android tooling:

Resources\mipmap-xxxhdpi\ic_launcher_round.png error APT2000: failed reading from input: PNG chunk type 49444154 is too large: chunk length is 10836 but chunk starts at byte 45/8187.

If I unzip this file myself (with a tool like 7-zip), it is actually larger than ~8kb:

image

I suspect if a template has a file larger than ~8kb you'll hit this issue.

This started happening in Maestro bumps here:

So likely the issue started somewhere bumping .NET 6.0.100-preview.6.21313.2 to 6.0.100-preview.6.21321.13.

@vlada-shubina
Copy link
Member

vlada-shubina commented Jun 23, 2021

@jonathanpeppers I tried to reproduce this bug, but was not able two - the files in template and in generated output are identical.
We were not touching core template engine (the assembly that generates the files) since preview 5. Anything else that can affect this?

In general I would recommend to use "copy-only" mode for files without replacements (see the link). This will avoid any file modification, the file will be copied from template content, and also should be faster as template engine will not scan the file content for variables.


Update: I was able to reproduce the issue, however only using dotnet/installer to install dotnet new. We should look on what has been changed in dotnet-sdk or dotnet/installer in these timeframe which started causing the issue.

@jonathanpeppers
Copy link
Member Author

I can confirm adding this to our template.json works around the issue:

  "sources": [
      {
        "source": "./",
        "target": "./",
        "copyOnly": "**/Resources/**/*.png"
      }
  ],

I'll set this up on any iOS/Android templates for .png files, thanks. It should improve performance as well.

jonathanpeppers added a commit to xamarin/xamarin-android that referenced this issue Jun 23, 2021
Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In the current bump, `dotnet new android` + `dotnet build` fails with:

    Resources\mipmap-xxxhdpi\ic_launcher_round.png error APT2000: failed reading from input: PNG chunk type 49444154 is too large: chunk length is 10836 but chunk starts at byte 45/8187.

The .NET templating system has a 'copyOnly' mode for files that do not
need any text replaced. This is a performance feature, but it also
happens to workaround this issue.

We should be doing this on `.png` files *anyway*, as we don't need
potential "text" to be replaced. I will probably need to make similar
changes in xamarin/xamarin-macios and dotnet/maui.
jonathanpeppers added a commit to xamarin/xamarin-android that referenced this issue Jun 23, 2021
Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In the current bump, `dotnet new android` + `dotnet build` fails with:

    Resources\mipmap-xxxhdpi\ic_launcher_round.png error APT2000: failed reading from input: PNG chunk type 49444154 is too large: chunk length is 10836 but chunk starts at byte 45/8187.

The .NET templating system has a 'copyOnly' mode for files that do not
need any text replaced. This is a performance feature, but it also
happens to workaround this issue.

We should be doing this on `.png` files *anyway*, as we don't need
potential "text" to be replaced. I will probably need to make similar
changes in xamarin/xamarin-macios and dotnet/maui.
@vlada-shubina vlada-shubina self-assigned this Jun 23, 2021
jonathanpeppers added a commit to xamarin/xamarin-android that referenced this issue Jun 23, 2021
Changes: dotnet/installer@abb57b4...12636f6
Changes: dotnet/linker@21df7db...f90f5c9
Changes: dotnet/runtime@5b8e178...8bb087d

* Update dependencies from https://github.com/dotnet/installer build 20210623.1

Microsoft.Dotnet.Sdk.Internal
 From Version 6.0.100-preview.6.21313.2 to 6.0.100-preview.6.21323.1

Dependency coherency updates

* Microsoft.NET.ILLink.Tasks: from 6.0.100-preview.6.21304.2 to 6.0.100-preview.6.21314.2
* Microsoft.NETCore.App.Ref: from 6.0.0-preview.6.21306.1 to 6.0.0-preview.6.21317.12

* [tests] temporarily skip asserts in BuildBasicBindingLibrary

Context: dotnet/msbuild#6609

It seems that properties are missing from MSBuild logs, ignoring part of this test for now.

* [templates] set .png files to `copyOnly`

Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In the current bump, `dotnet new android` + `dotnet build` fails with:

    Resources\mipmap-xxxhdpi\ic_launcher_round.png error APT2000: failed reading from input: PNG chunk type 49444154 is too large: chunk length is 10836 but chunk starts at byte 45/8187.

The .NET templating system has a `copyOnly` mode for files that do not
need any text replaced. This is a performance feature, but it also
happens to workaround this issue.

We should be doing this on `.png` files *anyway*, as we don't need
potential "text" to be replaced. I will probably need to make similar
changes in xamarin/xamarin-macios and dotnet/maui.

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
jonathanpeppers added a commit to xamarin/xamarin-android that referenced this issue Jun 23, 2021
Changes: dotnet/installer@abb57b4...38e12ca
Changes: dotnet/linker@21df7db...c739a81
Changes: dotnet/runtime@5b8e178...24950a3

Updates:

* Microsoft.Dotnet.Sdk.Internal: from 6.0.100-preview.6.21313.2 to 6.0.100-preview.7.21321.2
* Microsoft.NET.ILLink.Tasks: from 6.0.100-preview.6.21304.2 to 6.0.100-preview.6.21317.4
* Microsoft.NETCore.App.Ref: from 6.0.0-preview.6.21306.1 to 6.0.0-preview.7.21319.2

* Remove workarounds for <RuntimeConfigParserTask/>

Context: dotnet/runtime#53811

dotnet/runtime#53811 is now solved, so we can remove the workaround.

* Update .apkdesc files

* [tests] temporarily skip asserts in BuildBasicBindingLibrary

Context: dotnet/msbuild#6609

It seems that properties are missing from MSBuild logs, ignoring part of this test for now.

* [templates] set .png files to `copyOnly`

Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In the current bump, `dotnet new android` + `dotnet build` fails with:

    Resources\mipmap-xxxhdpi\ic_launcher_round.png error APT2000: failed reading from input: PNG chunk type 49444154 is too large: chunk length is 10836 but chunk starts at byte 45/8187.

The .NET templating system has a `copyOnly` mode for files that do not
need any text replaced. This is a performance feature, but it also
happens to workaround this issue.

We should be doing this on `.png` files *anyway*, as we don't need
potential "text" to be replaced. I will probably need to make similar
changes in xamarin/xamarin-macios and dotnet/maui.

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jun 24, 2021
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jun 24, 2021
@vlada-shubina
Copy link
Member

vlada-shubina commented Jun 24, 2021

The cause of the issue is: dotnet/runtime#53502, dotnet/docs#24649.
It affects only .NET core running on runtime later then .NET 6 preview 6, it does not affect full framework versions.
We will need to rewrite portions of the code in TemplateEngine.Core using stream.Read in a wrong way (about 20 occurrences).
This is tentatively scheduled for preview 7, might slip to rc1.
For those affected, please consider to use copyOnly (see #3325 (comment)) in case the files should be just copied to target folder without any modifications, especially in case of binary files; and see if it solves the problem.

It is strongly recommended to use copyOnly for all binary files in your templates (even without the context of the bug) as template engine is scanning files byte wise for making replacements. In case of the match the byte content will be replaced even in binary files, and this leads to corrupted file after template generation. Using copyOnly in your template for the files that should be copied without modifications also improves performance when instantiating the template.

@vlada-shubina vlada-shubina added this to the July Sprint milestone Jun 24, 2021
@vlada-shubina vlada-shubina added the triaged The issue was evaluated by the triage team, placed on correct area, next action defined. label Jun 24, 2021
jonathanpeppers added a commit to jonathanpeppers/xamarin-macios that referenced this issue Jun 24, 2021
Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In current .NET 6 Preview 6 builds, there is an issue if a template
includes a binary file larger than ~8kb, it seems to get truncated when
`dotnet new` extracts the template.

A workaround is to use the `copyOnly` feature for binary files. Really,
we should be doing this anyway, because otherwise the templating system
considers replacing *text* in these binary files. It improves
performance to do this and would hopefully prevent a future bug of
random bytes getting replaced.
rolfbjarne pushed a commit to xamarin/xamarin-macios that referenced this issue Jun 25, 2021
Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In current .NET 6 Preview 6 builds, there is an issue if a template
includes a binary file larger than ~8kb, it seems to get truncated when
`dotnet new` extracts the template.

A workaround is to use the `copyOnly` feature for binary files. Really,
we should be doing this anyway, because otherwise the templating system
considers replacing *text* in these binary files. It improves
performance to do this and would hopefully prevent a future bug of
random bytes getting replaced.
vs-mobiletools-engineering-service2 pushed a commit to vs-mobiletools-engineering-service2/xamarin-macios that referenced this issue Jun 25, 2021
Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In current .NET 6 Preview 6 builds, there is an issue if a template
includes a binary file larger than ~8kb, it seems to get truncated when
`dotnet new` extracts the template.

A workaround is to use the `copyOnly` feature for binary files. Really,
we should be doing this anyway, because otherwise the templating system
considers replacing *text* in these binary files. It improves
performance to do this and would hopefully prevent a future bug of
random bytes getting replaced.
rolfbjarne pushed a commit to xamarin/xamarin-macios that referenced this issue Jun 25, 2021
Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In current .NET 6 Preview 6 builds, there is an issue if a template
includes a binary file larger than ~8kb, it seems to get truncated when
`dotnet new` extracts the template.

A workaround is to use the `copyOnly` feature for binary files. Really,
we should be doing this anyway, because otherwise the templating system
considers replacing *text* in these binary files. It improves
performance to do this and would hopefully prevent a future bug of
random bytes getting replaced.

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Jun 25, 2021
Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In current .NET 6 Preview 6 builds, there is an issue if a template
includes a binary file larger than ~8kb, it seems to get truncated when
`dotnet new` extracts the template.

A workaround is to use the `copyOnly` feature for binary files. Really,
we should be doing this anyway, because otherwise the templating system
considers replacing *text* in these binary files. It improves
performance to do this and would hopefully prevent a future bug of
random bytes getting replaced.

For .NET MAUI, the following files need `copyOnly`:

* `wwwroot/css` folder has several types of  fonts inside
* `.svg` and `.ttf` files
mattleibow pushed a commit to dotnet/maui that referenced this issue Jun 28, 2021
Context: dotnet/templating#3325
Context: https://github.com/dotnet/templating/wiki/Reference-for-template.json#content-manipulation

In current .NET 6 Preview 6 builds, there is an issue if a template
includes a binary file larger than ~8kb, it seems to get truncated when
`dotnet new` extracts the template.

A workaround is to use the `copyOnly` feature for binary files. Really,
we should be doing this anyway, because otherwise the templating system
considers replacing *text* in these binary files. It improves
performance to do this and would hopefully prevent a future bug of
random bytes getting replaced.

For .NET MAUI, the following files need `copyOnly`:

* `wwwroot/css` folder has several types of  fonts inside
* `.svg` and `.ttf` files
vlada-shubina added a commit to vlada-shubina/templating that referenced this issue Jun 29, 2021
@vlada-shubina vlada-shubina added Priority:0 Work that we can't release without Priority:1 Work that is critical for the release, but we could probably ship without and removed Priority:0 Work that we can't release without labels Jun 29, 2021
vlada-shubina added a commit that referenced this issue Jun 30, 2021
GangWang01 pushed a commit to GangWang01/sdk that referenced this issue Jun 7, 2022
GangWang01 pushed a commit to GangWang01/sdk that referenced this issue Jun 21, 2022
GangWang01 pushed a commit to GangWang01/sdk that referenced this issue Jun 27, 2022
GangWang01 pushed a commit to GangWang01/sdk that referenced this issue Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:1 Work that is critical for the release, but we could probably ship without triaged The issue was evaluated by the triage team, placed on correct area, next action defined.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants