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

Add FreeBSD support #131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

yurivict
Copy link

No description provided.

@robbert-vdh
Copy link
Member

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
Copy link
Author

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

@wrl
Copy link
Member

wrl commented Nov 13, 2022

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
Copy link
Member

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
Copy link

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.

@BillyDM BillyDM mentioned this pull request Mar 18, 2024
27 tasks
@yurivict
Copy link
Author

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?

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

Successfully merging this pull request may close these issues.

4 participants