Status Update
No update yet.
Comments
al...@google.com
there are some image for help
pa...@google.com
I think that the @ in @androidx.car.app.annotations.CarProtocol is confusing Clikt (it has support for using @<file> to include the contents of file onto the command line).
Can you remove the leading @ and see if that fixes the issue?
pa...@google.com
I've reproduced it locally and indeed, adding a @ leading character does cause the problem. Metalava does not have any tests that use the leading @ which is why it slipped through the gap. I would like to deprecate that as it adds no value, makes things confusing and prevents use of a potentially useful mechanism.
The fix for the problem caused by @ is aosp/2640655.
I will keep this open as while the previous change fixes the underlying bug there is also an error in the error handling path which prevented a useful error message from being reported in the first place.
ap...@google.com
Branch: metalava-main
commit 171757cecfe7b1a2a4d3d3c878ad75e0ef37a46c
Author: Paul Duffin <paulduffin@google.com>
Date: Thu Jun 29 13:19:38 2023
Disable @argfile expansion to allow leading @s on annotation classes
Adds tests to cover that (there were no tests for this before).
The added tests reproduced the problem in the bug and worked after this
change.
Bug: 289264921
Test: ./gradlew
Change-Id: I22a03e0a2be4b7503c8306685f471bbbab4ee40f
M src/main/java/com/android/tools/metalava/MetalavaCommand.kt
M src/test/java/com/android/tools/metalava/OptionsTest.kt
M src/test/java/com/android/tools/metalava/ShowAnnotationTest.kt
al...@google.com
On Thu, Jun 29, 2023, 8:45 AM
apphosting-stubby-api--s-7egoogle-2ecom-3agitwatcher <
buganizer-system+apphosting-stubby-api--s-7egoogle-2ecom-3agitwatcher@google.com>
wrote:
pa...@google.com
There is some logic in metalava for removing @
val annotationText = annotationSource.replace("@", "")
That looks particularly nasty as it would allow @andr@oidx.c@ar.ap@p.annotati@ons.CarProt@ocol to work.
I did not find anything like that in other places where I would expect it so it's possible it does not work properly.
ap...@google.com
Branch: metalava-main
commit 653181bfe314e4726c6c248d168eb71a81da7a3f
Author: Paul Duffin <paulduffin@google.com>
Date: Thu Jun 29 14:18:53 2023
Improve error handling when failures occur during option parsing
Previously, if an error occurred before the `CommonOptions` was
initialized then any attempt to print the help or usage text would
cause another error to be thrown which would hide the root cause. That
happened because it would try and access the `CommonOptions.terminal`
property which would not be initialized and so would fail.
This change protects against that by renaming `terminal` to
`unsafeTerminal` and adding a new `terminal` property that is lazily
initialized and will ignore exceptions thrown while trying to access
`unsafeTerminal`. Default handling was moved into the `terminal`
property so that it was consistent whether the `unsafeTerminal` was
initialized or not. Removing the `default` from `unsafeTerminal`
caused it to be of type `Terminal?` with `null` being not set. The
lazy initializer for `terminal` handles an exception and null in the
same way.
It also adds a test which will ensure that when an error does occur
before `CommonOptions` is properly initialized that the
`unsafeTerminal` was not set. That is necessary because the window
within which this problem occurs is very small and it is quite possible
that a refactoring could narrow that window to the point that the test
was no longer testing the error handling in `terminal` which could
lead to that error handling degrading.
Bug: 289264921
Test: ./gradlew
Change-Id: Ifc519a6fe46bfb7400d934c5630b5fdea8f86704
M src/main/java/com/android/tools/metalava/CommonOptions.kt
M src/main/java/com/android/tools/metalava/MetalavaCommand.kt
A src/test/java/com/android/tools/metalava/MetalavaCommandTest.kt
Description
Migration to Clikt is breaking
car.app:apphandling of proto API surface: