Fix disc full enumeration by checking that $(MonoAndroidAssetsPrefix) is not empty string #46

moljac · 2022-11-21T10:21:07Z

During AndroidX and GooglePlaySerivces/Firebase/MLKit builds experienced following warning (which was crashing one of Mac boxes with running out of application memory):

MSBUILD : warning MSB5029: The value “\**\.*\**” of the “Exclude” attribute in element <ItemGroup> in file “/Users/moljac/.nuget/packages/xamarin.legacy.sdk/0.2.0-alpha2/Sdk/AutoImport.Android.props (26,61)” is a wildcard that results in enumerating all files on the drive, which was likely not intended. Check that referenced properties are always defined.

This PR fixes this warning by adding check for $(MonoAndroidAssetsPrefix) not being empty string.

jonathanpeppers

Do you have a .binlog of when this problem occurred? My memory is the latest workload wasn't installed, maybe we can error on that as well?

moljac · 2022-11-21T18:18:26Z

Do you have a .binlog of when this problem occurred?

Attached. This time it took long time to crash mac.

Compressed binlog:

restore.binlog.zip

My memory is the latest workload wasn't installed, maybe we can error on that as well?

Anything would help. Otherwise digging what is going on is quite time consuming.

moljac · 2022-11-22T10:14:44Z

MBP 17" dotnet restore single project.

Note: I did not kill it. (It was spinning from 20:48 until 10:45)

 dotnet restore generated/androidx.activity.activity/androidx.activity.activity.csproj /bl
/usr/local/share/dotnet/sdk/6.0.403/MSBuild.dll -nologo -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,/usr/local/share/dotnet/sdk/6.0.403/dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,/usr/local/share/dotnet/sdk/6.0.403/dotnet.dll -maxcpucount -target:Restore -verbosity:m /bl generated/androidx.activity.activity/androidx.activity.activity.csproj
  Determining projects to restore...
zsh: killed     dotnet restore  /bl

msbuild.binlog.zip

global.json

$ cat global.json 
{
    "sdk": 
    {
        "version": "6.0.403",
        "rollForward": "patch"
    },
    "msbuild-sdks": 
    {
        "MSBuild.Sdk.Extras": "3.0.44",
        "Microsoft.Build.Traversal": "3.2.0",
        "Microsoft.Build.NoTargets": "3.6.0",
        "Xamarin.Legacy.Sdk": "0.2.0-alpha2"
    }
}

workloads.json

$ cat workloads.json 
{
  "microsoft.net.sdk.android": "32.0.476/6.0.400",
  "microsoft.net.sdk.ios": "16.0.527/6.0.400",
  "microsoft.net.sdk.maccatalyst": "15.4.471/6.0.400",
  "microsoft.net.sdk.macos": "12.3.471/6.0.400",
  "microsoft.net.sdk.tvos": "16.0.527/6.0.400",
  "microsoft.net.sdk.maui": "6.0.547/6.0.400",
  "microsoft.net.workload.mono.toolchain": "6.0.11/6.0.400",
  "microsoft.net.workload.emscripten": "6.0.11/6.0.400"
}

src/Xamarin.Legacy.Sdk/Sdk/AutoImport.Android.props

jonathanpeppers · 2022-11-22T14:23:31Z

src/Xamarin.Legacy.Sdk/Sdk/AutoImport.Android.props

    <AndroidResource Include="$(MonoAndroidResourcePrefix)\font\*.ttf" />
    <AndroidResource Include="$(MonoAndroidResourcePrefix)\font\*.otf" />
    <AndroidResource Include="$(MonoAndroidResourcePrefix)\font\*.ttc" />
    <AndroidResource Include="$(MonoAndroidResourcePrefix)\raw\*" Exclude="$(MonoAndroidResourcePrefix)\raw\.*" />


Can we make the same checks for $(MonoAndroidResourcePrefix)?

If there are several lines, we can put them in an <ItemGroup> block with a single Condition.

Can we make the same checks for $(MonoAndroidResourcePrefix)?

Sure we can. I could not find other prop in the chat.

If there are several lines, we can put them in an <ItemGroup> block with a single Condition.

Changed. Both MonoAndroidResourcePrefix and MonoAndroidAssetPrefix checks are now in separate ItemGroup

Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>

…ecks in ItemGroup

jonathanpeppers · 2022-11-28T16:50:21Z

src/Xamarin.Legacy.Sdk/Sdk/AutoImport.Android.props

+  <ItemGroup Condition=" ('$(TargetPlatformIdentifier)' == 'android' or $(TargetFramework.StartsWith ('MonoAndroid', StringComparison.OrdinalIgnoreCase))) and '$(EnableDefaultXamarinLegacySdkItems)' == 'true' 
+                        and ('$(MonoAndroidAssetsPrefix)' != '' and '$(MonoAndroidResourcePrefix)' != '') ">


Can we make an <ItemGroup> block for $(MonoAndroidAssetsPrefix) and one for $(MonoAndroidResourcePrefix)?

There is probably a case where one could be blank and not the other.

jonathanpeppers

Actually what if we just set $(MonoAndroidAssetsPrefix)?

We would need to add a block here:

https://github.com/xamarin/Xamarin.Legacy.Sdk/blob/main/src/Xamarin.Legacy.Sdk/Sdk/Sdk.targets

Something like:

<PropertyGroup Condition=" '$(TargetPlatformIdentifier)' == 'android' or $(TargetFramework.StartsWith ('MonoAndroid', StringComparison.OrdinalIgnoreCase)) ">
  <MonoAndroidAssetsPrefix Condition=" '$(MonoAndroidAssetsPrefix)' == '' ">Assets</MonoAndroidAssetsPrefix>
  <MonoAndroidResourcePrefix Condition=" '$(MonoAndroidResourcePrefix)' == '' ">Resources</MonoAndroidResourcePrefix>
</PropertyGroup>

Context: dotnet/android#7837 Context: #46 I ended up making this change in xamarin-android to avoid a potential issue in all .NET projects, so bringing it here as well. In cases where the `android` workload is not installed, Xamarin.Legacy.Sdk can accidentally wildcard your entire disk: MSBUILD : warning MSB5029: The value “\**\.*\**” of the “Exclude” attribute in element <ItemGroup> in file “/Users/moljac/.nuget/packages/xamarin.legacy.sdk/0.2.0-alpha2/Sdk/AutoImport.Android.props (26,61)” is a wildcard that results in enumerating all files on the drive, which was likely not intended. Check that referenced properties are always defined. Add checks that `$(MonoAndroidAssetsPrefix)` and `$(MonoAndroidResourcePrefix)` are not blank.

jonathanpeppers · 2023-03-04T04:30:40Z

Closing in favor of: #49

I am just doing exactly what we did in xamarin-android. The changes here are just slightly different.

@moljac feel free to review the new PR, thanks!

Context: dotnet/android#7837 Context: #46 I ended up making this change in xamarin-android to avoid a potential issue in all .NET projects, so bringing it here as well. In cases where the `android` workload is not installed, Xamarin.Legacy.Sdk can accidentally wildcard your entire disk: MSBUILD : warning MSB5029: The value “\**\.*\**” of the “Exclude” attribute in element <ItemGroup> in file “/Users/moljac/.nuget/packages/xamarin.legacy.sdk/0.2.0-alpha2/Sdk/AutoImport.Android.props (26,61)” is a wildcard that results in enumerating all files on the drive, which was likely not intended. Check that referenced properties are always defined. Add checks that `$(MonoAndroidAssetsPrefix)` and `$(MonoAndroidResourcePrefix)` are not blank.

moljac added 2 commits November 21, 2022 11:16

added check for $(MonoAndroidAssetsPrefix) not being empty string

32f97da

version bump

adcd5e3

jonathanpeppers reviewed Nov 21, 2022

View reviewed changes

jonathanpeppers reviewed Nov 22, 2022

View reviewed changes

moljac and others added 2 commits November 24, 2022 11:56

Update src/Xamarin.Legacy.Sdk/Sdk/AutoImport.Android.props

9c8e98e

Co-authored-by: Jonathan Peppers <jonathan.peppers@gmail.com>

MonoAndroidAssetsPrefix and MonoAndroidResourcePrefix emtpy string ch…

479d967

…ecks in ItemGroup

jonathanpeppers reviewed Nov 28, 2022

View reviewed changes

jonathanpeppers suggested changes Nov 28, 2022

View reviewed changes

jonathanpeppers mentioned this pull request Mar 4, 2023

[android] guard against accidental \**\* wildcards #49

Merged

jonathanpeppers closed this Mar 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix disc full enumeration by checking that $(MonoAndroidAssetsPrefix) is not empty string #46

Fix disc full enumeration by checking that $(MonoAndroidAssetsPrefix) is not empty string #46

Uh oh!

moljac commented Nov 21, 2022

Uh oh!

jonathanpeppers left a comment

Uh oh!

moljac commented Nov 21, 2022

Uh oh!

moljac commented Nov 22, 2022 •

edited

Loading

Uh oh!

Uh oh!

jonathanpeppers Nov 22, 2022

Uh oh!

moljac Nov 24, 2022

Uh oh!

jonathanpeppers Nov 28, 2022

Uh oh!

jonathanpeppers left a comment

Uh oh!

jonathanpeppers commented Mar 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		<ItemGroup Condition=" ('$(TargetPlatformIdentifier)' == 'android' or $(TargetFramework.StartsWith ('MonoAndroid', StringComparison.OrdinalIgnoreCase))) and '$(EnableDefaultXamarinLegacySdkItems)' == 'true'
		and ('$(MonoAndroidAssetsPrefix)' != '' and '$(MonoAndroidResourcePrefix)' != '') ">

Uh oh!

Fix disc full enumeration by checking that $(MonoAndroidAssetsPrefix) is not empty string #46

Fix disc full enumeration by checking that $(MonoAndroidAssetsPrefix) is not empty string #46

Uh oh!

Conversation

moljac commented Nov 21, 2022

Uh oh!

jonathanpeppers left a comment

Choose a reason for hiding this comment

Uh oh!

moljac commented Nov 21, 2022

Uh oh!

moljac commented Nov 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jonathanpeppers Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

moljac Nov 24, 2022

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers left a comment

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers commented Mar 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

moljac commented Nov 22, 2022 •

edited

Loading