Some fixes to work with modern Gradle/Android

[tritao]

No description provided.

[polymonster]

Looks good, thanks for the updates and maintenance!

Do you mind adding info to the readme file to document the androidnamespace feature?

Edit: update - Please check the CI issue, and my comments which came from after the CI ran.

Cheers!

[polymonster]

Maybe we should consider making this a parameter instead of hardcoding it

[tritao]

I considered but seems a bit overkill. Just upgraded to 3.0 which is from 2014, should be safe enough.

[polymonster]

I still think it’s worth making this a configurable parameter and keeping the old default. You’re the first person who’s come along and wanted to change this value and so at this point the best way to do so is by a configurable option, it’s extendible for future use cases and it can retain the default value.

I’m not sure why 2.6 was used but it was the default for some reason and has been that way ever since.

people are using this in production environments and I want to ensure any new changes incoming are additive.

[tritao]

Updated PR with the changes requested. Also added 5cc63ed just to clean the log a bit. But happy to remove from the PR if you want to keep it.

[polymonster]

..

[polymonster]

The change looks good in principle and thanks for checking the gradle version thoroughly.

I am just wondering about some scenarios, and am not entirely sure what the package attribute does.

you mentioned the androidnamespace comment, so this package attribute is now replaced with the androidnamespace?

I am wondering what happens in the scenario where the package attribute is deprecated (but not removed) we omit this code, but the user does not supply an androidnamespace in the lua config.

[polymonster]

Thanks for the update, a few more things to chew on there. Hopefully you can still work with your changes locally and we can carefully move toward merging into main.

There aren’t really enough test cases in the CI here to move safely. so I am just picking through the changes to make them as low friction and as backwards compatible as possible to maintain any existing lua configs

[tritao]

The change looks good in principle and thanks for checking the gradle version thoroughly.

I am just wondering about some scenarios, and am not entirely sure what the package attribute does.

you mentioned the androidnamespace comment, so this package attribute is now replaced with the androidnamespace?

I am wondering what happens in the scenario where the package attribute is deprecated (but not removed) we omit this code, but the user does not supply an androidnamespace in the lua config.

I think one issue might exists if the user supplies a recent enough gradle version but does not provide the androidnamespace key. We can check for this case and provide a warning/error to the user. Let me know if that would work and I can add it.

Regarding the message verbosity issue, we can use verbosef, would that work?

There aren’t really enough test cases in the CI here to move safely. so I am just picking through the changes to make them as low friction and as backwards compatible as possible to maintain any existing lua configs

Aside from the CMake minimum version, which should not really be an issue (3.0 was released 10 years ago, but can add an option as you wish), the behaviour does not change for existing code. It only changes behaviour for recent Gradle Android versions (which is broken at the moment), so in theory it should only improve things and never break anything. I also tested and if you provide androidnamespace for old Gradle versions it still works, but I can remove the output for older Gradle versions, just to be even safer.

[polymonster]

Can we generate a default androidnamespace value for gradle versions that require it? Much like how the previous package attribute was derived by the project name?

verbosef sounds good for the log.

With regards to CMake version, I really think being able to specify it in the config is the best way to go, it provides more flexibility.

[tritao]

Can we generate a default androidnamespace value for gradle versions that require it? Much like how the previous package attribute was derived by the project name?

Sounds like a good solution, will try that.

verbosef sounds good for the log.

With regards to CMake version, I really think being able to specify it in the config is the best way to go, it provides more flexibility.

Alright, though that raises the question of the default value. Using 2.6 will still give trouble as I got an error with that version by default, without any changes to the generated CMake projects.

[polymonster]

Took these changes in the end, to fix windows build issues. seems like it's easiest just to upgrade the toolchain to gradle 8 at least now