Skip to content

Conversation

@xmcclure
Copy link
Contributor

To be merged after Wrench testing is complete and after the next xamarin-android branch.

This is an adjustment of PR #430 . Some discussion is archived there.

@xmcclure xmcclure added do-not-merge PR should not be merged. enhancement Proposed change to current functionality. labels Feb 15, 2017
@akoeplinger
Copy link
Member

build

@xmcclure xmcclure force-pushed the mono-2017-02 branch 6 times, most recently from 80e952c to 985ae35 Compare February 17, 2017 21:30
@xmcclure
Copy link
Contributor Author

build

1 similar comment
@xmcclure
Copy link
Contributor Author

build

@xmcclure xmcclure force-pushed the mono-2017-02 branch 2 times, most recently from 8b9ffb4 to 2ce37ff Compare February 21, 2017 22:30
@xmcclure
Copy link
Contributor Author

build

@xmcclure
Copy link
Contributor Author

Rebased on top of c76fc64

@xmcclure
Copy link
Contributor Author

build

3 similar comments
@xmcclure
Copy link
Contributor Author

build

@xmcclure
Copy link
Contributor Author

build

@xmcclure
Copy link
Contributor Author

build

@xmcclure
Copy link
Contributor Author

Rebased to pick up what is hopefully a fix for the failing Xamarin.Android.Build.Tests.PackagingTest.CheckBuildIdIsUnique test. Also removed the ManifestDocumentElement.cs fix (it is now in base xamarin.android) and the -v on monosymbolicate (it didn't help).

@borgdylan
Copy link

Is this PR supposed to bring in support for ppdb debugging, or is that still on the waiting list?

@xmcclure xmcclure force-pushed the mono-2017-02 branch 2 times, most recently from 763c982 to a75a2af Compare March 8, 2017 21:28
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 9, 2017
While reviewing [PR 445][0] I noticed that it said that *41* tests had
been run, and 0 failed. Compare to e.g. [PR 474][1], which has *164*
passing tests.

Something isn't right here.

Further investigation showed that for PR 445, execution on the
emulator had crashed ("Segmentation fault"). The error was ignored.

Ignoring errors was in fact By Design™; see commit 6358a64.

So what we need is for the `RunUnitTestApks` target to *not* report
errors, while still reporting errors.

Solve this conundrum by updating the `<RunInstrumentationTests/>` task
so that when the `nunit2-results-path` value can't be found, it
reports a *successful* run, and sets the
`RunInstrumentationTests.FailedToRun` property to the name of the
component that failed. This allows the `ReleaseAndroidTarget` target
to check for any failures, and report an `<Error/>` after shutting
down the emulator instance.

This should allow the build to "fail" when `nunit2-results-path` can't
be found -- a sign that the `.apk` unit tests didn't finish executing
-- while also allowing the emulator to be shut down.

Aside: `xbuild` doesn't support `ContinueOnError="ErrorAndContinue"`,
nor does it set `<Output/>` properties whenever a task fails.
Consequently, the `<RunInstrumentationTests/>` task *must* return true
when `nunit2-results-path` isn't found, because there's no other way
to convince `xbuild` to collect the `<Output/>` values.

[0]: dotnet#445
[1]: dotnet#474
dellis1972 pushed a commit that referenced this pull request Mar 9, 2017
While reviewing [PR 445][0] I noticed that it said that *41* tests had
been run, and 0 failed. Compare to e.g. [PR 474][1], which has *164*
passing tests.

Something isn't right here.

Further investigation showed that for PR 445, execution on the
emulator had crashed ("Segmentation fault"). The error was ignored.

Ignoring errors was in fact By Design™; see commit 6358a64.

So what we need is for the `RunUnitTestApks` target to *not* report
errors, while still reporting errors.

Solve this conundrum by updating the `<RunInstrumentationTests/>` task
so that when the `nunit2-results-path` value can't be found, it
reports a *successful* run, and sets the
`RunInstrumentationTests.FailedToRun` property to the name of the
component that failed. This allows the `ReleaseAndroidTarget` target
to check for any failures, and report an `<Error/>` after shutting
down the emulator instance.

This should allow the build to "fail" when `nunit2-results-path` can't
be found -- a sign that the `.apk` unit tests didn't finish executing
-- while also allowing the emulator to be shut down.

Aside: `xbuild` doesn't support `ContinueOnError="ErrorAndContinue"`,
nor does it set `<Output/>` properties whenever a task fails.
Consequently, the `<RunInstrumentationTests/>` task *must* return true
when `nunit2-results-path` isn't found, because there's no other way
to convince `xbuild` to collect the `<Output/>` values.

[0]: #445
[1]: #474
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 10, 2017
As noted in commit 3b893cd, [PR 445][0] was showing up as a
successful build, while none of the .apk tests ran.

Commit 3b893cd "improves" matters by ensuring that we flag this as an
error, instead of blithely ignoring it.

Unfortunately, knowing that there's an error doesn't help in
diagnosing what the error *is*. In the case of segmentation faults,
the `INSTRUMENTATION_RESULT` messages will *not* be helpful.

What *would* be helpful is `adb logcat` output, which we don't
capture.

Let's fix that: if an error occurs -- e.g.
`RunInstrumentationTests.FailedToRun` is a non-empty string, which is
collected in the `@(_FailedComponent)` item group -- then we should
run `adb logcat -d` before terminating the emulator. This will cause
the build and test log output to contain `adb logcat` output, which
may provide *some* additional context regarding .apk failures.

[0]: dotnet#445
radekdoulik pushed a commit that referenced this pull request Mar 10, 2017
As noted in commit 3b893cd, [PR 445][0] was showing up as a
successful build, while none of the .apk tests ran.

Commit 3b893cd "improves" matters by ensuring that we flag this as an
error, instead of blithely ignoring it.

Unfortunately, knowing that there's an error doesn't help in
diagnosing what the error *is*. In the case of segmentation faults,
the `INSTRUMENTATION_RESULT` messages will *not* be helpful.

What *would* be helpful is `adb logcat` output, which we don't
capture.

Let's fix that: if an error occurs -- e.g.
`RunInstrumentationTests.FailedToRun` is a non-empty string, which is
collected in the `@(_FailedComponent)` item group -- then we should
run `adb logcat -d` before terminating the emulator. This will cause
the build and test log output to contain `adb logcat` output, which
may provide *some* additional context regarding .apk failures.

[0]: #445
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 15, 2017
Commit 2063ab7 tried to improve error messages when required
dependencies are missing. Unfortunately, there was one case not
considered: when `$(AutoProvision)` is False, and a dependency to
install needs to be downloaded.

For example, take [PR 445][0], which requires an updated mono.
If the system mono is unsuported, the required mono hasn't already
been downloaded, and `$(AutoProvision)` is False, then the following
"helpful" error message is provided:

	error : Could not find required program 'mono'. Please run:
	installer -pkg "$HOME/android-archives/MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg" -target /

If `MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg` hasn't
already been downloaded, then the suggested command will fail.

This is not helpful. :-)

