search

openweave / openweave-core

openWeave is a home area network application protocol stack designed to enable asynchronous, symmetric, device-to-device, device-to-mobile and device-to-cloud communications for control path and data path messaging.
Apache License 2.0
232 stars 106 forks source link

Weave Device Manager Requires 'pyobjc' But the Requirement Is Not Captured in Documentation or in `configure` #251

Open gerickson opened 5 years ago

gerickson commented 5 years ago

There is an implicit, undocumented requirement of the Python module 'pyobjc' to run the Weave Device Manager on Mac OS X. Failure to meet this requirement results in this run time error:

% src/device-manager/python/weave-device-mgr
...
Traceback (most recent call last):
  File "src/device-manager/python/weave-device-mgr", line 96, in <module>
    from WeaveCoreBluetoothMgr import CoreBluetoothManager as BleManager
  File "/Users/gerickson/Source/github.com/openweave/openweave-core/src/device-manager/python/WeaveCoreBluetoothMgr.py", line 36, in <module>
    from Foundation import *
ImportError: No module named Foundation

This requirement should be, at minimum, be noted in documentation. Ideally, it would be codified in a requirements check in configure.

jaylogue commented 5 years ago

I don't think a check in configure makes sense. That checks the system on which OpenWeave is built, but not the system on which the device manager is run. We should probably look into making the error message more helpful.

gerickson commented 5 years ago

While I do see your point about the temporal and spatial disconnect, I think it'd be better than nothing.

It also gets down to the manner in which openweave-core is most likely to be used and consumed. Is it pre-built binary distributions? If so, then the check might be useless. If it's from source (either from git or a source archive), then the check might be helpful.

Whether we have the configure check or not, I agree that the error message can and should be more useful that it is now.

jaylogue commented 5 years ago

In principle it seems wrong for configure to have a check for something that isn't used during the build process. To wit: Why should I have to install packages on my build machines that are never going to be used there? Of course, successfully running unit tests on a build machine may necessitate installing extra packages. But that's not the job of configure to enforce.

In this case I don't really care that strongly. But a clearer runtime error is the highest priority fix for this issue, as it covers all use cases.