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

[.NET10] removal of generator generated code in Additions/ #1058

Open
wants to merge 143 commits into
base: main
Choose a base branch
from

Conversation

moljac
Copy link
Contributor

@moljac moljac commented Dec 10, 2024

Context: due to the changes in generator.exe output old generator code that was used must be removed or regenerated.

Initial investigation - files:

Highest priority for MAUI (needs deeper investigation, because of transitive dependencies):

./source/androidx.activity/activity/Additions/ActivityResultContracts.cs
./source/androidx.appcompat/appcompat/Additions/ActionMenuView.cs
./source/androidx.appcompat/appcompat/Additions/Additions.cs
./source/androidx.preference/preference/Additions/Additions.cs
./source/androidx.recyclerview/recyclerview-selection/Additions/Additions.cs
./source/androidx.recyclerview/recyclerview-selection/Additions/AndroidX.RecyclerView.Selection.SelectionTracker.cs
./source/androidx.swiperefreshlayout/swiperefreshlayout/Additions/Additions.cs
./source/androidx.transition/transition/Additions/Additions.cs

./source/com.google.android.material/material/Additions/Additions.cs
./source/com.google.android.material/material/Additions/Google.Android.Material.Navigation.NavigationBarItemView.cs
./source/com.google.android.material/material/Additions/Google.Android.Material.Snackbar.Snackbar.cs
./source/com.google.android.material/material/Additions/Google.Android.Material.Tabs.AppCompat.App.AppCompatActivity.cs
./source/com.google.crypto.tink/tink-android/Additions/Xamarin.Google.Crypto.Tink.Shaded.Protobuf.Internal.cs
./source/com.google.crypto.tink/tink-android/Additions/Xamarin.Google.Crypto.Tink.Shaded.Protobuf.LazyStringArrayList.cs
./source/com.google.crypto.tink/tink-android/Additions/Xamarin.Google.Crypto.Tink.Shaded.Protobuf.MapFieldLite.cs
./source/com.google.crypto.tink/tink-android/Additions/Xamarin.Google.Crypto.Tink.Shaded.Protobuf.UnmodifiableLazyStringList.cs


./source/com.google.android.gms/play-services-base/Additions/Additions.cs
./source/com.google.android.gms/play-services-base/Additions/BitmapTeleporter.cs
./source/com.google.android.gms/play-services-base/Additions/GoogleSignInAccount.cs
./source/com.google.android.gms/play-services-base/Additions/GoogleSignInOptions.cs
./source/com.google.android.gms/play-services-base/Additions/Statuses.cs
./source/com.google.android.gms/play-services-maps/Additions/Additions.cs

./source/com.google.protobuf/protobuf-javalite/Additions/Additions.cs
./source/com.google.protobuf/protobuf-lite/Additions/LazyStringArrayList.cs
./source/com.google.protobuf/protobuf-lite/Additions/MapFieldLite.cs

Full list:

