[Freeipa-devel] [PATCH] 14 Run lint during each build

[Martin Kosek]

On Wed, 2011-04-27 at 13:52 -0400, Dmitri Pal wrote:
> On 04/27/2011 12:33 PM, Adam Young wrote:
> > On 04/27/2011 10:24 AM, Martin Kosek wrote:
> >> On Wed, 2011-04-27 at 09:56 -0400, Adam Young wrote:
> >>> On 04/27/2011 09:34 AM, Dmitri Pal wrote:
> >>>> On 04/27/2011 08:13 AM, Jan Cholasta wrote:
> >>>>> On 27.4.2011 13:17, Martin Kosek wrote:
> >>>>>> On Wed, 2011-04-27 at 12:40 +0200, Jan Cholasta wrote:
> >>>>>>> On 26.4.2011 18:14, Martin Kosek wrote:
> >>>>>>>> On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote:
> >>>>>>>>> Automatically run the lint script during make
> >>>>>>>>> rpms|client-rpms|srpms.
> >>>>>>>>>
> >>>>>>>> NACK until ticket 1184 is resolved and pushed. Currently,
> >>>>>>>> pylint check
> >>>>>>>> fails when optional python packages (like python-rhsm) are not
> >>>>>>>> installed
> >>>>>>>> on the machine. We should be able to build IPA without those
> >>>>>>>> packages
> >>>>>>>> installed.
> >>>>>>> I think printing a note asking the developer to kindly install the
> >>>>>>> missing packages would be sufficient. AFAIK there are currently
> >>>>>>> only 2
> >>>>>>> optional packages: python-rhsm and python-krbV. python-krbV is
> >>>>>>> optional
> >>>>>>> only for the client part of IPA, so you most likely have it already
> >>>>>>> installed and installing python-rhsm is not really much of a chore.
> >>>>>>> That
> >>>>>>> way all of the code would always be checked and the lint script
> >>>>>>> would be
> >>>>>>> free of the unnecessary complexity of handling missing packages.
> >>>>>> I don't think this is a right approach. When the package is optional
> >>>>>> (currently it may be python-rhsm and python-krbV only, but there
> >>>>>> may be
> >>>>>> others in the future) I shouldn't be obliged to install them in
> >>>>>> order to
> >>>>>> build IPA.
> >>>>> You shouldn't be obliged to install them as a user. As a developer,
> >>>>> you should be ready for all kinds of crazy stuff IMHO.
> >>>>>
> >>>>>> When somebody develops something related with the optional
> >>>>>> package he has them installed and the lint will check the
> >>>>>> relevant code
> >>>>>> too.
> >>>>> All of the code goes to the package, so it all should be checked
> >>>>> during the build.
> >>>>>
> >>>>> Imagine situation like this: You change something in module A,
> >>>>> accidentally breaking functionality that module B depends on.
> >>>>> Module A
> >>>>> is checked and no error is found (because it's the kind of issue that
> >>>>> exhibits itself only in certain conditions). Module B isn't checked,
> >>>>> because it also depends on a not-installed optional package. If it
> >>>>> was
> >>>>> checked, it would report an error that would lead you to the error in
> >>>>> module A. But everything looks fine, so the build succeeds, even when
> >>>>> the error is there.
> >>>>>
> >>>>> The situation might be improbable, but IMO the code should be checked
> >>>>> in the same ecosystem every time anyway, because weird stuff could
> >>>>> happen if it wasn't.
> >>>>>
> >>>>>> It is not that big deal, I just think it would be an annoyance for
> >>>>>> developers. But maybe there is a different opinion.
> >>>>> I know we developers are lazy folk, but this is a matter of writing
> >>>>> one simple command, just one time.
> >>>>>
> >>>>>> Martin
> >>>>>>
> >>>> How about a compromise?
> >>>> By default everything is expected to be installed.
> >>>> But there is a command line switch that allows to skip modules you
> >>>> want
> >>>> to skip. You provide the switch and the list of the modules to skip
> >>>> and
> >>>> build will validate only modules that are not in the list.
> >>>>
> >>>> Will something like this work?
> >>>>
> >>> Actually, make the command line switch just means that a  Lint failure
> >>> doesn't stop the build.  That way, by default the build will fail
> >>> unless
> >>> everything is there and checked, but there is a way to move forward for
> >>> building with a subset of packages.
> >> Yes, I think we will can settle with a compromise. My only concern was
> >> not to force the developers to install unnecessary packages for build.
> >>
> >> I would suggest that the build (or "make lint") succeeds without those
> >> optional packages installed, but a warning is printed out that some
> >> packages are missing and not all the code is checked.
> >>
> >> Then it is a developers responsibility to handle this and wouldn't be
> >> force to install those packages for his test builds etc.
> >
> > How about instead it fails bny default, but prints the message "to
> > suppress the lint check stopping the build, run make
> > --no-fail-on-lint"  so that skipping lint is a deliberate decision?
> 
> 
> Yes this is the approach I prefer.

OK then, I won't go against the crowd here, it's not that big deal :-)
Honza, please, update the patch accordingly and I will review it.

When the "make lint" fails because of missing optional package(s), I
would like the missing package(s) to be printed out for the user. So
that user can easily do "yum install <PKG-LIST>" and finish the IPA
build.

Martin