Bug 94494

I'll make some PRs to homebrew or another OSX package manager to manually patch around the issues given certain OS versions, but it seems kind of odd to fix these at the distributor rather than in the source.

More authoritative info about the point at which ServiceIPC was deprecated: https://lists.macosforge.org/pipermail/launchd-dev/2009-June/000570.html

That commenter at least has an apple.com email address, which lends a bit of credibility :p

(In reply to zbbentley+accounts from comment #0)
> - The "OnDemand" section should only be present on OSX versions <= 10.4; it
> should be ommitted in subsequent versions.

Please assume that none of the D-Bus maintainers know anything about OS X; that's either true or a good first approximation. I think I last used 10.2 myself, and my only Apple hardware is a PowerPC. If you would like to be the new maintainer of D-Bus-on-OS-X, we'd appreciate the help.

How old are we talking about here? Does Apple still provide security support for 10.4? Our general policy <https://lists.freedesktop.org/archives/dbus/2013-October/015825.html> is that we'll try to support anything that still gets security patches from its vendor, but when the vendor abandons it, so do we. From <https://en.wikipedia.org/wiki/Mac_OS_X_Tiger> it sounds as though this policy would have led us to drop 10.4 support 5 years ago, if there had been someone who knows OS X among the maintainers.

(In reply to zbbentley+accounts from comment #0)
> Two fixes should be made

Please propose a patch that makes exactly the change you want us to do, and I'll be happy to review it. If I try to make changes based on your textual description, I'll probably get something subtly wrong.

(In reply to zbbentley+accounts from comment #1)
> I'll make some PRs to homebrew or another OSX package manager to manually
> patch around the issues given certain OS versions, but it seems kind of odd
> to fix these at the distributor rather than in the source.

Yes, please prefer to contribute patches here first, and backport them to the versions in package managers later.

Things that are wrong in D-Bus on non-Linux are most likely to be because we either don't know there's a problem, or are aware of a problem but have not received a suitable change to solve it.

To be clear, I'm suggesting that if versions < 10.5 are something we should no longer care about, then the templating you alluded to might not be necessary, and we could just hard-code "the right answer" for 10.5+. However, if you want to contribute a patch that detects <10.5 or >=10.5 and behaves accordingly, and it isn't disproportionately large, that would be fine too.

Thanks for the info! I'll see how complicated it would be to add templating based on OS version, but I suspect I'll fall back to your suggested alternative and just submit a patch that removes the legacy cruft altogether.

Created attachment 122242 [details] [review]
Proposed patch

Adds OS version detection for Linux and OSX. Uses the detected version to include or exclude template info from the launchd plist.

The template is a single line of XML that gets included, which is ugly, but the standard solution to make autoconf generate multiline makefile variables is even uglier.

There are almost certainly stupid things in here; I am an autoconf novice.

Created attachment 122243 [details] [review]
Fixed patch

Apparently my git hooks utterly mangled the whitespace in the first patch I submitted. This one contains no spurious changes to whitespace.

That'll teach me to trust git-diff-tools instead of actually reviewing patches. Sorry for the spam!

Comment on attachment 122243 [details] [review]
Fixed patch

Review of attachment 122243 [details] [review]:
-----------------------------------------------------------------

Is there a strong reason why we would need to support OS X 10.4 or older, even though they have gone out of vendor (Apple) security support and so in practice are already likely to be unsafe to use?

I don't understand why you're going to this effort for an unsupported version. If the current configuration is wrong for all security-supported OS X versions, and you have a change that is valid for all of those versions, we should probably just make the change and move on.

I don't think detecting the version at compile time is correct: what matters is the version where we will *run* dbus-daemon, and if binaries are copied from one machine to another, that won't necessarily match. Or do all users of open source on OS X build it from source locally, in practice?

::: configure.ac
@@ +1502,5 @@
>  
> +OS_FAMILY=$host_os
> +OS_TYPE=unknown
> +OS_VERSION=unknown
> +case $OS_FAMILY in

Probably better to use $host_os directly.

Prefer to use AS_CASE and AS_IF instead of using case/esac and if/fi directly:

AS_CASE([$host_os],
  [darwin*],
    [
      echo Do Darwin things here
    ],
  [linux*],
    [
      ...
    ])

(But in practice we probably don't even want to know the version on anything except Darwin.)

@@ +1507,5 @@
> +  darwin*)
> +    # The sw_vers utility on OSX can be used to get release information:
> +    AC_PATH_PROG([SW_VERS], [sw_vers])
> +    if test -n $sw_vers ; then
> +      OS_TYPE=$($SW_VERS | $SED -n 's/^ProductName:[[:space:]]*//p')

What is this? Server vs. workstation or something?

If we don't need to know (which I'm pretty sure we don't), then please don't capture it.

@@ +1510,5 @@
> +    if test -n $sw_vers ; then
> +      OS_TYPE=$($SW_VERS | $SED -n 's/^ProductName:[[:space:]]*//p')
> +      OS_VERSION=$($SW_VERS | $SED -n 's/^ProductVersion:[[:space:]]*//p')
> +    fi
> +    AS_VERSION_COMPARE($OS_VERSION,"10.11.0",[DBUS_PLIST_SUBST=$(echo "<key>ServiceIPC</key><true/><key>OnDemand</key><false/>")])

Why is this comparison looking at 10.11.0 when you've said the relevant "threshold" version is 10.5?

@@ +1522,5 @@
> +    # os-release is an attempt at canonicalization. See: https://www.freedesktop.org/software/systemd/man/os-release.html
> +    if test -f /etc/os-release ; then
> +      OS_VERSION=$($SED -n 's/^VERSION_ID="\(.*\)"[[:space:]]*$/\1/p' /etc/os-release)
> +    elif test -f /usr/lib/os-release ; then
> +      OS_VERSION=$($SED -n 's/^VERSION_ID="\(.*\)"[[:space:]]*$/\1/p' /usr/lib/os-release)

I'm pretty sure this is a wrong generalization. Please don't try to capture the OS version in a platform-independent way. If we only need to know on OS X, only capture it on OS X.

A version number is entirely meaningless without the context of the OS itself. Knowing the OS version is 9 might mean that it's horribly ancient (Mac OS, Fedora) or that it's a version that, as of 2016, hasn't even been released yet (Debian) or anything in between. Capturing OSX_VERSION on OS X would make more sense.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/146.