GNOME Bugzilla – Bug 335957
Addition of a --with-sysconfsubdir flag
Last modified: 2006-03-28 18:57:49 UTC
Please describe the problem: gdm currently uses a 'gdm' subdirectory inside sysconfdir. This is a good thing to do for cleanliness, but it currently gets in the way when creating a package for, e.g., pkgsrc. One of the neat features in pkgsrc is that it allows the administrator to choose where to place the configuration files for a given package. The problem is that, as gdm automatically appends /gdm to the directory given to --sysconfdir, the administrator will get strange results. For example: imagine that the administrator chose to install gdm configuration files inside /etc/gnome/gdm, for whatever reason. Theorically, he would do: ./configure --sysconfdir=/etc/gnome/gdm but doing so could make gdm look for its configuration files in /etc/gnome/gdm/gdm. Of course, he'd also do '--sysconfdir=/etc/gnome', but that'd be suboptimal because nothing could warrant gdm not to touch any other file in that directory. (I mean, by explicitly saying /etc/gnome/gdm, one should be confident that no files outside that directory are touched.) What I propose is to add a --with-sysconfsubdir flag to the configure script that lets the administrator choose the subdirectory within sysconfdir where gdm is supposed to place its files. Although this solution may sound strange at first (instead of, e.g., a --with-gdmconfdir variable), it is good because it does not change the existing behavior, which is important for backwards compatibility. With this argument, the administrator could do: ./configure --sysconfdir=/etc/gnome/gdm --with-sysconfsubdir= Making the new argument take an empty value could force gdm to not add a subdirectory to sysconfdir, which is what the admin (and pkgsrc) really wanted. I'm unsure about a thing, though. gdm currently looks for the sysconfdir/dm/Sessions directory. This seems to have been almost completely removed in the latest gdm version, but is still mentioned in a few places. Should this go away or stay? If the latter, I have added another flag, called --with-dmconfdir, to explicitly say where this directory is so that the admin can choose its location too. If the --with-dmconfdir thing has to stay, it might be a good idea to rename the --with-sysconfsubdir thing to --with-gdmconfdir to make it orthogonal to this one. So please let me know and I'll fix the attached patch. Also, if I haven't convinced you yet, consider this change as a general sysconfdir cleanup: it centralizes the subdirectory name in a single place, the configure script. Then, the code can freely use the new gdmconfdir variable to refer to the exact place where gdm configuration files are placed, instead of having to remind to explicitly add /gdm to sysconfdir. Please note that similar changes were successfully done and integrated to GConf2, which simplified a lot the package maintenance. (At the moment, gdm is really hard to maintain as a package in pkgsrc.) Steps to reproduce: 1. 2. 3. Actual results: Expected results: Does this happen every time? Other information:
Created attachment 61979 [details] [review] Proposed patch implementing --with-sysconfsubdir.
I've tried to raise the priority to high (but have been unable to do so) because this patch is quite intrusive and it can easily break if it stays in bugzilla for too long. So please keep this in mind. Also, the sooner it gets integrated, the earlier I'll be able to send some other needed cleanups and bug fixes ;)
Created attachment 62011 [details] [review] Fixed patch implementing --with-sysconfsubdir I've just learned from bug 324283 that the way I was using to define gdmconfdir was incorrect as it broke staged installs. This new patch fixes the issue.
I am okay with this patch, with one exception. In your patch you default gdmconfdir to sysconfdir (normally /etc). I think the configure script should default sysconfdir to /etc/gdm if a different value is not passed in. If the user passes in a --sysconfdir argument, that should be used instead of /etc/gdm. This keeps better backwards compatibility, and you point out in your first comment that keeping GDM's config files in a subdirectory is clean for default purposes. However, as you say, users should be able to specify a different value and not have gdm appended to it. In other words, I think the configure.ac should look like this: +if test x"${sysconfsubdir}" != x; then + gdmconfdir='${sysconfdir}/${sysconfsubdir}' +else + gdmconfdir='${sysconfdir}/gdm' +fi Note the only change is in the else, where I append the "/gdm". That would allow you to do what you want, and would keep the default behavior the same. And would keep the configuration files in the location users have gotten used to looking for them by default. Does this seem reasonable? If so, let me know and I'll apply the patch with this one modification (or you can update the patch and resend, either way).
Oh, sorry, looking at your patch again that won't work. I was misreading it. I think what I would prefer is a --without-sysconfsubdir argument to configure that would turn off adding "gdm" to the gdmconfdir rather than a --with-sysconfsubdir argument as implemented in your patch. In other words, I'd prefer the default configure behavior to stay the same, and users like yourself who want the different behavior should be the one to have to add a configure argument to get the behavior you want. Does this seem reasonable? Again, sorry for my confusing previous comment.
I think that configure behavior is also preserved in my original patch: doing, e.g., '--sysconfdir=/etc' would leave gdm configuration files in /etc/gdm as happened before. You'd need to '--sysconfdir=/etc --with-sysconfsubdir=' to put the files straight in /etc. Anyway I'm happy with both approaches; pick up the one you like most ;-) Even though you might want to keep in mind that GConf2 and gnopernicus already have a --with-sysconfdir flag so keeping consistency might be good. Oh, another possibility could be to change this line: if test x"${sysconfsubdir}" != x; then to: if test x"${sysconfsubdir}" != xno; then so that --without-sysconfsubdir could work too. Then both your idea and mine work. Now, strictly speaking: a --with argument for this feature is wrong, no matter that lots of people use AC_ARG_WITH in their packages for similar things. If you read the autoconf's documentation, you'll see why (summarizing, --with's purpose is not this). Ideally, the subdirectory should be given as an AC_ARG_VAR, but, IIRC, this requires a newer autoconf than the one currently used by gdm. I'm all for configure.ac cleanliness but I'm not sure you'd like to bump AC_PREREQ nor if you like AC_ARG_VAR at all ;)
Oh well, the line should read: if test x"${sysconfsubdir}" != xno -a x"${sysconfsubdir}" != x; then so that consistency with gconf and gnopernicus is really preserved.
Thanks, sorry for the noise. I accept the patch - just wanted to make sure we were not changing the default behavior. I think this patch looks really good after 3rd review (and misreading it twice).
Don't worry and thanks for commiting this :-) I see now that you commited it without the modification mentioned in my latest comment (#7). I didn't really consider the possibility of giving --without-sysconfsubdir before you mentioned it, but I think it should work as expected (i.e., disable the subdirectory). Without that line, a user passing that flag could get gdm configuration files installed in a 'no' subdirectory, which is counter-intuitive.
Updated CVS to reflect comment #7.