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.

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

Product Module name has changed since 4.5.0 - bug or feature? #259

Closed
KoCMoHaBTa opened this issue Jan 17, 2019 · 9 comments
Closed

Product Module name has changed since 4.5.0 - bug or feature? #259

KoCMoHaBTa opened this issue Jan 17, 2019 · 9 comments

Comments

@KoCMoHaBTa
Copy link
Contributor

Hi,
Since release 4.5.0 - the product module name has changed from NVActivityIndicatorView to NVActivityIndicatorView_iOS.
Not a big deal, but just wondering if this was intentional.
I see that tvOS support was added since that version and the product name was changed - that makes me assume that the module name change was not intentional.

Issues that might occur due to different module name between frameworks - if you share source files between iOS and tvOS - you will not be able to import neither module, unless using conditional compilation.

Also the current guide in the README is obsolete, as the example there is using the old module name.

screen shot 2019-01-17 at 16 44 08

@KoCMoHaBTa
Copy link
Contributor Author

I would recommend to override the module name on all frameworks and set it to be the common one - NVActivityIndicatorView

@KoCMoHaBTa
Copy link
Contributor Author

This also affects interface builder files that are using NVActivityIndicatorView
They are crashing during runtime with the following exception:

Unknown class _TtC23NVActivityIndicatorView23NVActivityIndicatorView in Interface Builder file.
*** -[NSMutableArray addObjectsFromArray:]: array argument is not an NSArray
*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '*** -[NSMutableArray addObjectsFromArray:]: array argument is not an NSArray'
*** First throw call stack:
(0x1c2150ec4 0x1c1321a40 0x1c20c8494 0x1c2046554 0x1ef7088bc 0x1ef7098e8 0x1ef709cb8 0x1c2af230c 0x1ef709bbc 0x1ef709944 0x1ef709cb8 0x1c2af230c 0x1ef709bbc 0x1ef709944 0x1ef708b2c 0x1ef6fbb64 0x1ef7be478 0x1ef7be658 0x1ef7b1844 0x1ef7b170c 0x1ef7c1450 0x1ef782034 0x1eecc4b1c 0x1eecc50f8 0x1eecc63f8 0x1eeca85e8 0x1ef7c77dc 0x1c674db74 0x1c6752b2c 0x1c66b144c 0x1c66dfd7c 0x1c66e0be4 0x1c20e07cc 0x1c20db460 0x1c20dba00 0x1c20db1f0 0x1c4354584 0x1ef31ed40 0x102b72294 0x1c1b9abb4)
libc++abi.dylib: terminating with uncaught exception of type NSException

screen shot 2019-01-17 at 16 53 53

@ninjaprox
Copy link
Owner

ninjaprox commented Jan 17, 2019

Thanks for the issue. It was intentional due to #252 and #248 which changed the product names, as a result the module name changed accordingly. If you're using Carthage, Usage has been updated to address this change, and it shouldn't affect CocoaPods users.

CMIIW, the reason of having different module names (are product names) for each target is due to the way Carthage build the original project with all defined schemes and targets. If all targets have the same product name, how do we differentiate which target to use? It doesn't happen with CocoaPods though. Because CocoaPods just uses code files specified in Podspec, then create its own project with configs in Podspec, so it'll produce the same module name of the pod regarless of the platform.

@KoCMoHaBTa
Copy link
Contributor Author

Product name is not used to differentiate targets - the schemes are used for that.
The product name is something that target generates during compilation.
If you set the same product name to all targets - it should be fine and Carthage will build them correctly. This way - there will be no need to have different imports.

I've forked and played a little with the project and can confirm that #251 is not present.
However, i found something weird - carthage messes up the iOS and the app extension targets.
Probably this is what you mean by how to differentiate the targets.
It builds them both, but since the produced name of the framework is the same - the last one wins.
Alternatively, only the app extension target could have different product name.
I haven't used app extensions so far, but what i can see is that the difference is 1 checkbox and 2 classes. I would be glad if you could share what's the deal with that?

Off topic regarding the build tools:
Cocoapods, as usual, is always messing up with the project settings and dishonor them, so i would assume that it could override it with the name of the library set in the podspec or some other place.
That's because it is generating the Pods project by its own rules and does not use what's defined in the framework's xcode project. I've seen libraries that are resolved trough cocoapods without even having xcode porject. :)

Carthage is using whatever is defined in the build settings of the project, as it uses xcodebuild under the hood.

@ninjaprox
Copy link
Owner

I've forked and played a little with the project and can confirm that #251 is not present.

You meant you managed to use import NVActivityIndicatorView even before the fix #252?

The purpose of the app extension target is to allow to use the library in an app extension which UIApplication.shared.keyWindow and UIApplication.shared.windows are not available (basically the target just excludes the entire Presenter folder from source codes).

Anyway, let me play a bit more, then find the best way for both worlds.

@KoCMoHaBTa
Copy link
Contributor Author

KoCMoHaBTa commented Jan 18, 2019

You meant you managed to use import NVActivityIndicatorView even before the fix #252?

I meant that i managed to use import NVActivityIndicatorView after i've set the same product name on all targets -> https://github.com/KoCMoHaBTa/NVActivityIndicatorView/commit/1e84bd1d3d60d5ccb4882d18d1009a10c9e8d3e5

@ninjaprox
Copy link
Owner

I tried out the change, and it worked perfectly. Thanks for pointing it out and correcting my misunderstanding.

In addition to unifying product names of NVActivityIndicatorView-iOS and NVActivityIndicatorView-tvOS to NVActivityIndicatorView, the product name of NVActivityIndicatorViewAppExtension-iOS will be set to NVActivityIndicatorViewAppExtension to resolve the conflict you mentioned earlier. Do you want to make a PR for this as I see you already have it?

@KoCMoHaBTa
Copy link
Contributor Author

PR created - #260
Please check if all works well with cocoapods

@ninjaprox
Copy link
Owner

4.6.0 has been shipped included the PR. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants