user-configuration-directories suboptimal

Bug #1437480 reported by Jason Miller
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ASDF
Fix Released
Undecided
Unassigned

Bug Description

E-mail and response from Robert:

Jason Miller wrote:
> Hi,
>
> With $XDG_CONFIG_DIRS unset, (uiop:user-configuration-directories)
> returns only $XDG_CONFIG_HOME/common-lisp/
>
> However, with it set to "/etc/xdg" it returns a list that starts with
> "/etc/xdg/common-lisp"
>
> There are two problems with this:
>
> 1) The XDG Base Directory Specification says that "If $XDG_CONFIG_DIRS is
> either not set or empty, a value equal to /etc/xdg should be used."
>
> 2) My understanding of uiop:user-configuration-directories is that it's
> listed in order of importance, but, from the XDG spec:
>
> "The base directory defined by $XDG_CONFIG_HOME is considered more
> important than any of the base directories defined by
> $XDG_CONFIG_DIRS"
>
> So, what I think is correct is that with $XDG_CONFIG_DIRS set it should
> return a list starting with $XDG_CONFIG_HOME, followed by the lists in
> $XDG_CONFIG_DIRS, and with it not set, should return a list of
> $XDG_CONFIG_HOME followed by /etc/xdg/common-lisp/
>
> Even if that's not correct, due to #1 the current implementation is
> definitely wrong.
>
> Regards,
> Jason
>

1. Please put a ticket to this effect on the launchpad bugs page,
https://launchpad.net/asdf.

2. I am not sure that this is a bug. The XDG specifies how a general
framework for user configuration should operate. But AFAICT
UIOP:USER-CONFIGURATION-DIRECTORIES doesn't claim to return something
specified by this standard, but rather, claims to return a list of user
configuration directories *FOR COMMON LISP* that somehow comply with the
specification. I.e., adding "common-lisp/" is acceptable.

I'm not sure about this since the documentation string for the UIOP
function does not clearly state what it claims to be supplying.
"Determine user configuration directories" is not specific enough to
support claims about whether the implementation is correct or buggy.

3. If we can determine that the implementation is erroneous, I would
welcome patches that would fix it. I do not plan to try to fix it
myself. I don't use this means of configuration, and don't have the time
to investigate it, or the specification that underlies it. I still use
ASDF:*CENTRAL-REGISTRY*, since it permits me to configure ASDF entirely
in common lisp without using a DSL or wandering off into configuration
files which the shell will hide from me when I am confused about my
configuration.

Best,
r

Revision history for this message
Jason Miller (aidenn0) wrote :

After that e-mail, I dug a bit and found a few things:

1) Other parts of ASDF that claim to implement the XDG standard do use this function

2) A few other people do use (in-user-configuration-directory) to generate their own configuration files; this will break on e.g. Gentoo linux which sets XDG_CONFIG_DIRS but not XDG_CONFIG_HOME; this is how I originally stumbled upon this

3) We actually do prioritize XDG_CONFIG_HOME over XDG_CONFIG_DIRS when both are set, but we don't ever provide a fallback value for either of them; we manually add in $HOME/.config/common-lisp at the end.

The fix is fairly simple: just insert the default values if either of the two environment variables we query isn't set. I've attached a patch that does it one way. Another way that might be preferrable would be to extend getenvp to take a default value.

Revision history for this message
Faré (fahree) wrote :

Please consider this patch, that tries to fix XDG paths on unix, and play nice with windows.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

OK. I will merge this, but aside from code review, I'm not sure how I am to tell whether it works or not.

Is this the specification in question: http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html ?

I just looked at this specification: it is not at all impressive. It's riddled with typographical errors; it uses ill-defined terms like "more important" (why use this instead of specifying how shadowing works?); bakes in over-specific information ("permission 0700" instead of a definition of what 0700 is intended to achieve so that the spec can be extended to arbitrary operating systems); etc.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

Did we pick up some other patches in this modification?

It looks like there are modifications to bundle.lisp which should probably be part of a separate commit, and similarly a bunch of changes that just came in to replace arbitrary constructs with use of OS-COND.

Are those all part of fixing this bug, or should I peel them out separately?

Revision history for this message
Faré (fahree) wrote :

I needed to introduce cases for windows everywhere, but on abcl, these have to be runtime tests, whereas on all other implementations they can be compile-time tests. I initially used a runtime test everywhere, but sbcl is clever enough to optimize a runtime test away and issue annoying warning about unreachable code. So I introduced os-cond, and used it everywhere there was a call to os--windows-p.

I could try to divide the patch in two, one that introduces os-cond, and one that uses it in fixing the search paths, if you insist.

My main issue with my patch is that the refactoring just eliminates a few functions that don't make sense anymore and can't be backward-compatibly implemented on Windows:

user-configuration-directories
system-configuration-directories
in-first-directory
in-user-configuration-directory
in-system-configuration-directory

Let me send the patch split in two...

Revision history for this message
Faré (fahree) wrote :

Patch 1: introduce os-cond

Revision history for this message
Faré (fahree) wrote :

Patch 2: fix the configuration

Revision history for this message
Faré (fahree) wrote :

Jason, you say that some people are using in-user-configuration-directory --- who is it? We need to find a way to warn them and/or introduce costly backward-compatible transition...

Revision history for this message
Jason Miller (aidenn0) wrote :

Faré: I just know that PJB recommended in-user-configuration-director to someone on #lisp, who was running Gentoo (which configures the environment in the way mentioned above) and ran into this problem.

Revision history for this message
Faré (fahree) wrote :

Oh well, we may have to provide backward compatility functions. Or we might decide the functionality is too obscure and too unportable to deserve support, and just tell people to move to the newer, better interface.

Revision history for this message
Faré (fahree) wrote :

New version of the fix, that still depends on the first patch for os-cond.

This variant includes backward compatibility with the previous not-so-portable API, and passes all tests on CCL and ABCL on Linux.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

Given the following characterization of the patch:

My main issue with my patch is that the refactoring just eliminates a few functions that don't make sense anymore and can't be backward-compatibly implemented on Windows:

user-configuration-directories
system-configuration-directories
in-first-directory
in-user-configuration-directory
in-system-configuration-directory

Let me send the patch split in two...

-------------------------------------

I'm afraid I'm going to have to postpone incorporating the patch. I will be busy all next week, so I am not able to push a version of ASDF that potentially removes useful -- and used! -- functions. I shouldn't be doing this when I can't respond to people who have problems.

AFAICT we have gotten ourselves in trouble by importing a Unix-family-specific specification -- XDG -- into ASDF, which is not supposed to be Unix-family-specific.

Let me ask some follow-up questions:

1. Am I correct in thinking that the modifications are confined to the uiop/configuration facility, but that they have ramifications for ASDF's use of this facility for configuration?
2. I thought the issue was simply that we didn't apply the defaults in the order envisaged by the specification. If that's the case, how do we lose functionality in the patch?
3. Do we have a specification (informal is fine) for what the configuration facility is intended to do?
4. Related to the above, are we implementing some external specification when we try to map XDG onto Windows, and if not, can we state what we are trying to do when porting XDG to Windows?
5. If we are porting XDG to Windows, what's the justification for mapping the directory names over onto Mac OS X? I.e., why are we using the Unix names instead of those kooky Mac Library/ pathnames? I'm not saying we should, but if we aren't trying to replicate the Mac-specific directory structure, why should we be trying to be Windows-specific? That would lead to a PoV where we say "this will work for you if you are willing to pretend to have a Unix machine, and if not, you're on your own," which seems much easier to implement. After all, it's not as if we require users to employ the XDG files: one can always put something into a lisp init file.

