[pulseaudio-discuss] [PATCH] Don't let user-set PULSE_RUNTIME_PATH values affect behaviour

[David Henningsson]

On 2014-10-19 22:39, Glenn Golden wrote:
> Tanu Kaskinen <tanu.kaskinen at linux.intel.com> [2014-10-06 13:58:09 +0300]:
>> On Mon, 2014-10-06 at 12:46 +0200, David Henningsson wrote:
>>>
>>> On 2014-10-06 12:41, Tanu Kaskinen wrote:
>>>> On Sat, 2014-10-04 at 11:13 -0600, Glenn Golden wrote:
>>>>> David Henningsson <david.henningsson at canonical.com> [2014-10-02 11:29:50 +0200]:
>>>>>>
>>>>>>
>>>>>> On 2014-10-02 11:17, Tanu Kaskinen wrote:
>>>>>>> On Mon, 2014-09-29 at 13:50 +0200, David Henningsson wrote:
>>>>>>>>
>>>>>>>> On 2014-09-28 11:23, Tanu Kaskinen wrote:
>>>>>>>>> The logic for choosing the runtime directory is complicated enough
>>>>>>>>> also without adding PULSE_RUNTIME_PATH into the mix. XDG_RUNTIME_DIR
>>>>>>>>> is sufficient for users to control the runtime directory.
>>>>>>>>> PULSE_RUNTIME_PATH has not been documented, so this change doesn't
>>>>>>>>> constitute an interface break.
>>>>>>>>
>>>>>>>> A quick googling of PULSE_RUNTIME_PATH seems to indicate usage of this
>>>>>>>> environment variable in at least chromium and enlightenment, and also
>>>>>>>> recommended in several blog posts and mailing lists, including this one.
>>>>>>>> It is likely used in several home-made scripts.
>>>>>>>>
>>>>>>>> I'm hesitant to remove it for that reason.
>>>>>>>
>>>>>>> The argument that "if you use undocumented interfaces, you can only
>>>>>>> blame yourself if your script breaks" probably won't change your mind,
>>>>>>> so I guess we'll just have to make this a documented interface then.
>>>>>>
>>>>>> Well, while not officially documented, we have still advocated the use of it
>>>>>> on this mailing list [1], which to some degree could be seen as the de-facto
>>>>>> documentation of PULSE_RUNTIME_PATH, given the lack of official
>>>>>> documentation saying otherwise.
>>>>>>
>>>>>
>>>>> 2c suggestion from the albatross-avoidance dept: How about adding it to the
>>>>> official doc, but as an explcitly deprecated feature (and with an explicit
>>>>> associated date/version beyond which it will not be supported)?
>>>>
>>>> I'd be ok with that. PulseAudio should then also print warnings when it
>>>> notices that PULSE_RUNTIME_PATH has been set. David, what do you think?
>>>> I volunteer to write the patch that prints those warnings.
>>>
>>> What's the reason to remove it in the first place? Does it cause any
>>> significant problem, or is it just to have one environment variable less
>>> to care about? If it is the latter, not sure if it's worth the effort (i
>>> e, other people's effort) to remove it.
>>
>> It's only about having one less environment variable to care about.
>>
>>> Btw; as for XDG_RUNTIME_DIR vs PULSE_RUNTIME_PATH, there might be a use
>>> case for both, if we want PULSE_RUNTIME_PATH to be auto-created if the
>>> dir does not exist, which we don't want for XDG_RUNTIME_DIR.
>>
>> We don't have a use case for PULSE_RUNTIME_PATH. Other people may have a
>> use case for it, but we don't know whether they want the directory to be
>> created or not (we have certainly never promised them any particular
>> behaviour).
>>
>> If you don't think it's a good idea to deprecate the variable, then
>> let's not do that.
>>
>
> Based on the above discussiion, since it seemed to have been decided not to
> deprecate PULSE_RUNTIME_PATH, I tried a few experiments to see what I could
> learn about its [undocumented] symtax/semantics, in order to document it
> properly in my writeup.  I was a bit surprised at what I found:
>
> Given the name of the envar (ending in "...PATH") I mistakenly assumed that
> it would be treated as a PATH-like variable, i.e. a list of comma-delmited
> paths to putative runtime dirs which would be tried in sequence.  So I tried
> this experiment:
>
>      $ unset XDG_RUNTIME_DIR		# Just to be safe
>      $ unset XDG_CONFIG_HOME		# Just to be safe
>      $ export PULSE_RUNTIME_PATH='/tmp/a:/tmp/b:/tmp/c'
>      $ rm -fr /tmp/a  /tmp/b
>      $ mkdir /tmp/c
>      $ [kill any running pulseaudio processes by hand]
>      $ pulseaudio -v --start
>
> My expectation was that the runtime directory would be created in /tmp/c.  But
> instead, it was created in a directory named '/tmp/a:/tmp/b:/tmp/c'.  I.e. it
> evidently treated $PULSE_RUNTIME_PATH not as a list of paths to be tried in
> sequence, but as the name of a directory, and which is evidently to be created
> [if possible] even if non-existent at the time the PA server is started.
>
> Thinking that perhaps I'd made an unwarranted assumption about the delimiter
> character being colon, I tried the same experiment as above except using a
> single space as the 'delimiter'.  Same thing: The runtime dir was again created
> in a directory whose name was precisely the contents of PULSE_RUNTIME_PATH,
> i.e.  the directory was named  '/tmp/a /tmp/b /tmp/c'.
>
> Is it really intended to work this way or is this a bug?  If it is intended
> to work as it does, then imo, the name choice for the envar is very misleading,
> given the defacto convention that envars named as "....PATH" behave as PATH-
> like variables (e.g. CDPATH, MANPATH, MAILPATH, INFOPATH...).

Given your findings, how about this: let's say that PULSE_RUNTIME_PATH 
is deprecated in the documentation (and refer to XDG_RUNTIME_DIR as a 
preferred solution), and also document the above shortcoming / 
unexpected behaviour? I e, if it's deprecated, we don't have to fix it. :-)

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic