bazelbuild / platforms

Constraint values for specifying platforms and toolchains
Apache License 2.0
108 stars 71 forks source link

os: add openbsd to the constraint values #8

Closed x1ddos closed 4 years ago

x1ddos commented 5 years ago

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

gregestren commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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 :)