./source/androidx.activity/activity/Additions/ActivityResultContracts.cs
./source/androidx.appcompat/appcompat/Additions/ActionMenuView.cs
./source/androidx.appcompat/appcompat/Additions/Additions.cs
./source/androidx.car/car/Additions/Additions.cs
./source/androidx.compose.animation/animation-core/Additions/AndroidX.Compose.Animation.Core.IAnimationSpec.cs
./source/androidx.compose.animation/animation-core/Additions/AndroidX.Compose.Animation.Core.IDecayAnimationSpec.cs
./source/androidx.compose.animation/animation-core/Additions/AndroidX.Compose.Animation.Core.IFiniteAnimationSpec.cs
./source/androidx.compose.animation/animation-core/Additions/AndroidX.Compose.Animation.Core.InfiniteRepeatableSpec.cs
./source/androidx.compose.animation/animation-core/Additions/AndroidX.Compose.Animation.Core.RepeatableSpec.cs
./source/androidx.compose.animation/animation-core/Additions/AndroidX.Compose.Animation.Core.SnapSpec.cs
./source/androidx.compose.runtime/runtime-android/Additions/AndroidX.Compose.Runtime.Snapshots.SnapshotStateMap.cs
./source/androidx.dynamicanimation/dynamicanimation/Additions/Additions.cs
./source/androidx.emoji/emoji/Additions/Additions.cs
./source/androidx.leanback/leanback/Additions/Additions.cs
./source/androidx.leanback/leanback/Additions/ConstantState.cs
./source/androidx.leanback/leanback/Additions/StreamingTextView.cs
./source/androidx.legacy/legacy-support-core-ui/Additions/Additions.cs
./source/androidx.media3/media3-exoplayer-hls/Additions/AndroidX.Media3.ExoPlayer.Hls.Playlist.HlsPlaylist.cs
./source/androidx.media3/media3-ui/Additions/DefaultTimeBar.cs
./source/androidx.preference/preference/Additions/Additions.cs
./source/androidx.recyclerview/recyclerview-selection/Additions/Additions.cs
./source/androidx.recyclerview/recyclerview-selection/Additions/AndroidX.RecyclerView.Selection.SelectionTracker.cs
./source/androidx.swiperefreshlayout/swiperefreshlayout/Additions/Additions.cs
./source/androidx.transition/transition/Additions/Additions.cs
./source/androidx.wear.watchface/watchface-style/Additions/AndroidX.Wear.Watchface.Style.UserStyle.cs
./source/com.google.android.gms/play-services-ads-lite/Additions/Additions.cs
./source/com.google.android.gms/play-services-base/Additions/Additions.cs
./source/com.google.android.gms/play-services-base/Additions/BitmapTeleporter.cs
./source/com.google.android.gms/play-services-base/Additions/GoogleSignInAccount.cs
./source/com.google.android.gms/play-services-base/Additions/GoogleSignInOptions.cs
./source/com.google.android.gms/play-services-base/Additions/Statuses.cs
./source/com.google.android.gms/play-services-fido/Additions/TaskExtensions.cs
./source/com.google.android.gms/play-services-fitness/Additions/IPendingResultExtensions.cs
./source/com.google.android.gms/play-services-fitness/Additions/TaskExtensions.cs
./source/com.google.android.gms/play-services-games/Additions/Android.Gms.Games.GamesClass.cs
./source/com.google.android.gms/play-services-maps/Additions/Additions.cs
./source/com.google.android.gms/play-services-mlkit-barcode-scanning/Additions/Additions.cs
./source/com.google.android.gms/play-services-mlkit-text-recognition-common/Additions/Additions.cs
./source/com.google.android.gms/play-services-nearby/Additions/IPendingResultExtensions.cs
./source/com.google.android.gms/play-services-nearby/Additions/TaskAdditions.cs
./source/com.google.android.gms/play-services-vision/Additions/Additions.cs
./source/com.google.android.gms/play-services-wearable/Additions/TaskExtensions.cs
./source/com.google.android.material/material/Additions/Additions.cs
./source/com.google.android.material/material/Additions/Google.Android.Material.Navigation.NavigationBarItemView.cs
./source/com.google.android.material/material/Additions/Google.Android.Material.Snackbar.Snackbar.cs
./source/com.google.android.material/material/Additions/Google.Android.Material.Tabs.AppCompat.App.AppCompatActivity.cs
./source/com.google.crypto.tink/tink-android/Additions/Xamarin.Google.Crypto.Tink.Shaded.Protobuf.Internal.cs
./source/com.google.crypto.tink/tink-android/Additions/Xamarin.Google.Crypto.Tink.Shaded.Protobuf.LazyStringArrayList.cs
./source/com.google.crypto.tink/tink-android/Additions/Xamarin.Google.Crypto.Tink.Shaded.Protobuf.MapFieldLite.cs
./source/com.google.crypto.tink/tink-android/Additions/Xamarin.Google.Crypto.Tink.Shaded.Protobuf.UnmodifiableLazyStringList.cs
./source/com.google.firebase/firebase-appcheck/Additions/Firebase.AppCheck.FirebaseAppCheck.cs
./source/com.google.firebase/firebase-appcheck/Additions/Firebase.AppCheck.Internal.DefaultFirebaseAppCheck.cs
./source/com.google.firebase/firebase-appindexing/Additions/Additions.cs
./source/com.google.firebase/firebase-firestore/Additions/Additions.cs
./source/com.google.firebase/firebase-storage/Additions/Additions.cs
./source/com.google.mlkit/common/Additions/Additions.cs
./source/com.google.mlkit/common/Additions/Xamarin.Google.MLKit.Common.SDKInternal.OptionalModuleUtils.cs
./source/com.google.mlkit/image-labeling-automl/Additions/AutoMLImageLabelerOptions.cs
./source/com.google.mlkit/object-detection-custom/Additions/Xamarin.Google.MLKit.Vision.Objects.Custom.Internal.CustomObjectsRegistrar.cs
./source/com.google.mlkit/object-detection/Additions/Xamarin.Google.MLKit.Vision.Objects.Defaults.Internal.DefaultObjectsRegistrar.cs
./source/com.google.mlkit/vision-common/Additions/Additions.cs
./source/com.google.protobuf/protobuf-javalite/Additions/Additions.cs
./source/com.google.protobuf/protobuf-lite/Additions/LazyStringArrayList.cs
./source/com.google.protobuf/protobuf-lite/Additions/MapFieldLite.cs
./source/io.grpc/grpc-api/Additions/Xamarin.Grpc.CodecGzip.cs
./source/org.tensorflow/tensorflow-lite-support-api/Additions/Xamarin.TensorFlow.Lite.Support.Common.TensorProcessor.cs
./source/org.tensorflow/tensorflow-lite-support-api/Additions/Xamarin.TensorFlow.Lite.Support.Image.ImageProcessor.cs