What I'm looking for is a justifiable way to have a minimal-perturbation patch, instead of a substantial redesign.

Revision history for this message
Faré (fahree) wrote :

The latest version of the patch restores these functions, leaving the symbols in uiop/configuration, but implementing them in uiop/backward-driver in terms of the new functions, marking them deprecated.

Here's a slightly improved version.

The XDG specification makes total sense on Unix, is used on Unix, and is exactly what the doctor ordered in this case.
On Windows, more or less the same concepts exist, except in a more sucky way, and any further configuration is I suppose meant to happen in the Windows registry, that we can't portably support in ASDF. Happily, Windows provides environment variables that are a sufficient substitute in the simple cases used by ASDF.

Actually, I hesitated about making the Windows support slightly more Unixy, by having a shared cache/ directory under local_appdata and a subdirectory for each application, instead of a cache/ subdirectory for each application directory under local_appdata. It is so much easier for both users and programs to deal with. But for the sake of compatibility, I didn't. Which is unchanged since ASDF2.

1- the ramifications for ASDF is that the intended behavior is fixed in UIOP. Which means that the search order between ~/.config/ and $XDG_CONFIG_DIRS when XDG_CONFIG_HOME is not defined.

2- the patch implements some more general primitives for accessing configuration data, that work on both Unix and Windows, and not just for common-lisp, but arbitrary applications, i.e. the new primitives are not specialized for the string "common-lisp" anymore. You could use them for e.g. "Google/Chrome" or "Mozilla/Firefox". And as noted above, the functionality is not lost anymore.

3- The informal specification is to allow to search for configuration data in the expected place that the user, his local administrator, the application distributor, or the system distributor, may have placed it — following the conventions of the underlying platform, and allowing for users to override everything with suitable configuration variables. In the case of Unix, there's a standard, XDG, which isn't that great but infinitely better than the void before it. In the case of Windows, there is a truly ugly mess with some information conventions. I haven't tried to seriously address MacOS X, treating it as Unix for now.

4- Yes, I'm trying to follow for common-lisp configuration and cache conventions similar to those of other modern applications, such as Google Chrome, or whatever.

5- We're not actually porting XDG to Windows, but we are providing an interface that abstracts over both XDG and whatever Windows conventions there are. I don't know enough about MacOS conventions, and the little I know (~/Library/Preferences/net.common-lisp.asdf.source-registry.plist) seem to be as alien to ASDF as trying to use the Windows registry would be.

The patch is not that big, and manages to abstract over the differences between Windows and Unix.
It does make ASDF ~60 lines bigger in the end though (over half of which for backward compatibility).

Revision history for this message
Faré (fahree) wrote :

Same patch, cleaned up more.

Revision history for this message
Faré (fahree) wrote :

Same patch, after merge and tweaks.

It stands alone, since the os-cond patch was already committed.

Revision history for this message
Faré (fahree) wrote :

ping. May I push?

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

I'll review this tomorrow. BTW, what about making an ASDF-deprecated-facility condition (subclass of STYLE-WARNING?) and signaling it appropriately?

That might get us out of having to maintain everything deprecated forever.

Revision history for this message
Faré (fahree) wrote :

If you're going to do that, that will be great, but you'll also want to grep quicklisp for software that use this functionality and warn authors.

Also there is plenty of stuff in backward-*.lisp file that need to be annotated this way.

A good plan for deprecating things would be great: first only in documentation, then issue style-warning, then issue full warning, then issue cerror, then remove functionality, one at each release or such (although, remove functionality probably only at the next major.minor branch). UIOP and/or asdf/upgrade could include support for that, but oh well.

So yes, that would be a great idea, but would require long-term commitment and a lot of work. I'm not volunteering.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

Why is it that issuing a STYLE-WARNING needs wait to be the second stage in deprecation? It won't actively break anything, unlike a full WARNING, so it seems like this could be done right away.

Revision history for this message
Faré (fahree) wrote :

You might be right that a STYLE-WARNING might be appropriate:

At ITA, we used to break the build on a STYLE-WARNING, but in this case, that would be exactly what we want: detect that an API has changed and must be updated.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

I have some questions about the change in the API before committing:
------------------------------------
My main issue with my patch is that the refactoring just eliminates a few functions that don't make sense anymore and can't be backward-compatibly implemented on Windows:

user-configuration-directories
system-configuration-directories
in-first-directory
in-user-configuration-directory
in-system-configuration-directory
-------------------------------------

Will you please comment on these? Why can't they be implemented on Windows? I.e., what changed? If these functions made sense before, how can re-evaluating the XDG spec (which doesn't apply to Windows anyway), change whether they make sense on Windows now?

If they never made sense on Windows, what were they supposed to do that we can't do now?

When I look at the backward compatibility code, I see this:
  (defun user-configuration-directories ()
    "Determine user configuration directories.
DEPRECATED. Use uiop:config-pathnames instead."
    (config-pathnames "common-lisp"))

So it seems like this function *can* be implemented on Windows. So why is it wrong to use (UIOP:user-configuration-directories) which expands to (config-pathnames "common-lisp") but it's right to use (config-pathnames "common-lisp")?

Why should that be deprecated? It seems unnecessary to be so purist: breaking the API seems worse than leaving around an (admittedly unnecessary) function.

Revision history for this message
Faré (fahree) wrote :

I wanted to provide some uniform fgeneral-purpose API for Linux and Windows (and later Mac) to locate data, config, etc., files. However, whereas the Linux API is to "first locate a base directory, then deduce a category subdirectory, then take a subdirectory of the application, then a subdirectory for the item within the application", the Windows API is to "first locate a base directory, then a subdirectory for the application, then one for the category, then for the item". So the order of subcomponents is different.

It so happens that if the application is always common-lisp, we can fold the common-lisp/ component into a constant before or after the variable part and kind of make it work. But for a general-purpose API we can't do that and we must embrace the difference.

In the long run, it's probably better to encourage users to use the higher level api.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

OK, but USER-CONFIGURATION-DIRECTORIES and SYSTEM-CONFIGURATION-DIRECTORIES are both specific to common-lisp, so there isn't any sense that they are bad and need to be deprecated. So I would prefer to keep the new definitions, and not deprecate them.

Similarly, I don't see any reason why IN-FIRST-DIRECTORY needs to be deprecated: it says what it does. On the other hand, it seems odd to define it in terms of FIND-PREFERRED-DIRECTORY: IN-FIRST-DIRECTORY is a simple function which does what it claims to do. On the other hand IN-PREFERRED-DIRECTORY appeals to some opaque notion of "preference." I'd be inclined to define the latter in terms of the former, if that's appropriate, or just decouple the two.

I see your point that these are all defined in terms of other functions, but I don't think thats sufficient reason to put the community through the nuisance of deprecation. In the best of all worlds, we would have gotten the API right the first time, but when we let it into the open....

Actually, this may be a problem with the idea of having some of these ASDF-specific functions in UIOP: they allow special-purpose functions to slip out into the wild, and then they're out there for anyone to use.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

I have pushed a topic branch for launchpad-1437480

Revision history for this message
Faré (fahree) wrote :

I pushed two trivial tweaks to that branch. One fixes a comment of yours (that I saw in the diff from master), the other uses os-cond everywhere I saw if, when or unless used with os-*-p.

I thought you wanted to NOT use the XDG_ functions on MacOS X. Maybe the /opt/local thing ought to be a MacOS X only option that *replaces* the Linux XDG_* which would then NOT use /opt/local which is not XDG-compliant.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

My concern remains that we are over-reaching in configuration.lisp. The contract we are making with the caller is "we will find the configuration file for your application." We did not make this contract before: we were just offering to find ASDF's configuration files.

My ad hoc modifications were to try to make this plausible on the Mac.

Note that *there is no way to make this contract and limit ourselves to XDG*, since XDG is not even trying to be portable.

I propose that we break out of this logjam as follows: rename all these functions (except for the backwards compatible ones) to have "xdg" in their names. E.g. change "config-pathnames," which makes a contract we cannot keep, to "xdg-config-pathnames," which is well-defined and which we can perform correctly on general platforms (whether it makes sense to do this on an arbitrary platform is a separate question).

Revision history for this message
Faré (fahree) wrote :

I initially named the functions xdg-config-pathnames, etc., but then I realized that what they had to do on Windows was quite unlike XDG, so I removed the XDG prefix. Or perhaps we SHOULD make it XDG, and accept the XDG configuration variables even on Windows and Mac, and revert back to native behavior if it's not found... but I think that's a stretch. OR, we could say "to hell with native behavior at all, this is a Unix-style application, even if it's not native".

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

I favor the alternative of " make it XDG, and accept the XDG configuration variables even on Windows and Mac" except I will go further: do NOT revert back to native behavior, because there is no defined native behavior. There may actually be a defined native behavior on Windows, but I don't know what it is. I don't believe that there's a "native behavior" on Mac that's very well-defined, since Mac OS X is a mish-mash of Unix and something else (?) that has a "Library" directory. That mish-mash is often further mashed by the addition of Mac Ports, Homebrew, Fink, etc. For that matter Windows has an even more messy structure if you run cygwin, since it even has two virtual directory structures (native and cygwin's unix emulation).

So I say let's provide XDG functions, since we can mostly make them work on Linux (I assume on BSDs, too) and Mac.

The only problem with this theory is Windows, but I propose we special case this. I.e., at an outermost level go to either windows config or XDG config.

Not ideal, but I don't to be responsible for an API that claims to find arbitrary config files. Not only does that feel like a huge technical debt, it's so ill-specified as to make it difficult to even tell when you're doing it right or wrong.

Revision history for this message
Faré (fahree) wrote :

OK, so what would we do on Windows? Declare that
$LOCAL_APPDATA/
or maybe
$LOCAL_APPDATA/Unix/
is like /opt/local/
on Unix and do everything below?
etc/ or config/ for XDG_CONFIG_HOME
share/ for XDG_CONFIG_DATA
cache/ for XDG_CACHE_HOME
?

And on Mac, have defaults that include /opt/local ?

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

My feeling is that if we say this is XDG then we don't have to screw with it for Mac. If someone is running mac ports and wants to use XDG, then she must set the XDG_CONFIG variables, because they don't fit the Mac. That's fair. If we said this was just "config" rather than specifically XDG, we'd be implicitly promising to make it work.

So let's rename, and we'll let the Mac users who want to use it with Mac Ports set the XDG environment variables appropriately. They'd have to do that under any circumstances.

So this would mean: rename to add XDG and cut out my ad hoc (non-XDG compliant) pathname additions.

I still need to think some more about Windows.

Revision history for this message
Faré (fahree) wrote :

It seems fair to require explicit XDG_ variables for Mac users who want to use this feature. Then, we can just document that they may want to include /opt/local or whatever else in their appropriate variable configuration menu.

For Windows, I see two possible solutions:

1- try to support the "windows way", which my patch currently does, but is ugly as hell and obviously not designed.

2- declare the "windows way" the bullshit it is, and adopt the Unix way under the Windows directories, where LOCAL_APPDATA replaces /usr/local and APPDATA replaces /usr. Maybe use config/ instead of etc/

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

The fix is in place now. I have closed out the topic branch, and pushed 3.1.4.5.

I believe that we should release this as 3.2 because of the incompatibility in the API with the new functions from UIOP.

Changed in asdf:
status: New → Fix Committed
milestone: none → asdf3.1.5
Changed in asdf:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.