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
>From 560f25cea9363f4e3d9041895e85b5eec024a0f9 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 28 Apr 2011 18:14:43 +0200
Subject: [PATCH] Run lint during each build.

ticket 1180
---
 Makefile |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index f8f5987..d12bb43 100644
--- a/Makefile
+++ b/Makefile
@@ -37,6 +37,11 @@ IPA_RPM_RELEASE=$(shell cat RELEASE)
 
 LIBDIR ?= /usr/lib
 
+DEVELOPER_MODE ?= 0
+ifneq ($(DEVELOPER_MODE),0)
+LINT_OPTIONS=--no-fail
+endif
+
 all: bootstrap-autogen server
 	@for subdir in $(SUBDIRS); do \
 		(cd $$subdir && $(MAKE) $@) || exit 1; \
@@ -73,7 +78,7 @@ client-install: client
 	fi
 
 lint:
-	./make-lint
+	./make-lint $(LINT_OPTIONS)
 
 test:
 	$(MAKE) -C install/po test_lang
@@ -148,21 +153,21 @@ rpmdistdir:
 	mkdir -p dist/rpms
 	mkdir -p dist/srpms
 
-rpms: rpmroot rpmdistdir version-update tarballs
+rpms: rpmroot rpmdistdir version-update lint tarballs
 	cp dist/sources/$(TARBALL) $(RPMBUILD)/SOURCES/.
 	rpmbuild --define "_topdir $(RPMBUILD)" -ba freeipa.spec
 	cp rpmbuild/RPMS/*/$(PRJ_PREFIX)-*-$(IPA_VERSION)-*.rpm dist/rpms/
 	cp rpmbuild/SRPMS/$(PRJ_PREFIX)-$(IPA_VERSION)-*.src.rpm dist/srpms/
 	rm -rf rpmbuild
 
-client-rpms: rpmroot rpmdistdir version-update tarballs
+client-rpms: rpmroot rpmdistdir version-update lint tarballs
 	cp dist/sources/$(TARBALL) $(RPMBUILD)/SOURCES/.
 	rpmbuild --define "_topdir $(RPMBUILD)" --define "ONLY_CLIENT 1" -ba freeipa.spec
 	cp rpmbuild/RPMS/*/$(PRJ_PREFIX)-*-$(IPA_VERSION)-*.rpm dist/rpms/
 	cp rpmbuild/SRPMS/$(PRJ_PREFIX)-$(IPA_VERSION)-*.src.rpm dist/srpms/
 	rm -rf rpmbuild
 
-srpms: rpmroot rpmdistdir version-update tarballs
+srpms: rpmroot rpmdistdir version-update lint tarballs
 	cp dist/sources/$(TARBALL) $(RPMBUILD)/SOURCES/.
 	rpmbuild --define "_topdir $(RPMBUILD)" -bs freeipa.spec
 	cp rpmbuild/SRPMS/$(PRJ_PREFIX)-$(IPA_VERSION)-*.src.rpm dist/srpms/
-- 
1.7.4.4

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

Reply via email to