Details

Some bugs (generics arity) or missing features in generator were solved by using remove-node with previously copied generator code which was added to Additions/ and manually modified.

@moljac moljac marked this pull request as draft December 10, 2024 15:14
@jpobst
Copy link
Contributor

jpobst commented Dec 10, 2024

Honestly that list isn't as bad as I feared.

I guess for each manual code we have 3 options:

  1. If generator has fixed the bug, remove the addition and the <remove-node> and let generator do its thing.
  2. Remove the addition, which will break API compatibility.
  3. Commit both a NET8_0/NET9_0 manual version and a NET10_0_OR_GREATER manual version.

I suspect we'll have to do option (3) for most of these.

@moljac
Copy link
Contributor Author

moljac commented Dec 11, 2024

Honestly that list isn't as bad as I feared.

I was pretty scared when JonP told me, but after I searched and gathered some info. It does not look bad.

I guess for each manual code we have 3 options:

  1. If generator has fixed the bug, remove the addition and the <remove-node> and let generator do its thing.
  2. Remove the addition, which will break API compatibility.
  3. Commit both a NET8_0/NET9_0 manual version and a NET10_0_OR_GREATER manual version.

I suspect we'll have to do option (3) for most of these.

This is what I intended - early discussion and those points were in my plan too.

Maybe additionally

  1. Fix the generator along the way based on research (only if it is not too complex)

Bugs/problems will be in comments. 1st one coming in today.

@moljac
Copy link
Contributor Author

moljac commented Dec 11, 2024

20241211 Problem

<!--
Problem!!!
./generated/androidx.activity.activity/obj/Debug/net8.0-android/generated/src/AndroidX.Activity.Result.Contract.ActivityResultContracts.cs(526,24): error CS0534: 'ActivityResultContracts.OpenDocument' does not implement inherited abstract member 'ActivityResultContract.CreateIntent(Context, Object?)' [./generated/androidx.activity.activity/androidx.activity.activity.csproj::TargetFramework=net8.0-android]
./generated/androidx.activity.activity/obj/Debug/net8.0-android/generated/src/AndroidX.Activity.Result.Contract.ActivityResultContracts.cs(776,24): error CS0534: 'ActivityResultContracts.OpenMultipleDocuments' does not implement inherited abstract member 'ActivityResultContract.CreateIntent(Context, Object?)' [./generated/androidx.activity.activity/androidx.activity.activity.csproj::TargetFramework=net8.0-android]
./generated/androidx.activity.activity/obj/Debug/net8.0-android/generated/src/AndroidX.Activity.Result.Contract.ActivityResultContracts.cs(1638,31): error CS0534: 'ActivityResultContracts.RequestMultiplePermissions' does not implement inherited abstract member 'ActivityResultContract.CreateIntent(Context, Object?)' [./generated/androidx.activity.activity/androidx.activity.activity.csproj::TargetFramework=net8.0-android]
-->

For errors like:

error CS0534: 'ActivityResultContracts.OpenDocument' does not implement inherited abstract member 'ActivityResultContract.CreateIntent(Context, Object?)'

Fix is to change parameter managedType (Variant A) like