Thus, when `$(AutoProvision)` is False and URLs need to be downloaded,
then provide a message stating that:

	Please download
	`https://.../MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg`
	into `$HOME/android-archives/MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg`.

[0]: dotnet#445
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Mar 15, 2017
Commit 2063ab7 tried to improve error messages when required
dependencies are missing. Unfortunately, there was one case not
considered: when `$(AutoProvision)` is False, and a dependency to
install needs to be downloaded.

For example, take [PR 445][0], which requires an updated mono.
If the system mono is unsuported, the required mono hasn't already
been downloaded, and `$(AutoProvision)` is False, then the following
"helpful" error message is provided:

	error : Could not find required program 'mono'. Please run:
	installer -pkg "$HOME/android-archives/MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg" -target /

If `MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg` hasn't
already been downloaded, then the suggested command will fail.

This is not helpful. :-)

Thus, when `$(AutoProvision)` is False and URLs need to be downloaded,
then provide a message stating that:

	Please download
	`https://.../MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg`
	into `$HOME/android-archives/MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg`.

[0]: dotnet#445
radekdoulik pushed a commit that referenced this pull request Mar 15, 2017
Commit 2063ab7 tried to improve error messages when required
dependencies are missing. Unfortunately, there was one case not
considered: when `$(AutoProvision)` is False, and a dependency to
install needs to be downloaded.

For example, take [PR 445][0], which requires an updated mono.
If the system mono is unsuported, the required mono hasn't already
been downloaded, and `$(AutoProvision)` is False, then the following
"helpful" error message is provided:

	error : Could not find required program 'mono'. Please run:
	installer -pkg "$HOME/android-archives/MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg" -target /

If `MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg` hasn't
already been downloaded, then the suggested command will fail.

This is not helpful. :-)

Thus, when `$(AutoProvision)` is False and URLs need to be downloaded,
then provide a message stating that:

	Please download
	`https://.../MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg`
	into `$HOME/android-archives/MonoFramework-MDK-4.8.0.489.macos10.xamarin.universal.pkg`.

[0]: #445
xmcclure and others added 6 commits March 16, 2017 19:16
The previous commit's attempts to copy the pdb/mdb files for monodoc
had a problem where they would be evaluated too early (probably before
the files existed) and in the wrong directory. To address this, the
files are instead specified as a MonoDocCopyItemOptional, a new target
_GetMonodocItems is created and the ItemGroups for
_MonoDocCopyItems/_MonoDocInstalledItems are evaluated in this target
with correct Exists logic for the Optional files. Additional
DependsOnTargets attributes were distributed through the file to
ensure this new target is evaluated at the correct time.
We are now producing .pdb files for the main assemblies in the
app. So we should not be looking for .dll.mdb files in the
apk for those files.

This commit fixes the tests to look for .pdb files.
…tion to use it.

Fixes a problem where previously-hardcoded directory paths were breaking monodroid build.
@xmcclure
Copy link
Contributor Author

build

3 similar comments
@directhex
Copy link
Contributor

build

@directhex
Copy link
Contributor

build

@directhex
Copy link
Contributor

build

@xmcclure xmcclure changed the title Update mono to 2017-02 "monthly master" branch [do not merge yet] Update mono to 2017-02 "monthly master" branch Mar 20, 2017
@xmcclure
Copy link
Contributor Author

After discussion with @jonpryor and all checks passing, merging

@xmcclure xmcclure merged commit 76be1fb into master Mar 20, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

do-not-merge PR should not be merged. enhancement Proposed change to current functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants