Modified code to work with universal build. #8307

Merged
merged 3 commits into from May 25, 2017

Conversation

4 participants
@AhiyaHiya

This pullrequest changes

Modified Obj-C++ code (.mm file), so that it can compile for both 32-bit and 64-bit macOS; in the code's current state, you cannot build a Universal Intel binary for macOS.

@alalek

This comment has been minimized.

Show comment
Hide comment
@alalek

alalek Mar 3, 2017

Contributor

Looks like this patch partially reverts changes from #7193.

Contributor

alalek commented Mar 3, 2017

Looks like this patch partially reverts changes from #7193.

@AhiyaHiya

This comment has been minimized.

Show comment
Hide comment
@AhiyaHiya

AhiyaHiya Mar 3, 2017

I took a look from changelist #7193 ; the new Obj-C features, at that time, allowed developers to create "@properties" without also having to create class instance variables and also removed the requirement to type "@synthesize" in the implementation file. Unfortunately, Apple did not include this feature for the 32-bit version of Obj-C, and for a reason. So, to support Intel Universal Builds, instance variables and other minor changes are needed.

AhiyaHiya commented Mar 3, 2017

I took a look from changelist #7193 ; the new Obj-C features, at that time, allowed developers to create "@properties" without also having to create class instance variables and also removed the requirement to type "@synthesize" in the implementation file. Unfortunately, Apple did not include this feature for the 32-bit version of Obj-C, and for a reason. So, to support Intel Universal Builds, instance variables and other minor changes are needed.

@AhiyaHiya

This comment has been minimized.

Show comment
Hide comment
@AhiyaHiya

AhiyaHiya Apr 11, 2017

Would it be possible to accept this pull request or was there something missing that is preventing this request from being accepted?

Would it be possible to accept this pull request or was there something missing that is preventing this request from being accepted?

@vpisarev

This comment has been minimized.

Show comment
Hide comment
@vpisarev

vpisarev May 3, 2017

Contributor

I think, it needs some further discussion. We ourselves are not using any 32-bit macOS installations anywhere. So we are waiting for more inputs from other people

Contributor

vpisarev commented May 3, 2017

I think, it needs some further discussion. We ourselves are not using any 32-bit macOS installations anywhere. So we are waiting for more inputs from other people

@vpisarev

This comment has been minimized.

Show comment
Hide comment
@vpisarev

vpisarev May 24, 2017

Contributor

@AhiyaHiya, looks like iOS build fails with the patch applied

Contributor

vpisarev commented May 24, 2017

@AhiyaHiya, looks like iOS build fails with the patch applied

@AhiyaHiya

This comment has been minimized.

Show comment
Hide comment
@AhiyaHiya

AhiyaHiya May 25, 2017

I will look into the iOS build failure.

I will look into the iOS build failure.

@AhiyaHiya

This comment has been minimized.

Show comment
Hide comment
@AhiyaHiya

AhiyaHiya May 25, 2017

So according to the message posted above, I guess all tests passed then? I built this branch on my development computer, using the python build script described in the build log, and it built without failure.

So according to the message posted above, I guess all tests passed then? I built this branch on my development computer, using the python build script described in the build log, and it built without failure.

@vpisarev

This comment has been minimized.

Show comment
Hide comment
@vpisarev

vpisarev May 25, 2017

Contributor

thank you! let's merge it in 👍

Contributor

vpisarev commented May 25, 2017

thank you! let's merge it in 👍

@vpisarev vpisarev self-assigned this May 25, 2017

@opencv-pushbot opencv-pushbot merged commit 5d03262 into opencv:master May 25, 2017

1 check passed

default Required builds passed
Details

@AhiyaHiya AhiyaHiya deleted the AhiyaHiya:dev_xcode_macos_universal_binary branch Jun 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment