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

[mono-2018-10] add build rules for arm64_32 watch #5439

Closed
wants to merge 6 commits into from
Closed

[mono-2018-10] add build rules for arm64_32 watch #5439

wants to merge 6 commits into from

Conversation

lewurm
Copy link
Contributor

@lewurm lewurm commented Jan 18, 2019 •
edited
Loading

NOTE1: tested with this hacky patch for xharness: https://gist.github.com/lewurm/b4243bad2b4407bdd7f056bbdbeafd5b

NOTE2: depends on mono/mono#12498 merged ✅

In terms of mono-native* the handling for the new watch is a bit different, due to watchOS 5.0 doesn't require compat/unified as far as I understand.

Verified by running mini regression suite. It's green 🙂

Context: mono/mono#10641

lewurm
lewurm commented Jan 18, 2019
View reviewed changes
builds/Makefile
$(Q) $(WATCHOS_BIN_PATH)/bitcode_strip $(WATCHOS_TARGET_SHAREDLIBLOGPROFILER) -m -o $@
$(Q) $(WATCHOS_BIN_PATH)/install_name_tool -id @rpath/libmono-profiler-log.dylib -change $(SDK_DESTDIR)/ios-targetwatch-release/lib/libmonosgen-2.0.1.dylib @rpath/libmonosgen-2.0.dylib $@
$(Q) $(WATCHOS_BIN_PATH)/bitcode_strip $(BUILD_DESTDIR)/targetwatch/lib/libmono-profiler-log.0.dylib -m -o $(BUILD_DESTDIR)/targetwatch/tmp-lib/libmono-profiler-log.0.dylib
$(Q) $(WATCHOS_BIN_PATH)/bitcode_strip $(BUILD_DESTDIR)/targetwatch64_32/lib/libmono-profiler-log.0.dylib -m -o $(BUILD_DESTDIR)/targetwatch64_32/tmp-lib/libmono-profiler-log.0.dylib
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rolfbjarne do you have a suggestion to make that a bit nicer?

@monojenkins
Copy link
Collaborator

Build failure
❌ Build was aborted

@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
✅ Generator Diff (no change)
🔥 Test run failed 🔥

Test results

9 tests failed, 0 tests skipped, 129 tests passed.

Failed tests

  • introspection/Mac Classic/Modern: BuildFailure
  • introspection/Mac Unified/Modern: BuildFailure
  • introspection/Mac Unified 32-bit/Modern: BuildFailure
  • Xtro/Mac: BuildFailure
  • introspection/iOS Unified 32-bits - simulator/Debug: TimedOut
  • mono-native-unified/iOS Unified 32-bits - simulator/Debug (static registrar): Crashed
  • mmptest/macOS/NonSystem: Failed (Execution failed with exit code 18)
  • MSBuild tests/macOS/NonSystem: Failed (Execution failed with exit code 4)
  • MTouch tests/NUnit: Failed (Execution failed with exit code 2)

@rolfbjarne
Copy link
Member

This looks good, but I'm wondering if we should keep the arm64_32 development on a different branch than mono-2018-10 so that we don't add too much stuff into mono-2018-10 (which would probably make the mono-2018-10 PR easier to complete/merge).

@chamons / @spouliot ^?

@monojenkins
Copy link
Collaborator

Build failure
❌ Build was aborted

🔥 Build failed 🔥

@monojenkins
Copy link
Collaborator

Build failure
❌ Build was aborted

@spouliot
Copy link
Contributor

IMO this work should be based on (not in) 2018-08 since we do not have any dates for 16.1 which is the earliest date where mono 2018-10 changes could be released.

Right now if we need a quick release (even a preview) to support this new architecture we would be blocking ourselves (and our customers) on completing mono 2018-10 integration first.

@luhenry
Copy link
Contributor

luhenry commented Jan 23, 2019

Following Slack discussion (https://xamarinhq.slack.com/archives/C03CFKFGL/p1548259145038500) a custom xamarin-macios branch will be created tracking a custom mono:2018-08-watchos branch, avoiding the need for 2 mono submodules. This mono:2018-08-watchos branch will follow closely 2018-08 and will contain any fixes needed for watchOS.

@lewurm lewurm added the do-not-merge Do not merge this pull request label Jan 23, 2019
@lewurm
Copy link
Contributor Author

lewurm commented Jan 23, 2019

superseded by #5465

@lewurm lewurm closed this Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rolfbjarne rolfbjarne Awaiting requested review from rolfbjarne

@chamons chamons Awaiting requested review from chamons

@emaf emaf Awaiting requested review from emaf

@jstedfast jstedfast Awaiting requested review from jstedfast

@kzu kzu Awaiting requested review from kzu

@spouliot spouliot Awaiting requested review from spouliot

@VincentDondain VincentDondain Awaiting requested review from VincentDondain

Assignees
No one assigned
Labels
do-not-merge Do not merge this pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lewurm @monojenkins @rolfbjarne @spouliot @luhenry