[msbuild] Fixed IsWatchExtension state property #913
Conversation
The problem was that this property was evaluating to True for the main app bundle because it assumed that if the project contained a Watch app, that it was the WatchExtension. This logic only holds true for WatchOS1, but not WatchOS2. Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=44841
|
Build failure |
|
That sounds like a check that should be part of the msbuild tests. We don't want to find out, way too late, we have a known-to-be-invalid (for submission) flag in our bundles. @VincentDondain could you also review the PR ? thanks! |
|
I'm working on expanding the ValidateAppBundleTask as we speak (for both WatchOS1 and WatchOS2 stuff). @spouliot Should we merge this PR now and have me submit a new PR for the Validate changes when I'm done with that? Or just commit my Validate changes to this PR and wait to merge it until that's done (and reviewed) as well? |
|
@jstedfast I'd rather have it merged together if that can be done prior to the C9 branch. |
|
@spouliot @VincentDondain okay, I've committed a bunch of new validation tests for WatchOS2 apps/extensions and a check to make sure WatchOS1 App Extensions have a watch-companion UIRequiredDeviceCapability |
|
Build failure |
|
I'm unable to reproduce the unit test failure locally and the error is that multiple Watch Apps are being included in the main app which makes no sense unless Mono's Directory.EnumerateDirectories() is broken on the build bot. |
|
build |
|
Build failure |
|
build |
|
Build failure |
Fixes unit tests
| if (count > 0) | ||
| Log.LogError ("The Watch App '{0}' contains more than 1 Watch Extension.", name); | ||
| else if (count == 0) | ||
| if (count == 0) |
There was a problem hiding this comment.
Does Xcode allow multiple watchkit extensions per app?
There was a problem hiding this comment.
I don't know how to even go about testing that, but in the interest of getting our unit tests to pass (this check in ValidateTask was added by me as part of this patch) so that we can merge it into master, I figured it was worth relaxing the Validate task for.
There was a problem hiding this comment.
OK, if you're just removing tests that you added in this PR it's fine.
|
Build success |
The problem was that this property was evaluating to True for the main app bundle because it assumed that if the project contained a Watch app, that it was the WatchExtension. This logic only holds true for WatchOS1, but not WatchOS2. Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=44841
The problem was that this property was evaluating to True for the main app bundle because it assumed that if the project contained a Watch app, that it was the WatchExtension. This logic only holds true for WatchOS1, but not WatchOS2. Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=44841
The problem was that this property was evaluating to True for the main app bundle because it assumed that if the project contained a Watch app, that it was the WatchExtension. This logic only holds true for WatchOS1, but not WatchOS2. Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=44841 Also fixes https://bugzilla.xamarin.com/show_bug.cgi?id=46266 because this fix wasn't backported to cycle8.
The problem was that this property was evaluating to True for the main app bundle because it assumed that if the project contained a Watch app, that it was the WatchExtension. This logic only holds true for WatchOS1, but not WatchOS2. Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=44841 Also fixes https://bugzilla.xamarin.com/show_bug.cgi?id=46266 because this fix wasn't backported to cycle8.
6 participants
The problem was that this property was evaluating to True
for the main app bundle because it assumed that if the
project contained a Watch app, that it was the WatchExtension.
This logic only holds true for WatchOS1, but not WatchOS2.
Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=44841