search

JuliaOpt / CoinOptServices.jl

Julia interface to COIN-OR Optimization Services https://projects.coin-or.org/OS
Other
16 stars 4 forks source link

Fix build from source with Clang v4 #37

Closed jgoldfar closed 6 years ago

jgoldfar commented 6 years ago

(makes ordered comparison between pointer and zero an error)

jgoldfar commented 6 years ago

Not sure how this would be tested through CI; would perhaps be worthwhile to be able to select a BinDeps provider manually and test more combinations? Homebrew install was not working locally, hence this fix

tkelman commented 6 years ago

This isn't the right fix, the comparison shouldn't be on the pointer but on the dereferenced value, as was done upstream in https://github.com/coin-or/SYMPHONY/commit/907c0be7372580e849421ffda073af45daed35e5

jgoldfar commented 6 years ago

Fair enough (and evident in retrospect.) Worth updating this PR or close?

tkelman commented 6 years ago

Happy to merge if you want to update, whether it's worth it is up to you. See discussion in https://github.com/JuliaOpt/CoinOptServices.jl/pull/44

jgoldfar commented 6 years ago

PR is updated (but this code path remains untested in any automated way...)

tkelman commented 6 years ago

Suppose the homebrew provider could be put behind an env var flag so it would be possible to disable without needing code modifications, if you wanted to get fancy

jgoldfar commented 6 years ago

My comment above is mostly a dupe of https://github.com/JuliaLang/BinDeps.jl/pull/334, but I wonder what the point of such an option would be if not also tested. Not that every project even could build on e.g. Travis before timing out; there's definitely merit to having one provider that works for 99% of cases. Looking forward to BinaryBuilder making our lives easier! PS many thanks for your contributions to Julia and the surrounding ecosystem

tkelman commented 6 years ago

You would want to test things naturally, but Travis can't really do some of the more obscure cases like freebsd, and non-ubuntu distros or non-default configurations of julia would require more docker work than most people probably want to undertake.

Thanks for the kind words. I'm hardly involved much any more, hence the few month gap where I wasn't looking at things like this.