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

os: add openbsd to the constraint values #8

Merged
merged 2 commits into from
Nov 8, 2019
Merged

os: add openbsd to the constraint values #8

merged 2 commits into from
Nov 8, 2019

Conversation

x1ddos
Copy link
Contributor

@x1ddos x1ddos commented Aug 9, 2019

There are notable differences from freebsd such that it warrants a constraint value.

There are notable differences from freebsd such that it warrants a constraint value.
@gregestren
Copy link
Collaborator

This sounds reasonable. I'm a bit concerned about knowing where the granularity limits are. There's a number of BSD and BSD-based OSes. What's the threshold point at which they should be distinguished through other means?

What do you think?

@x1ddos
Copy link
Contributor Author

x1ddos commented Aug 20, 2019

I'd say it depends but for what concerns a build system like bazel, this probably lies in the convenience and readability. Plus, you already have freebsd defined.

For instance, OpenBSD porting guide does not recomend using constructs like #if defined(__NetBSD__) || defined(__FreeBSD__) and instead rely on the value of BSD.

I proposed this change mainly because I wanted to split parts of a build target depedencies and use pledge + unveil on OpenBSD vs other platforms.

It is of course all possible to do in one source file using #if defined. But then I'd rename freebsd to bsd for all BSD-based OSes, to make it equal.

All of the above is of course within C/C++ world. Other build targets might be more difficult to implement without matching a specific OS value. But then again, one can always run a shell script and check some environment variables and branch based on that. Less convenient and readable though imho.

Also, looking at the existing os values, you already have ios, osx, tvos and watchos. I guess it also makes it easier for integrations with various IDEs. But still, there would be a similar question: why not merge them all and call it apple or darwin. Could still work, in theory, but will probably be very inconvenient.

Another answer might be to look at what other tools are doing. For instance, Go has a list of supported platforms from which I can get a list of operating system families:

$ go version
go version go1.12.6 linux/amd64

$ go tool dist list | cut -d'/' -f1 | sort -u
aix
android
darwin
dragonfly
freebsd
js
linux
nacl
netbsd
openbsd
plan9
solaris
windows

@hlopko
Copy link
Member

hlopko commented Nov 7, 2019

IMHO adding openbsd won't cause harm. We already have freebsd, and if we want to change freebsd to be a part of a different, more granular setting, we have to be very careful because of backwards compatibility. So let's add openbsd as os, and if in the future we have to change them, let's change them both, it will still be just one breaking change.

Oh I see you already approved Greg, so I'll merge it now. Please complain to me if you disagree :)

bazel-io pushed a commit that referenced this pull request Nov 8, 2019
PiperOrigin-RevId: 279293049
Change-Id: I475bad63cb23f87067acd3d5e77d26759599da93
@bazel-io bazel-io merged commit 63bcda3 into bazelbuild:master Nov 8, 2019
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.

5 participants