ladybug-tools / ladybug-legacy

:beetle: Ladybug is an environmental plugin for Grasshopper.
http://ladybug.tools
Other
194 stars 82 forks source link

Address missing Tls12 protocol preventing ladybug flying on MacOS #443

Closed philipbelesky closed 6 years ago

philipbelesky commented 6 years ago

Apologies, I couldn't find any where in the documentation that lists whether MacOS is officially supported on the legacy plugin. If that's not the case feel free to reject this PR.

This issue is with the legacy version of the plugin LB 0.0.66 and HB 0.0.63 downloaded from Food4Rhino. Rhino 5.4.1 for Mac and GH 0.9.0080. I've reproduced it on several similar setups.

Placing the ladybug_ladybug component on a blank canvas yields an exception "'type' object has no attribute Tls12" on MacOS. This is due to line 63 of the source:

System.Net.ServicePointManager.SecurityProtocol = System.Net.SecurityProtocolType.Tls12

...because TLS 1.2 isn't available to .NET Core on MacOS. As a result of the exception ladybug does not fly. However catching the exception and reverting to importing TLS 1.0 will allow the component to execute normally.

Thanks again Mostapha and Chris for your work in maintaining and developing ladybug tools as OSS!

chriswmackey commented 6 years ago

@philipbelesky , Thanks for reporting this and I was not aware that Tls12 is not yet implemented in Mac OSS. Technically we don't offer full support for Mac OSS on the legacy version but I know that there are a number of ladybug features that still work over there and we shouldn't limit what people can do if we don't have to. I agree that we should merge this change in and I'll add the same try/except loop to the other places where we use the new protocol.

BTW, the reason why the new protocol was implemented is that github updated to use Tls12 and so we could not automatically download certain files in the background that get used by the components. So, if you are going to use Ladybug legacy on Mac, you'll like have to download some of these files manually or just not use the features that rely on this ability to download.

philipbelesky commented 6 years ago

@chriswmackey Thanks for the merge, and I totally get that cross-platform development for a plugin that is in large part underpinned by external dependencies and integrations is a very difficult endeavour.

One quick suggestion on that point would be that it might be worth making it more explicit on the food4rhino page about the nature of MacOS support. This is just my experience, but it seems that students the see the Apple icon on that page and assume that full support is offered for MacOS. Also speaking from experience because a lot of documentation specifies to use Ladybug or Ladybug+Honeybee people then overlook the Honeybee[+] download link in favour of the legacy download links because they assume the '+' version is missing Ladybug. Particularly as Mac installations of Grasshopper continue to grow it might be helpful to clearly direct those users to the '+' version, to the GhPython dependency, and to set expectations about which parts may or may not work.

mostaphaRoudsari commented 6 years ago

Hi @philipbelesky, fair comments. I'm not sure if we can do this on Food4Rhino page. Once you pick support for mac for one of the plugins it turns the mac support on. I think this happens because they have designed the page for a single plugin not a set of tools as we use it like now.

Also @chriswmackey, this PR is missing the userobject.

chriswmackey commented 6 years ago

No worries @mostaphaRoudsari . I took care of all the userojbects here: https://github.com/mostaphaRoudsari/ladybug/commit/eda83af8f880abd1aab03c14551bb8b358ab6dfa

There are a number of components that have that call

philipbelesky commented 6 years ago

Apologies for missing the user object!

Agree that food4rhino has some more structural issues here that make it difficult to indicate compatibility more precisely. I was more imagining a quick piece of text adding to the description pointing people coming from Mac in the right direction (towards the '+' version I assume) or at least warning them about partial support.

If I have the time in the future, and you were amenable to it, I could go through the legacy/+ code and try to add explicit warning remarks for components that won't function in MacOS.

mostaphaRoudsari commented 6 years ago

Hi @philipbelesky, no need to apologies. Thank you for figuring this out and fixing it.

I personally think if we do it as it goes it should not be that hard to get the legacy version to work for Mac. None of the core developers are MAC users otherwise this should have been already addressed by making changes to the broken components. @yafimski reported the issues a couple of times but he couldn't report the exact issue as you did and I have been unable to support as I already have too much on my plate!

If you can help with getting these fixes sorted out or point us to the exact issue so we can fix it we might be able to get this all figured out at some point.