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.

Sign up for GitHub

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

Jump to bottom

Bump to mono:2019-10 #7192

Merged
merged 42 commits into from
Dec 4, 2019
Merged

Bump to mono:2019-10 #7192

merged 42 commits into from
Dec 4, 2019

Conversation

directhex
Copy link
Member

No description provided.

@directhex directhex added run-all-tests Run all our tests. run-internal-tests If tests should be executed on the internal Jenkins instance. skip-api-comparison Skips API / generator diffs when testing pull requests labels Oct 8, 2019
@monojenkins

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@rolfbjarne
Copy link
Member

rolfbjarne commented Oct 8, 2019 •
edited
Loading

Looks like something in the linker changed, so mmp doesn't build anymore:

[2019-10-08T13:42:03.020Z] /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/builds/mono-ios-sdk-destdir/ios-sources/external/linker/src/linker/Linker.Steps/OutputStep.cs(110,15): error CS0246: The type or namespace name 'OutputException' could not be found (are you missing a using directive or an assembly reference?) [/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/mmp/mmp.csproj]
13:42:03  /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/linker/MobileSweepStep.cs(52,5): error CS0103: The name 'SweepCollectionNonAttributable' does not exist in the current context [/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/mmp/mmp.csproj]
13:42:03  /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/linker/MobileSweepStep.cs(57,5): error CS0103: The name 'SweepCollectionNonAttributable' does not exist in the current context [/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/mmp/mmp.csproj]
13:42:03  Done Building Project "/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/mmp/mmp.csproj" (default targets) -- FAILED.
13:42:03  Done Building Project "/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/Xamarin.Mac.sln" (mmp target(s)) -- FAILED.

@directhex
Copy link
Member Author

A new source file is needed to build the linker - mono/mono@08e406e

@thaystg
Fixing
b9f1a4e 
/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/builds/mono-ios-sdk-destdir/ios-sources/external/linker/src/linker/Linker.Steps/OutputStep.cs(110,15): error CS0246: The type or namespace name ‘OutputException’ could not be found (are you missing a using directive or an assembly reference?) [/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/mmp/mmp.csproj]
@monojenkins

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@monojenkins

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@monojenkins

This comment has been minimized.

Sign in to view
@rolfbjarne rolfbjarne added enable-device-build Makes our build include device support (which we disable for simple PRs to speed them up) skip-device-tests Skip device tests and removed enable-device-build Makes our build include device support (which we disable for simple PRs to speed them up) labels Oct 9, 2019
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@monojenkins

This comment has been minimized.

Sign in to view
@rolfbjarne
Copy link
Member

The test report says that the watchOS test failures are known failures (HE0038), but that's not correct. The watch apps crash upon launch with this:

error: mono_threads_enter_gc_safe_region Cannot transition thread 0x6dd080 from STATE_BLOCKING with DO_BLOCKING

Crash report: dont link_2019-10-11-150426_xam-mac-mini-27-20191011_150427.txt

@rolfbjarne rolfbjarne mentioned this pull request Dec 3, 2019
Merged
@baulig
Copy link
Contributor

baulig commented Dec 3, 2019

Yes, looks like that's the case: mono/corefx@f7a852f

If it doesn't work at all on those older OS X versions, then we should replace the static init with an OS version check - and ideally discuss / upstream that.

@baulig
Copy link
Contributor

baulig commented Dec 3, 2019

Alternatively, we could also add a check to that GetVersion () function - and either hard fail or even try whether we can silently fall back. I already upgraded my machine to Catalina, so I will need some help testing that.

@rolfbjarne
Copy link
Member

The problem is that in 2019-10 the HttpClient implementation comes from corefx, in 2019-08 we were still using mono's version, which defaults to HTTP 1.1: https://github.com/mono/mono/blob/56325f4097f3460b05395c68ccf3f47996ceea52/mcs/class/System.Net.Http/System.Net.Http/HttpRequestMessage.cs#L118

Martin Baulig and others added 2 commits December 3, 2019 18:57
[CFNetwork]: Http 2.0 requires OS X 10.11 or later.
7a0ba27 
Check whether `_HTTPVersion2_0` is available and fallback to HTTP 1.1 otherwise.
@akoeplinger
Remove accidental bump to macios-binaries
c51ef02
@xamarin xamarin deleted a comment from monojenkins Dec 3, 2019
@xamarin xamarin deleted a comment from monojenkins Dec 3, 2019
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@monojenkins

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@akoeplinger
Copy link
Member

Ok, we're down to this failure now on 10.10:

2019-12-03T20:08:20.173Z] 1) RejectSslCertificatesServicePointManager(Foundation.NSUrlSessionHandler) (MonoTests.System.Net.Http.MessageHandlerTest.RejectSslCertificatesServicePointManager(Foundation.NSUrlSessionHandler))
[2019-12-03T20:08:20.173Z]      Expected: True
[2019-12-03T20:08:20.173Z]   But was:  False
[2019-12-03T20:08:20.173Z] 
[2019-12-03T20:08:20.173Z]   at MonoTests.System.Net.Http.MessageHandlerTest.RejectSslCertificatesServicePointManager (System.Type handlerType) [0x00169] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tests/monotouch-test/System.Net.Http/MessageHandlers.cs:225 
[2019-12-03T20:08:20.173Z]   at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
[2019-12-03T20:08:20.173Z]   at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0006a] in /Library/Frameworks/Xamarin.Mac.framework/Versions/Current/src/Xamarin.Mac/mcs/class/corlib/System.Reflection/RuntimeMethodInfo.cs:395

@xamarin-release-manager

This comment has been minimized.

Sign in to view
@xamarin-release-manager

This comment has been minimized.

Sign in to view
@monojenkins

This comment has been minimized.

Sign in to view
@xamarin-release-manager
Copy link
Collaborator

❌ Build was (probably) aborted

🔥 Jenkins job (on internal Jenkins) failed in stage(s) 'Test run, Test run' 🔥

✅ Build succeeded
✅ Packages:

✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
🔥 Test run failed 🔥

Test results

1 tests failed, 163 tests passed.

Failed tests

  • monotouch-test/watchOS 32-bits - simulator/Debug: Crashed

@monojenkins
Copy link
Collaborator

Build success
✅ Build succeeded
✅ API Diff (from stable)
❎ Skipped API comparison because the PR has the label 'skip-api-comparison'
✅ Test run succeeded

rolfbjarne
rolfbjarne approved these changes Dec 4, 2019
View reviewed changes
Copy link
Member

@rolfbjarne rolfbjarne left a comment

Choose a reason for hiding this comment

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

This looks good to merge, all test failures (including device test failures) occur in master as well.

@VincentDondain VincentDondain removed the do-not-merge Do not merge this pull request label Dec 4, 2019
@VincentDondain
Copy link
Contributor

Mono 2019-10 was merged in Xamarin.Android, we want to land that for 16.5 P2 so this is ready to be merged.

@VincentDondain VincentDondain merged commit 5d4ada3 into master Dec 4, 2019
VincentDondain pushed a commit to VincentDondain/xamarin-macios that referenced this pull request Dec 5, 2019
@directhex @VincentDondain
Bump to mono:2019-10 (xamarin#7192)
7542e05 
* Fixed
`/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/builds/mono-ios-sdk-destdir/ios-sources/external/linker/src/linker/Linker.Steps/OutputStep.cs(110,15): error CS0246: The type or namespace name ‘OutputException’ could not be found (are you missing a using directive or an assembly reference?) [/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/mmp/mmp.csproj]`
* Changed the name of the method that is used from linker. Because of this commit dotnet/linker@6be2677
* Added `OutputException.cs` file on `mtouch.csproj`.
* Removing enter_gc_safe and exit_gc_safe because now it's already gc_safe in this part of code, after a mono change.
* Added known exceptions to LLVM exception list.
* Needs `ifdef` because of this mono/mono#17260.
* Bump MIN_MONO_VERSION to 6.8.0.41 and point MIN_MONO_URL to the PR.
* Add ENABLE_IOS=1 and ENABLE_MAC=1.
* Added switch to disable packaged mono build
* [Tests] Ignore tests that fail on 32b.
    Ignore the test on 32b, and filled issue: mono/mono#17752
* [Tests] Ignore a couple of tests causing OOM.
    Hopefully fixes xamarin/maccore#1659 for good.
* Ignore `MM0135` test on Catalina+ because it needs Xcode 9.4.
* [monotouch-test] Add null checks for teardown when test didn't run because of a too early OS version.
* [CFNetwork]: Http 2.0 requires OS X 10.11 or later.
    Check whether `_HTTPVersion2_0` is available and fallback to HTTP 1.1 otherwise.

* xamarin#7346
* This bumps Mono to use mono/mono#17645 (which is the 2019-10 backport
of mono/mono#17628).
* The big user-visible change is in regards to certificate validation, everything below are just
some minor adjustments to tests.

CoreFX uses a completely new `HttpClientHandler` implementation called `SocketsHttpHandler`,
which you can find at https://github.com/dotnet/corefx/tree/release/3.0/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler.

Since this is not based on the web stack anymore, it does not use any of the related APIs such
as `ServicePointManager` or `WebException`.

There is a new API called `HttpClientHandler.ServerCertificateCustomValidationCallback`.
- https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclienthandler.servercertificatecustomvalidationcallback?view=netframework-4.8
- https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Unix.cs#L154
- https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Windows.cs#L383

The `ServicePointManager.ServerCertificateValidationCallback` is no longer invoked and on
certificate validation failure, `AuthenticationException` (from `System.Security.Authentication`)
is thrown instead of `WebException`.

At the moment, the `NSUrlSessionHandler` still uses it's own validation callback and also still
throws `WebException` on failure; we should probably look into making this consistent with the
other handlers.

* `HttpContent.SerializeToStreamAsync()` is now `protected` (changed from `protected internal`).
  - src/Foundation/NSUrlSessionHandler.cs: changed overload accordingly.
  - src/System.Net.Http/CFContentStream.cs: likewise.

* `HttpHeaders.GetKnownHeaderKind()` is an internal Mono API.
   There is a new internal API called `System.Net.Http.PlatformHelper.IsContentHeader(key)`
   which exists in both the old as well as the new implementation.
   The correct way of doing it with the CoreFX handler is
   `HeaderDescriptor.TryGet (key, out var descriptor) && descriptor.HeaderType == HttpHeaderType.Content`

* `HttpClientHandler.MaxRequestContentBufferSize` is now longer supported, you can set it to
  any non-negative value, the getter will always return 0.
  See https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Core.cs#L18.
  - tests/linker/ios/link sdk/HttpClientHandlerTest.cs: removed assertion from test.

* `HttpMessageInvoker.handler` is a `protected private` field - in the CoreFX handler, it is
  called `_handler` and `private`.  This is accessed via reflection by some of the tests, which are
  now using the new name.
  - tests/mmptest/src/MMPTest.cs: here
  - tests/mtouch/MTouch.cs: here

* tests/monotouch-test/System.Net.Http/MessageHandlers.cs:
  Adjust `RejectSslCertificatesServicePointManager` to reflect the certificate validation
  changes described above.
  - FIXME: There was an `Assert.Ignore()` related to `NSUrlSessionHandler` and macOS 10.10;
    I removed that to reenable the test because the description linked to an old issue in
    the private repo that was referenced by several "Merged" PR's, so it looked to me that
    this might have already been fixed - and I also didn't see why it would fail there.
mandel-macaque pushed a commit that referenced this pull request Dec 18, 2019
@VincentDondain @mandel-macaque
Bump to mono:2019-10 (#7192) (#7547)
fd3f86d 
* Fixed
`/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/builds/mono-ios-sdk-destdir/ios-sources/external/linker/src/linker/Linker.Steps/OutputStep.cs(110,15): error CS0246: The type or namespace name ‘OutputException’ could not be found (are you missing a using directive or an assembly reference?) [/Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/tools/mmp/mmp.csproj]`
* Changed the name of the method that is used from linker. Because of this commit dotnet/linker@6be2677
* Added `OutputException.cs` file on `mtouch.csproj`.
* Removing enter_gc_safe and exit_gc_safe because now it's already gc_safe in this part of code, after a mono change.
* Added known exceptions to LLVM exception list.
* Needs `ifdef` because of this mono/mono#17260.
* Bump MIN_MONO_VERSION to 6.8.0.41 and point MIN_MONO_URL to the PR.
* Add ENABLE_IOS=1 and ENABLE_MAC=1.
* Added switch to disable packaged mono build
* [Tests] Ignore tests that fail on 32b.
    Ignore the test on 32b, and filled issue: mono/mono#17752
* [Tests] Ignore a couple of tests causing OOM.
    Hopefully fixes xamarin/maccore#1659 for good.
* Ignore `MM0135` test on Catalina+ because it needs Xcode 9.4.
* [monotouch-test] Add null checks for teardown when test didn't run because of a too early OS version.
* [CFNetwork]: Http 2.0 requires OS X 10.11 or later.
    Check whether `_HTTPVersion2_0` is available and fallback to HTTP 1.1 otherwise.

* #7346
* This bumps Mono to use mono/mono#17645 (which is the 2019-10 backport
of mono/mono#17628).
* The big user-visible change is in regards to certificate validation, everything below are just
some minor adjustments to tests.

CoreFX uses a completely new `HttpClientHandler` implementation called `SocketsHttpHandler`,
which you can find at https://github.com/dotnet/corefx/tree/release/3.0/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler.

Since this is not based on the web stack anymore, it does not use any of the related APIs such
as `ServicePointManager` or `WebException`.

There is a new API called `HttpClientHandler.ServerCertificateCustomValidationCallback`.
- https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclienthandler.servercertificatecustomvalidationcallback?view=netframework-4.8
- https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Unix.cs#L154
- https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Windows.cs#L383

The `ServicePointManager.ServerCertificateValidationCallback` is no longer invoked and on
certificate validation failure, `AuthenticationException` (from `System.Security.Authentication`)
is thrown instead of `WebException`.

At the moment, the `NSUrlSessionHandler` still uses it's own validation callback and also still
throws `WebException` on failure; we should probably look into making this consistent with the
other handlers.

* `HttpContent.SerializeToStreamAsync()` is now `protected` (changed from `protected internal`).
  - src/Foundation/NSUrlSessionHandler.cs: changed overload accordingly.
  - src/System.Net.Http/CFContentStream.cs: likewise.

* `HttpHeaders.GetKnownHeaderKind()` is an internal Mono API.
   There is a new internal API called `System.Net.Http.PlatformHelper.IsContentHeader(key)`
   which exists in both the old as well as the new implementation.
   The correct way of doing it with the CoreFX handler is
   `HeaderDescriptor.TryGet (key, out var descriptor) && descriptor.HeaderType == HttpHeaderType.Content`

* `HttpClientHandler.MaxRequestContentBufferSize` is now longer supported, you can set it to
  any non-negative value, the getter will always return 0.
  See https://github.com/dotnet/corefx/blob/c1778515a3bee34cc09c757b5563d0af0c8b1e99/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Core.cs#L18.
  - tests/linker/ios/link sdk/HttpClientHandlerTest.cs: removed assertion from test.

* `HttpMessageInvoker.handler` is a `protected private` field - in the CoreFX handler, it is
  called `_handler` and `private`.  This is accessed via reflection by some of the tests, which are
  now using the new name.
  - tests/mmptest/src/MMPTest.cs: here
  - tests/mtouch/MTouch.cs: here

* tests/monotouch-test/System.Net.Http/MessageHandlers.cs:
  Adjust `RejectSslCertificatesServicePointManager` to reflect the certificate validation
  changes described above.
  - FIXME: There was an `Assert.Ignore()` related to `NSUrlSessionHandler` and macOS 10.10;
    I removed that to reenable the test because the description linked to an old issue in
    the private repo that was referenced by several "Merged" PR's, so it looked to me that
    this might have already been fixed - and I also didn't see why it would fail there.
@rolfbjarne rolfbjarne added the not-notes-worthy Ignore for release notes label Sep 14, 2020
@mandel-macaque mandel-macaque deleted the mono-2019-10 branch December 7, 2021 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akoeplinger akoeplinger akoeplinger left review comments

@rolfbjarne rolfbjarne rolfbjarne approved these changes

@spouliot spouliot spouliot approved these changes

@mandel-macaque mandel-macaque mandel-macaque approved these changes

@VincentDondain VincentDondain VincentDondain approved these changes

@chamons chamons Awaiting requested review from chamons

Assignees

@rolfbjarne rolfbjarne

@spouliot spouliot

@thaystg thaystg

@VincentDondain VincentDondain

Labels
not-notes-worthy Ignore for release notes run-all-tests Run all our tests. run-internal-tests If tests should be executed on the internal Jenkins instance. skip-api-comparison Skips API / generator diffs when testing pull requests skip-device-tests Skip device tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@directhex @monojenkins @xamarin-release-manager @rolfbjarne @VincentDondain @thaystg @mandel-macaque @baulig @akoeplinger @spouliot