RustAudio / baseview

low-level window system interface for audio plugin UIs
Apache License 2.0
267 stars 57 forks source link

Add FreeBSD support #131

Open yurivict opened 1 year ago

robbert-vdh commented 1 year ago

Wouldn't it make more sense to change cfg(target_os = "linux") into cfg(and(unix, not(target_os = "macos"))) or cfg(any(target_os = "linux", target_os = "freebsd")) (since the former would also target Android and iOS) instead of duplicating entries?

yurivict commented 1 year ago

This is possibly the case, but this is a wider patch. Project maintainers should decide.

wrl commented 1 year ago

hey, thanks for the PR!

echoing robbert's sentiments, if there's a broader "linux and BSDs" or "general non-macOS UNIX-like" feature we can select for, that would be preferable. i know there aren't many (any?) folks producing on openbsd, but that doesn't mean that baseview shouldn't work there.

robbert-vdh commented 1 year ago

if there's a broader "linux and BSDs" or "general non-macOS UNIX-like" feature we can select for, that would be preferable.

Yeah that's and(unix, not(target_os = "macos")). Though that also matches iOS and Android. Would have been nice if there was a default alias for that since it's used all over the place..

ryanavella commented 11 months ago

if there's a broader "linux and BSDs" or "general non-macOS UNIX-like" feature we can select for, that would be preferable

One potential issue I see with the "subtractive" definition of UNIX-like (i.e. cfg(and(unix, not(any(..))))) is that it may not produce a compilation error for an unsupported or untested target. For example, in addition to Android and iOS mentioned above, it also includes targets like aix, illumos, solaris. And it could hypothetically include any new targets added to the "unix" family in the future.

So the benefit of an explicit, "additive" definition, is that we make it very clear which targets we test/support, and we don't mislead any users on targets that are unsupported.

yurivict commented 3 months ago

Since alternatives to the attached patch caused various concerns, maybe this PR can be merged to unblock it on FreeBSD, and let someone else could come up with further improvements in subsequent PRs?