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

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to