Skip to content

Conversation

@jonpryor
Copy link
Contributor

The internal Xamarin.Android CI system is reporting an error when
running the generator unit tests, but it's a false positive.
It's not a bug in generator. Rather, it's a bug in Mono 4.2, in
which CSharpCodeCompiler treats multi-line warnings as errors.
Since the generator unit tests emit such multi-line warnings when
hiding Java.Lang.Object.Handle (via
Xamarin.Test.SomeObject.Handle()) and System.Exception.Message
(via Java.Lang.Throwable.Message), and Mono 4.2 treats the
multi-line warnings as errors, the tests fail.

There are three plausible solutions so that we don't get erroneous
reports from the generator tests when running on Mono 4.2:

  1. Remove those tests. (Uh...no.)
  2. Upgrade Mono on the CI machine to Mono 4.4, which has the fix.
  3. Fix generator so that the warning isn't emitted.

(2) would require an unknown timeframe with unknown repercussions.

Thus, the chosen fix: improve generator so that the warning isn't
generated.

The internal Xamarin.Android CI system is reporting an error when
running the `generator` unit tests, but it's a false positive.
It's *not* a bug in `generator`. Rather, it's a bug in Mono 4.2, in
which [`CSharpCodeCompiler` treats multi-line warnings as errors][0].
Since the `generator` unit tests emit such multi-line warnings when
hiding `Java.Lang.Object.Handle` (via
`Xamarin.Test.SomeObject.Handle()`) and `System.Exception.Message`
(via `Java.Lang.Throwable.Message`), and Mono 4.2 treats the
multi-line warnings as errors, the tests fail.

There are three plausible solutions so that we don't get erroneous
reports from the `generator` tests when running on Mono 4.2:

1. Remove those tests. (Uh...no.)
2. Upgrade Mono on the CI machine to Mono 4.4, which has the fix.
3. Fix `generator` so that the warning isn't emitted.

(2) would require an unknown timeframe with unknown repercussions.

Thus, the chosen fix: improve `generator` so that the warning isn't
generated.

[0]: mono/mono#2248
@atsushieno
Copy link
Contributor

Your fix is not going to change the situation with respect to this, but shouldn't Message override Exception.Message instead of declaring as new?

I'm also quite unsure why Handle is generated like that, shouldn't that rather be eliminated?

@jonpryor
Copy link
Contributor Author

Message is new and not override because of java.lang.Throwable.getMessage(), and (mostly) the generator doesn't know about Exception.Message.

So it was easier to make it new.

Additionally, Throwable.Message is already (implicitly) new. I don't know if it would be a breaking change to change it to override. :-(

I'm also quite unsure why Handle is generated like that, shouldn't that rather be eliminated?

I don't understand. Do you mean SomeObject.Handle()? If so, that was to test the qualification of the Object.Handle property, so that new Handle() members could be introduced without breaking things.

@jonpryor jonpryor merged commit 24f8084 into dotnet:master Jun 23, 2016
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants