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

Jan Cholasta jcholast at redhat.com
Thu Apr 28 16:22:37 UTC 2011


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.

>
> Martin


-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-14.1-lint-build.patch
Type: text/x-patch
Size: 1971 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110428/00354797/attachment.bin>


More information about the Freeipa-devel mailing list