[Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Lukas Slebodnik]

ehlo,

This is a small continuation fo discussin from pull request
"Make pylint and jsl optional" #502[1]

Pylint and jslint are already optional because some downstream distributions
does not have such packages. This is a reason why desing document[2]
mention configuration options for disabling them.
   --disable-pylint --without-jslint

Previusly (4.4) "pylint was executed" before building rpm packages.
This strict requirement was changed because "make lint" is executed
with each pull request in travis.

It was changed in commits
master:

* 5c18feaa206bbaee692fc3640b7b79c8d9d6a638 CONFIGURE: Fix detection of pylint
* 3f91469f327d8d9f3b27e0b67c54a4f47ad845c1 CONFIGURE: Update help message for jslint
* b82d285a4a75e11cc9291ecca12d2fcc26f43ed1 SPEC: Fix build in mock

The main intention of PR#502 [1] is to make it even more optional
and do not fail if pylint is not installed on machine.
In another words, changing default value from "yes" to "autodetect".
I think the main reason is that it is not obvious that it is an optional
dependency if you run just "./configure". But that can be improved with
better error message. @see attachments.

  checking if source directory is a Git reposistory... yes
  checking for more warnings... no
  checking for Pylint... /usr/bin/python: No module named pylint
  configure: error: cannot find pylint for /usr/bin/python

Cristian wrote some explanation in pull request:
  Rational:
  pylint and jsl are not required to build FreeIPA. Both are useful
  developer tools. It's more user friendly to make both components
  optionally with default config arguments. There is no reason to
  fail building on a build system without development tools.

But there is also another opinion. pylint/jslint is not usefull just for
developers but also for packagers. I would personally encourage packagers
to run pylint as part of build. (it took just 2 minutes on my laptop 8 CPUs)
Pylint/jslint should check typical issues in downstream only patches
or with backported patches. My experience with optional dependencies for
unit tests is that packagers tend to remove them in case of failure in tests
and then forget to return back with next release because it is optional.
This is a reason why explicit ./configure --disable-pylint will be a reminder
for them to try run "make lint" with next release.

Cristian's version will not affect fedora developers because there is
recommendation to install all required dependencies for running "make lint"
  dnf builddep -b -D "with_lint 1" --spec freeipa.spec.in

However, there is not such simple way for other distributions
debian unstable[3]/ubuntu 16.04/openSUSE[4]. This is a reason why I
would prefer to keep default vaue to "yes" and just improve
error message. It will remind packagers/developers to run lint.
It does not force them to run it.

So the main question is whether we want to change default for configure
time options --enable-pylint --enable-jslint.

LS

[1] https://github.com/freeipa/freeipa/pull/502
[2] http://www.freeipa.org/page/V4/Build_system_refactoring
[3] https://anonscm.debian.org/cgit/pkg-freeipa/freeipa.git
[4] https://en.opensuse.org/Portal:FreeIPA