Skip to content

Conversation

@grendello
Copy link
Contributor

@grendello grendello commented Jun 12, 2023

Xamarin.Android's native runtime logs a handful of version information
on application startup. This information includes XA version, up until
now represented by the $(ProductVersion) MSBuild property. However,
the property hasn't been updated for quite a while, with Xamarin.Android
having switched to $(AndroidPackVersion) instead.

Modify xaprepare to use $(AndroidPackVersion) instead of
$(ProductVersion) so that we log the correct version at application
startup.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I guess it uses a task to get the commit distance...

 (_GetSubmodulesVersionInfo target) -> 
/Users/builder/azdo/_work/1/s/xamarin-android/build-tools/scripts/XAVersionInfo.targets(25,5): error MSB4062: The "Xamarin.Android.BuildTools.PrepTasks.GitCommitInfo" task could not be loaded from the assembly /Users/builder/azdo/_work/1/s/xamarin-android/bin/BuildRelease/netstandard2.0/xa-prep-tasks.dll. Could not load file or assembly '/Users/builder/azdo/_work/1/s/xamarin-android/bin/BuildRelease/netstandard2.0/xa-prep-tasks.dll'. The system cannot find the file specified. [/Users/builder/azdo/_work/1/s/xamarin-android/build-tools/xaprepare/xaprepare/xaprepare.csproj]
/Users/builder/azdo/_work/1/s/xamarin-android/build-tools/scripts/XAVersionInfo.targets(25,5): error MSB4062:  Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask.

We could either try to move this task to xa-prep-tasks.csproj, or use these values instead?

https://github.com/xamarin/xamarin-android/blob/4125c0ce14549623df7e860bf4ad67d0d9738aad/Directory.Build.props#L37-L38

These aren't quite as good, with the distance missing.

@grendello
Copy link
Contributor Author

It built for me locally because I didn't build from a clean checkout, doh. Yeah, I think I'll switch to the properties in Directory.Build.props, they're good enough.

Xamarin.Android's native runtime logs a handful of version information
on application startup.  This information includes XA version, up until
now represented by the `$(ProductVersion)` MSBuild property.  However,
the property hasn't been updated for quite a while, with Xamarin.Android
having switched to `$(AndroidPackVersion)` instead.

Modify xaprepare to use `$(AndroidPackVersion)` instead of
`$(ProductVersion)` so that we log the correct version at application
startup.
@grendello grendello force-pushed the xaprepare-version branch from 98e953d to 5ef90a5 Compare June 12, 2023 17:33
@jonpryor
Copy link
Contributor

Should this PR also remove $(ProductVersion) entirely, as it's "vestigial" and not useful anymore?

* main: (102 commits)
  Bump to dotnet/installer@2809943e7a 8.0.100-rc.2.23431.5 (dotnet#8317)
  [build] Use Microsoft OpenJDK 17.0.8 (dotnet#8309)
  [Mono.Android] Add missing `[Flags]` attribute for generated enums. (dotnet#8310)
  Bump to dotnet/installer@c5e45fd659 8.0.100-rc.2.23427.4 (dotnet#8305)
  [xaprepare] Improve dotnet-install script logging (dotnet#8312)
  [xaprepare] Fix dotnet-install script size check (dotnet#8311)
  [Xamarin.Android.Build.Tasks] improve net6.0 "out of support" message (dotnet#8307)
  [monodroid] Fix the EnableNativeAnalyzers build (dotnet#8293)
  Bump to dotnet/installer@56d8c6497c 8.0.100-rc.2.23422.31 (dotnet#8291)
  [Xamarin.Android.Build.Tasks] Fix APT2264 error on Windows. (dotnet#8289)
  [Mono.Android] Marshal .NET stack trace to Throwable.getStackTrace() (dotnet#8185)
  [tests] Skip sign check when installing MAUI (dotnet#8288)
  Bump to xamarin/monodroid@057b17fe (dotnet#8286)
  [Xamarin.Android.Build.Tasks] add $(AndroidStripILAfterAOT) (dotnet#8172)
  Bump to dotnet/installer@ec2c1ec1b1 8.0.100-rc.2.23420.6 (dotnet#8281)
  Bump to dotnet/installer@001d8e4465 8.0.100-rc.2.23417.14 (dotnet#8276)
  [Mono.Android] [IntentFilter] pathSuffix & pathAdvancedPattern  (dotnet#8261)
  $(AndroidPackVersionSuffix)=rc.2; net8 is 34.0.0-rc.2 (dotnet#8278)
  Bump to xamarin/xamarin-android-tools/main@52f0866 (dotnet#8241)
  [build] set file extension for common `ToolExe` values (dotnet#8267)
  ...
@grendello
Copy link
Contributor Author

Should this PR also remove $(ProductVersion) entirely, as it's "vestigial" and not useful anymore?

I guess it depends on whether code like the GetAndroidDefineConstants task is still used? It defines a macro based on ProductVersion and is called here. It appears to be legacy code, but is it really so?

I think ProductVersion should be removed in a separate PR once we will have removed all the traces of legacy code since it's used in a few locations around the code.

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