search

spanezz / nodm

Automatic display manager
GNU General Public License v2.0
141 stars 21 forks source link

suggestion: make PAM optional at build time as well #6

Open rofl0r opened 7 years ago

rofl0r commented 7 years ago

patch: https://github.com/sabotage-linux/sabotage/commit/68552a8c75bc95ca3fbdecdebe749b862fcf25c9

MarcinWieczorek commented 7 years ago

That's a nice patch. Why won't you commit directly to nodm and create a Pull Request? This way you could spare the maintainer some time.

rofl0r commented 7 years ago

well, i thought i'd first hear what the maintainer has to say, if anything at all, because making the patch fit for primetime would require to check for availability of libpam in configure.ac instead of just removing the hunk as it's currently done. and doing autoconf work is a PitA i'd rather spare myself from going through, if there's no interest anyhow.

rofl0r commented 7 years ago

linking to my commit also serves another purpose: to make the author realize what other patches distros currently use to make his package compile. for example we can find the following gems there: 

sed -i 's/ -Werror//g' Makefile.am

would not have been necessary if the author would follow common sense and disable -Werror for non-devel builds.

another gem here

sed -i Makefile.am -e 's;man_MANS = nodm.8;;g' #stolen from void

fixes the build failure due to usage of some uncommon manpage generator tool that's not installed (hint: if it's not installed, which can be found out via a configure check, it would be nicer not to force the user to install it, but just not build the manpage, or even better, ship the prebuilt thing!)

...and another gem here:

sed -i 's/AM_CONFIG_HEADER/AC_CONFIG_HEADERS/g' configure.ac

this fixes autoreconf -i from failing due to usage of obsolete m4 macros

..and there's more!

autoreconf -i

why do we have to use autoreconf at all ? wouldnt it be much better to ship the generated configure script and spare the user to have to install autoconf and automake ?

and last but not least:

CPPFLAGS="-D_GNU_SOURCE -include unistd.h -include stdlib.h"

fix the build due to usage of things declared in unistd.h and stdlib.h, which were due to luck imported via the pam headers.