Created attachment 304356 [details] [review]
Make Xsession installation conditional, and off by default

Most distros use custom logic for their Xsession, so we can't really
ship a default here. Doing so would break people running "make install"
and expecting things to work.

Review of attachment 304356 [details] [review]:

seems fine except for the default. some suggestions below, but commit whatever

::: configure.ac
@@ +179,3 @@
 dnl - Configure arguments
 dnl ---------------------------------------------------------------------------
+AC_ARG_ENABLE(enable-gdm-xsession,

maybe gdm is redundant here

@@ +182,3 @@
+              AS_HELP_STRING([--enable-gdm-xsession],
+                             [Enable installing the gdm Xsession file @<:@default=no@:>@]),,
+              enable_gdm_xsession=yes)

you want enable_gdm_xsession=no right?

::: data/Makefile.am
@@ +236,3 @@
 	fi
 
+if ENABLE_GDM_XSESSION

little funky to have two different kinds of ifs right next to each other. If it works, whatever.  Might be nicer to move this out of install-data-hook

Created attachment 304361 [details] [review]
Make Xsession installation conditional, and off by default

Most distros use custom logic for their Xsession, so we can't really
ship a default here. Doing so would break people running "make install"
and expecting things to work.


--

Thanks. There was a bug before with the switch logic. Fixed that, along with the default.

As for the install rule, the if statement does work (because it's not indented), and I can't really move it out of there since you chmod the install directory in a weird way, and I think automake creating the install directory for us would cause that code not to be hit.

Review of attachment 304361 [details] [review]:

++

Attachment 304361 [details] pushed as afca46e - Make Xsession installation conditional, and off by default