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

Thu May 5 09:54:31 UTC 2011

On Mon, 2011-05-02 at 09:34 +0200, Martin Kosek wrote:
> On Thu, 2011-04-28 at 18:22 +0200, Jan Cholasta wrote:
> > On 28.4.2011 08:57, Martin Kosek wrote:
> > > 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.
> > 
> > I've added two new variables to the makefile: DEVELOPER_MODE and 
> > LINT_OPTIONS. LINT_OPTIONS contains the command line options passed to 
> > make-lint. Setting DEVELOPER_MODE to 1 enables the developer mode, which 
> > currently just presets LINT_OPTIONS to --no-fail (it might be used for 
> > more in future), so you can build your rpms even without python-rhsm 
> > installed by invoking:
> > 
> >      make rpms DEVELOPER_MODE=1
> > 
> > >
> > > 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.
> > 
> > This will be in my next patch, dealing with ticket 1184.
> > 
> 
> ACK. This should be pushed along with your patch 15 (ticket 1184).
> 
> Martin
> 

Pushed to master, ipa-2-0.

Martin