<attr
path="/api/package[@name='androidx.activity.result.contract']/class[@name='ActivityResultContracts.CreateDocument']/method[@name='createIntent' and count(parameter)=2 and parameter[1][@type='android.content.Context'] and parameter[2][@type='java.lang.String']]/parameter[2]"
name="managedType"
>
Java.Lang.Object
</attr>

but in some cases this does not work:

 error CS0266: Cannot implicitly convert type 'System.Collections.Generic.IList<Android.Net.Uri>' to 'Java.Lang.Object'. An explicit conversion exists (are you missing a cast?)

Less optional alternative (Variant B) would be to change native Java typewhich leads to similar errors:

Variant B - change `type`
./generated/androidx.activity.activity/obj/Debug/net8.0-android/generated/src/AndroidX.Activity.Result.Contract.ActivityResultContracts.cs(883,13): error CS0266: Cannot implicitly convert type 'System.Collections.Generic.IList<Android.Net.Uri>' to 'Java.Lang.Object'. An explicit conversion exists (are you missing a cast?) [./generated/androidx.activity.activity/androidx.activity.activity.csproj::TargetFramework=net8.0-android]
./generated/androidx.activity.activity/obj/Debug/net8.0-android/generated/src/AndroidX.Activity.Result.Contract.ActivityResultContracts.cs(1104,13): error CS0266: Cannot implicitly convert type 'System.Collections.Generic.IList<Android.Net.Uri>' to 'Java.Lang.Object'. An explicit conversion exists (are you missing a cast?) [./generated/androidx.activity.activity/androidx.activity.activity.csproj::TargetFramework=net8.0-android]
./generated/androidx.activity.activity/obj/Debug/net8.0-android/generated/src/AndroidX.Activity.Result.Contract.ActivityResultContracts.cs(1784,13): error CS0266: Cannot implicitly convert type 'System.Collections.Generic.IDictionary<string, Java.Lang.Boolean>' to 'Java.Lang.Object'. An explicit conversion exists (are you missing a cast?) [./generated/androidx.activity.activity/androidx.activity.activity.csproj::TargetFramework=net8.0-android]
<attr
path="/api/package[@name='androidx.activity.result.contract']/class[@name='ActivityResultContracts.OpenDocument']/method[@name='createIntent' and count(parameter)=2 and parameter[1][@type='android.content.Context'] and parameter[2][@type='java.lang.String[]']]/parameter[2]"
name="type"
>
java.lang.Object
</attr>
<attr
path="/api/package[@name='androidx.activity.result.contract']/class[@name='ActivityResultContracts.OpenMultipleDocuments']/method[@name='createIntent' and count(parameter)=2 and parameter[1][@type='android.content.Context'] and parameter[2][@type='java.lang.String[]']]/parameter[2]"
name="type"
>
java.lang.Object
</attr>
<attr
path="/api/package[@name='androidx.activity.result.contract']/class[@name='ActivityResultContracts.RequestMultiplePermissions']/method[@name='createIntent' and count(parameter)=2 and parameter[1][@type='android.content.Context'] and parameter[2][@type='java.lang.String[]']]/parameter[2]"
name="type"
>
java.lang.Object
</attr>

@moljac
Copy link
Contributor Author

moljac commented Dec 19, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moljac
Copy link
Contributor Author

moljac commented Mar 15, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moljac
Copy link
Contributor Author

moljac commented Mar 16, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moljac
Copy link
Contributor Author

moljac commented Mar 16, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moljac
Copy link
Contributor Author

moljac commented Mar 16, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moljac
Copy link
Contributor Author

moljac commented Mar 17, 2025

Overall this PR changes thousands of lines: image

Yes. I am aware, a lot of comments and generated code were deleted. Then I had to move some of the metadata to understand and find workaround for cases when generated metadata was used and moving metadata within the file is deletion + addition.

Should we split this up into smaller PRs?

Maybe I can create one where only deleted files and comments are, but chunk of work was bigger than usual, so is PR.

Maybe it would be easier to do subsets of packages, maybe AndroidX first?

I don't see too much advantages in package subsets. There are GPS-FB dependencies for AndroidX. Because of those dependencies merging 2 repos made updates significantly easier, because everything is in single repo. OK. I am keeping PR smaller through more frequent updates, although this became problematic lately due to some tooling limitations I need to investigate and didn't have time for that yet.

@moljac
Copy link
Contributor Author

moljac commented Mar 18, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moljac
Copy link
Contributor Author

moljac commented Mar 19, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants