Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update autotools, add Travis-CI support, and remove some unused code #93

Merged
merged 41 commits into from
Jun 22, 2017

Conversation

grondo
Copy link
Member

@grondo grondo commented Jun 21, 2017

The initial objective of these changes was to be able to update autotools for pdsh and remove autotools products from version control, in order to make addition of new modules much easier on the developer (e.g. see #91).

To make this easier, the use of libltdl was dropped in favor of straight dlopen(3) and dlsym(3), which should be fine for most if not all pdsh users nowadays.

Along the way many modules that were presumed unused were dropped, including

  • Quadrics/Elan support (qshell, mqshell, and the rmsquery module)
  • The SDR module used to get lists of hosts from SDRGetObjects on IBM SPs
  • The nodeattr module (everyone uses libgenders now)

After this work, the xpopen() function had no users so it was removed.

The use of META and config/ac_meta.m4 were removed.

Finally, support for Travis-CI was added, and many compiler warnings were fixed along the way.

Support for --enable-code-coverage and make check-code-coverage was also added, but I couldn't get this working in Travis yet to send results to codecov.io.

@garlick, @chu11, happy to take any feedback here.

@tabaer, you might want to rebase #91 on this and see how it goes -- it should be much smoother.

Fixes #79

grondo added 9 commits June 19, 2017 13:47
Add /.libs to path for PDSH_MODULE_DIR in tests, so that we find
and load the .so files instead of .la libtool archives.
Set pdsh_module_dir to src/modules/.libs so that we load .so module
files, instead of attempting to load the libtool archive .la files.
Maintaining the autotools generated files in version control is a huge
pain in the rear end. Every time someone runs ./bootstrap on a new
system they create huge amounts of unnecessary churn.

Yes, this requires that all developers and users working from a git
checkout have compatible autotools installed on their system, so that
they can successfully run ./bootstrap and ./configure. However, the
alternative is a big mess and who knows how many years of therapist
payments.
In the slurm and torque modules, the ltdl.h header is included but
not used. Remove it in preparation for removal of libltdl.
Dump libltdl in favor of straight dlopen/dlsym for pdsh modules.
The new libltdl wrapper is a whole thing of its own and a big burden
to use (it wants to be installed as a subproject inside of the
project which uses it)

This likely means pdsh dynamic modules are no longer supported on
systems. Likely those systems don't work now anyway. In the future
if an unlikely port is needed to a system requiring libltdl, it should
be possible to tackle re-implementation of libltdl (or some other
portability layer) at that time.
Fix most autotools warnings by pushing the result of `autoupdate`
plus some other various fixes by hand.
Remove non-portable use of $(wildcard) in test/Makefile.am.

Just explicitly list the tests instead. This works better
for newer automake parallel-tests anyway.
Remove support for launching jobs on older Quadrics/Elan HW via
the old (m)qcmd rcmd modules. There are very likely no more users
of this interface.
The etc/ subdir only had files related to Qsw/elan services, which
are no longer supported, so remove them.
config/ac_pam.m4 Outdated
@@ -21,8 +21,7 @@ AC_DEFUN([AC_PAM],
#
AC_MSG_CHECKING([for whether to build with pam support])
AC_ARG_WITH([pam],
AC_HELP_STRING([--without-pam],
[Do not build qshell/mqshell with pam support]),
AS_HELP_STRING([--without-pam],[Do not build qshell/mqshell with pam support]),
[ case "$withval" in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably remove ac_pam? It appears to be specific to qshell/mqshell?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch forgot about that one.

grondo added 20 commits June 21, 2017 12:57
PAM checks were only required by qshell/mqshell daemons, so
the autoconf tests can now be removed.
Remove support for querying WCOLL from RMS resource manager via
the rms misc module. There are likely no current users of this
interface.
Remove the SDR module, which queried the SDR on IBM SP machines
using a call to SDRGetObjects. There are likely no users of this
module anymore.
Remove the nodeattr module. All users should be using the genders
module instead.
The xpopen() call is no longer used after removal of the nodeattr
module. Remove the unused code from the source.
Udpate pdsh manpage to remove modules that are deprecated or no
longer included in the distribution.
Remove unsupported modules from the static modules macros.
Remove qsw modules from rcmd_rank_list used for default rcmd
module priority.
Remove reference to unsupported modules from README and README.modules
Detect pdsh version from output of git describe.
Simplify the bootstrap file after autotools updates.
Fix make check for cases where builddir != srcdir
Add scripts/travis-ci-deps.sh script to download and build extra
travis-ci build and test dependencies. For now this is only the
updated autotools versions that have not made it into travis.
grondo added 12 commits June 21, 2017 12:58
Remove unused variable `q` in pipecmd_format_arg() to silence
warning on GCC 6.
Add missing return code checks for all write(2) calls to silence
compiler warnings.
Add checks for return codes from write(2) to silence compiler
warnings.
Fix compiler warning in _read_netgroup (suggested parenthesis)
Check return code of system(3) in interactive dsh to silence
compiler warnings.
Silence compiler warning by checking return code of setuid(2) in
interactive dsh, even though setuid(2) is probably not necessary here
anymore since pdsh is using privilege separation.
remove unused variable in pcptest_argv_create
Check return codes on all write(2) calls to silence compiler warnings.
Fix compiler warning for failing to check return code of setuid,
even though likely these calls are not used.
Fix compiler warning for unchecked return code of setXid calls.
Using the ax_code_coverage.m4 macro, enable a --enable-code-coverage
option for ./configure which activates a `make check-code-coverage`
target that builds an integrated code coverage report using lcov(1)
when available.
@grondo grondo force-pushed the autotools-update branch from 990a8e7 to 9cc020c Compare June 21, 2017 20:02
@garlick
Copy link
Member

garlick commented Jun 21, 2017

There are a ton of great cleanups here. Thanks for doing all this painful work! Good riddance to the outdated modules too.

@garlick
Copy link
Member

garlick commented Jun 22, 2017

FWIW I had zero issues bootstrapping, building, and checking this branch from a freshly cloned repo on c9.io.

@grondo
Copy link
Member Author

grondo commented Jun 22, 2017

Thanks @garlick, @chu11. I'm going to merge this here and tag a pdsh-2.32. Then we should move the main project over to chaos/pdsh at some point.

@grondo grondo merged commit b52893c into master Jun 22, 2017
@grondo grondo deleted the autotools-update branch June 22, 2017 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update autotools
3 participants