Re: [Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"
On Fri, 2012-04-20 at 10:14 +0200, Petr Viktorin wrote: > On 04/19/2012 08:35 PM, John Dennis wrote: > > On 04/19/2012 07:04 AM, Petr Viktorin wrote: > >> On 04/18/2012 09:32 PM, John Dennis wrote: > Now that there are warnings, is pedantic mode necessary? > >>> > >>> Great question, I also pondered that as well. My conclusion was there > >>> was value in separating aggressiveness of error checking from the > >>> verbosity of the output. Also I didn't think we wanted warnings showing > >>> in normal checking for things which are suspicious but not known to be > >>> incorrect. So under the current scheme pedantic mode enables reporting > >>> of suspicious constructs. You can still get a warning in the normal mode > >>> for things which aren't fatal but are provably incorrect. An example of > >>> this would be missing plural translations, it won't cause a run time > >>> failure and we can be definite about their absence, however they should > >>> be fixed, but it's not mandatory they be fixed, a warning in this case > >>> seems appropriate. > >> > >> If they should be fixed, we should fix them, and treat them as errors > >> the same way we treat lint's warnings as errors. If the pedantic mode is > >> an obscure option of some test module, I worry that nobody will ever > >> run it. > > > > The value of pedantic mode is for the person maintaining the > > translations (at the moment that's me). It's not normally needed, but > > when something goes wrong it may be helpful to diagnose what might be > > amiss, in this case false positives are tolerable, in normal mode false > > positives should be silenced (a request of yours from an earlier > > review). Another thing to note is that a number of the warnings are > > limited to po checking, once again this is a translation maintainer > > feature, not a general test/developer feature. > > Thanks for the clarification. Now I see the use. > > >> Separating aggressiveness of checking from verbosity is not a bad idea. > >> But since we now have two severity levels, and the checking is cheap, > >> I'm not convinced that the aggressiveness should be tunable. > >> How about always counting the pedantic warnings, but not showing the > >> details? Then, if such warnings are found, have the tool say how to run > >> it to get a full report. That way people will notice it. > > > > In an earlier review you asked to limit the output to only what is an > > actual provable error. I agreed with you and modified the code. One > > reason not to modify it again is the amount of time being devoted to > > polishing what is an internal developer tool. I've tweaked the reporting > > 3-4 times already, I don't think it's time well spent to go back and do > > it again. After all, this is an internal tool, it will never be seen by > > a customer, if as we get experience with it we discover it's needs > > tweaking because it's not doing the job we hoped it would then that's > > the moment to invest more engineering resources on the output, > > validation, or whatever the deficiency is. > > > > > ACK (it's a 3.0 task, please push to master only) > > Rebased and pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"
On 04/19/2012 08:35 PM, John Dennis wrote: On 04/19/2012 07:04 AM, Petr Viktorin wrote: On 04/18/2012 09:32 PM, John Dennis wrote: Now that there are warnings, is pedantic mode necessary? Great question, I also pondered that as well. My conclusion was there was value in separating aggressiveness of error checking from the verbosity of the output. Also I didn't think we wanted warnings showing in normal checking for things which are suspicious but not known to be incorrect. So under the current scheme pedantic mode enables reporting of suspicious constructs. You can still get a warning in the normal mode for things which aren't fatal but are provably incorrect. An example of this would be missing plural translations, it won't cause a run time failure and we can be definite about their absence, however they should be fixed, but it's not mandatory they be fixed, a warning in this case seems appropriate. If they should be fixed, we should fix them, and treat them as errors the same way we treat lint's warnings as errors. If the pedantic mode is an obscure option of some test module, I worry that nobody will ever run it. The value of pedantic mode is for the person maintaining the translations (at the moment that's me). It's not normally needed, but when something goes wrong it may be helpful to diagnose what might be amiss, in this case false positives are tolerable, in normal mode false positives should be silenced (a request of yours from an earlier review). Another thing to note is that a number of the warnings are limited to po checking, once again this is a translation maintainer feature, not a general test/developer feature. Thanks for the clarification. Now I see the use. Separating aggressiveness of checking from verbosity is not a bad idea. But since we now have two severity levels, and the checking is cheap, I'm not convinced that the aggressiveness should be tunable. How about always counting the pedantic warnings, but not showing the details? Then, if such warnings are found, have the tool say how to run it to get a full report. That way people will notice it. In an earlier review you asked to limit the output to only what is an actual provable error. I agreed with you and modified the code. One reason not to modify it again is the amount of time being devoted to polishing what is an internal developer tool. I've tweaked the reporting 3-4 times already, I don't think it's time well spent to go back and do it again. After all, this is an internal tool, it will never be seen by a customer, if as we get experience with it we discover it's needs tweaking because it's not doing the job we hoped it would then that's the moment to invest more engineering resources on the output, validation, or whatever the deficiency is. ACK (it's a 3.0 task, please push to master only) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"
On 04/19/2012 07:04 AM, Petr Viktorin wrote: On 04/18/2012 09:32 PM, John Dennis wrote: Now that there are warnings, is pedantic mode necessary? Great question, I also pondered that as well. My conclusion was there was value in separating aggressiveness of error checking from the verbosity of the output. Also I didn't think we wanted warnings showing in normal checking for things which are suspicious but not known to be incorrect. So under the current scheme pedantic mode enables reporting of suspicious constructs. You can still get a warning in the normal mode for things which aren't fatal but are provably incorrect. An example of this would be missing plural translations, it won't cause a run time failure and we can be definite about their absence, however they should be fixed, but it's not mandatory they be fixed, a warning in this case seems appropriate. If they should be fixed, we should fix them, and treat them as errors the same way we treat lint's warnings as errors. If the pedantic mode is an obscure option of some test module, I worry that nobody will ever run it. The value of pedantic mode is for the person maintaining the translations (at the moment that's me). It's not normally needed, but when something goes wrong it may be helpful to diagnose what might be amiss, in this case false positives are tolerable, in normal mode false positives should be silenced (a request of yours from an earlier review). Another thing to note is that a number of the warnings are limited to po checking, once again this is a translation maintainer feature, not a general test/developer feature. Separating aggressiveness of checking from verbosity is not a bad idea. But since we now have two severity levels, and the checking is cheap, I'm not convinced that the aggressiveness should be tunable. How about always counting the pedantic warnings, but not showing the details? Then, if such warnings are found, have the tool say how to run it to get a full report. That way people will notice it. In an earlier review you asked to limit the output to only what is an actual provable error. I agreed with you and modified the code. One reason not to modify it again is the amount of time being devoted to polishing what is an internal developer tool. I've tweaked the reporting 3-4 times already, I don't think it's time well spent to go back and do it again. After all, this is an internal tool, it will never be seen by a customer, if as we get experience with it we discover it's needs tweaking because it's not doing the job we hoped it would then that's the moment to invest more engineering resources on the output, validation, or whatever the deficiency is. -- John Dennis Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"
On 04/18/2012 09:32 PM, John Dennis wrote: On 04/18/2012 07:33 AM, Petr Viktorin wrote: On 04/16/2012 10:32 PM, John Dennis wrote: On 04/12/2012 09:26 AM, Petr Viktorin wrote: On 03/30/2012 03:45 AM, John Dennis wrote: Translatable strings have certain requirements for proper translation and run time behaviour. We should routinely validate those strings. A recent checkin to install/po/test_i18n.py makes it possible to validate the contents of our current pot file by searching for problematic strings. However, we only occasionally generate a new pot file, far less frequently than translatable strings change in the source base. Just like other checkin's to the source which are tested for correctness we should also validate new or modified translation strings when they are introduced and not accumulate problems to fix at the last minute. This would also raise the awareness of developers as to the requirements for proper string translation. The top level Makefile should be enhanced to create a temporary pot files from the current sources and validate it. We need to use a temporary pot file because we do not want to modify the pot file under source code control and exported to Transifex. NACK install/po/Makefile is not created early enough when running `make rpms` from a clean checkout. # git clean -fx ... # make rpms rm -rf /rpmbuild mkdir -p /rpmbuild/BUILD mkdir -p /rpmbuild/RPMS mkdir -p /rpmbuild/SOURCES mkdir -p /rpmbuild/SPECS mkdir -p /rpmbuild/SRPMS mkdir -p dist/rpms mkdir -p dist/srpms if [ ! -e RELEASE ]; then echo 0> RELEASE; fi sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ freeipa.spec.in> freeipa.spec sed -e s/__VERSION__/2.99.0GITde16a82/ version.m4.in \ version.m4 sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/setup.py.in \ ipapython/setup.py sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/version.py.in \ ipapython/version.py perl -pi -e "s:__NUM_VERSION__:2990:" ipapython/version.py perl -pi -e "s:__API_VERSION__:2.34:" ipapython/version.py sed -e s/__VERSION__/2.99.0GITde16a82/ daemons/ipa-version.h.in \ daemons/ipa-version.h perl -pi -e "s:__NUM_VERSION__:2990:" daemons/ipa-version.h perl -pi -e "s:__DATA_VERSION__:2010061412:" daemons/ipa-version.h sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ ipa-client/ipa-client.spec.in> ipa-client/ipa-client.spec sed -e s/__VERSION__/2.99.0GITde16a82/ ipa-client/version.m4.in \ ipa-client/version.m4 if [ "redhat" != "" ]; then \ sed -e s/SUPPORTED_PLATFORM/redhat/ ipapython/services.py.in \ ipapython/services.py; \ fi if [ "" != "yes" ]; then \ ./makeapi --validate; \ fi make -C install/po validate-src-strings make[1]: Entering directory `/home/pviktori/freeipa/install/po' make[1]: *** No rule to make target `validate-src-strings'. Stop. make[1]: Leaving directory `/home/pviktori/freeipa/install/po' make: *** [validate-src-strings] Error 2 Updated patch attached. The fundamental problem is that we were trying to run code before configure had ever been run. We have dependencies on the output and side effects of configure. Therefore the solution is to add the bootstrap-autogen target as a dependency. FWIW, I tried an approach that did not require having bootstrap-autogen run first by having the validation occur as part of the normal "make all". But that had some undesirable properties, first we really only want to run validation for developers, not during a normal build, secondly the file generation requires a git repo (see below). But there is another reason to require running bootstrap-autogen prior to any validation (e.g. makeapi, make-lint, etc.) Those validation utilities need access to generated source files and those generated source files are supposed to be generated by the configure step. Right now we're generating them as part of updating version information. I will post a long email describing the problem on the devel list. So we've created a situation we we must run configure early on, even as part of "making the distribution" because part of "making the distribution" is validating the distribution and that requires the full content of the distribution be available. Also while trying to determine if the i18n validation step executed correctly I realized the i18n validation only emitted a message if an error was detected. That made it impossible to distinguish between empty input (a problem when not running in a development tree) and successful validation. Therefore I also updated i18n.py to output counts of messages checked which also caused me to fix some validations that were missing on plural forms. This still doesn't solve the problem: Make doesn't guarantee the order of dependencies, so with the rule: > lint: bootstrap-autogen validate-src-strings > ./make-lint $(LINT_OPTIONS) the validate-src-strings can fire before bootstrap-autogen: Fixed by having bootstrap-autogen be a dependency of the lint target and adding validate-src-strings to the lint rules. $ make li
Re: [Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"
On 04/18/2012 07:33 AM, Petr Viktorin wrote: On 04/16/2012 10:32 PM, John Dennis wrote: On 04/12/2012 09:26 AM, Petr Viktorin wrote: On 03/30/2012 03:45 AM, John Dennis wrote: Translatable strings have certain requirements for proper translation and run time behaviour. We should routinely validate those strings. A recent checkin to install/po/test_i18n.py makes it possible to validate the contents of our current pot file by searching for problematic strings. However, we only occasionally generate a new pot file, far less frequently than translatable strings change in the source base. Just like other checkin's to the source which are tested for correctness we should also validate new or modified translation strings when they are introduced and not accumulate problems to fix at the last minute. This would also raise the awareness of developers as to the requirements for proper string translation. The top level Makefile should be enhanced to create a temporary pot files from the current sources and validate it. We need to use a temporary pot file because we do not want to modify the pot file under source code control and exported to Transifex. NACK install/po/Makefile is not created early enough when running `make rpms` from a clean checkout. # git clean -fx ... # make rpms rm -rf /rpmbuild mkdir -p /rpmbuild/BUILD mkdir -p /rpmbuild/RPMS mkdir -p /rpmbuild/SOURCES mkdir -p /rpmbuild/SPECS mkdir -p /rpmbuild/SRPMS mkdir -p dist/rpms mkdir -p dist/srpms if [ ! -e RELEASE ]; then echo 0> RELEASE; fi sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ freeipa.spec.in> freeipa.spec sed -e s/__VERSION__/2.99.0GITde16a82/ version.m4.in \ version.m4 sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/setup.py.in \ ipapython/setup.py sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/version.py.in \ ipapython/version.py perl -pi -e "s:__NUM_VERSION__:2990:" ipapython/version.py perl -pi -e "s:__API_VERSION__:2.34:" ipapython/version.py sed -e s/__VERSION__/2.99.0GITde16a82/ daemons/ipa-version.h.in \ daemons/ipa-version.h perl -pi -e "s:__NUM_VERSION__:2990:" daemons/ipa-version.h perl -pi -e "s:__DATA_VERSION__:2010061412:" daemons/ipa-version.h sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ ipa-client/ipa-client.spec.in> ipa-client/ipa-client.spec sed -e s/__VERSION__/2.99.0GITde16a82/ ipa-client/version.m4.in \ ipa-client/version.m4 if [ "redhat" != "" ]; then \ sed -e s/SUPPORTED_PLATFORM/redhat/ ipapython/services.py.in \ ipapython/services.py; \ fi if [ "" != "yes" ]; then \ ./makeapi --validate; \ fi make -C install/po validate-src-strings make[1]: Entering directory `/home/pviktori/freeipa/install/po' make[1]: *** No rule to make target `validate-src-strings'. Stop. make[1]: Leaving directory `/home/pviktori/freeipa/install/po' make: *** [validate-src-strings] Error 2 Updated patch attached. The fundamental problem is that we were trying to run code before configure had ever been run. We have dependencies on the output and side effects of configure. Therefore the solution is to add the bootstrap-autogen target as a dependency. FWIW, I tried an approach that did not require having bootstrap-autogen run first by having the validation occur as part of the normal "make all". But that had some undesirable properties, first we really only want to run validation for developers, not during a normal build, secondly the file generation requires a git repo (see below). But there is another reason to require running bootstrap-autogen prior to any validation (e.g. makeapi, make-lint, etc.) Those validation utilities need access to generated source files and those generated source files are supposed to be generated by the configure step. Right now we're generating them as part of updating version information. I will post a long email describing the problem on the devel list. So we've created a situation we we must run configure early on, even as part of "making the distribution" because part of "making the distribution" is validating the distribution and that requires the full content of the distribution be available. Also while trying to determine if the i18n validation step executed correctly I realized the i18n validation only emitted a message if an error was detected. That made it impossible to distinguish between empty input (a problem when not running in a development tree) and successful validation. Therefore I also updated i18n.py to output counts of messages checked which also caused me to fix some validations that were missing on plural forms. This still doesn't solve the problem: Make doesn't guarantee the order of dependencies, so with the rule: > lint: bootstrap-autogen validate-src-strings > ./make-lint $(LINT_OPTIONS) the validate-src-strings can fire before bootstrap-autogen: Fixed by having bootstrap-autogen be a dependency of the lint target and adding validate-src-strings to the lint rules. $ make lint if [ ! -e RELEASE ]; t
Re: [Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"
On 04/16/2012 10:32 PM, John Dennis wrote: On 04/12/2012 09:26 AM, Petr Viktorin wrote: On 03/30/2012 03:45 AM, John Dennis wrote: Translatable strings have certain requirements for proper translation and run time behaviour. We should routinely validate those strings. A recent checkin to install/po/test_i18n.py makes it possible to validate the contents of our current pot file by searching for problematic strings. However, we only occasionally generate a new pot file, far less frequently than translatable strings change in the source base. Just like other checkin's to the source which are tested for correctness we should also validate new or modified translation strings when they are introduced and not accumulate problems to fix at the last minute. This would also raise the awareness of developers as to the requirements for proper string translation. The top level Makefile should be enhanced to create a temporary pot files from the current sources and validate it. We need to use a temporary pot file because we do not want to modify the pot file under source code control and exported to Transifex. NACK install/po/Makefile is not created early enough when running `make rpms` from a clean checkout. # git clean -fx ... # make rpms rm -rf /rpmbuild mkdir -p /rpmbuild/BUILD mkdir -p /rpmbuild/RPMS mkdir -p /rpmbuild/SOURCES mkdir -p /rpmbuild/SPECS mkdir -p /rpmbuild/SRPMS mkdir -p dist/rpms mkdir -p dist/srpms if [ ! -e RELEASE ]; then echo 0> RELEASE; fi sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ freeipa.spec.in> freeipa.spec sed -e s/__VERSION__/2.99.0GITde16a82/ version.m4.in \ > version.m4 sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/setup.py.in \ > ipapython/setup.py sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/version.py.in \ > ipapython/version.py perl -pi -e "s:__NUM_VERSION__:2990:" ipapython/version.py perl -pi -e "s:__API_VERSION__:2.34:" ipapython/version.py sed -e s/__VERSION__/2.99.0GITde16a82/ daemons/ipa-version.h.in \ > daemons/ipa-version.h perl -pi -e "s:__NUM_VERSION__:2990:" daemons/ipa-version.h perl -pi -e "s:__DATA_VERSION__:2010061412:" daemons/ipa-version.h sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ ipa-client/ipa-client.spec.in> ipa-client/ipa-client.spec sed -e s/__VERSION__/2.99.0GITde16a82/ ipa-client/version.m4.in \ > ipa-client/version.m4 if [ "redhat" != "" ]; then \ sed -e s/SUPPORTED_PLATFORM/redhat/ ipapython/services.py.in \ > ipapython/services.py; \ fi if [ "" != "yes" ]; then \ ./makeapi --validate; \ fi make -C install/po validate-src-strings make[1]: Entering directory `/home/pviktori/freeipa/install/po' make[1]: *** No rule to make target `validate-src-strings'. Stop. make[1]: Leaving directory `/home/pviktori/freeipa/install/po' make: *** [validate-src-strings] Error 2 Updated patch attached. The fundamental problem is that we were trying to run code before configure had ever been run. We have dependencies on the output and side effects of configure. Therefore the solution is to add the bootstrap-autogen target as a dependency. FWIW, I tried an approach that did not require having bootstrap-autogen run first by having the validation occur as part of the normal "make all". But that had some undesirable properties, first we really only want to run validation for developers, not during a normal build, secondly the file generation requires a git repo (see below). But there is another reason to require running bootstrap-autogen prior to any validation (e.g. makeapi, make-lint, etc.) Those validation utilities need access to generated source files and those generated source files are supposed to be generated by the configure step. Right now we're generating them as part of updating version information. I will post a long email describing the problem on the devel list. So we've created a situation we we must run configure early on, even as part of "making the distribution" because part of "making the distribution" is validating the distribution and that requires the full content of the distribution be available. Also while trying to determine if the i18n validation step executed correctly I realized the i18n validation only emitted a message if an error was detected. That made it impossible to distinguish between empty input (a problem when not running in a development tree) and successful validation. Therefore I also updated i18n.py to output counts of messages checked which also caused me to fix some validations that were missing on plural forms. This still doesn't solve the problem: Make doesn't guarantee the order of dependencies, so with the rule: > lint: bootstrap-autogen validate-src-strings >./make-lint $(LINT_OPTIONS) the validate-src-strings can fire before bootstrap-autogen: $ make lint if [ ! -e RELEASE ]; then echo 0 > RELEASE; fi /usr/bin/make -C install/po validate-src-strings make[1]: Entering directory `/home/pviktori/freeipa/install/po' make[1]: *** No rule to make target `validate-
Re: [Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"
On 04/12/2012 09:26 AM, Petr Viktorin wrote: On 03/30/2012 03:45 AM, John Dennis wrote: Translatable strings have certain requirements for proper translation and run time behaviour. We should routinely validate those strings. A recent checkin to install/po/test_i18n.py makes it possible to validate the contents of our current pot file by searching for problematic strings. However, we only occasionally generate a new pot file, far less frequently than translatable strings change in the source base. Just like other checkin's to the source which are tested for correctness we should also validate new or modified translation strings when they are introduced and not accumulate problems to fix at the last minute. This would also raise the awareness of developers as to the requirements for proper string translation. The top level Makefile should be enhanced to create a temporary pot files from the current sources and validate it. We need to use a temporary pot file because we do not want to modify the pot file under source code control and exported to Transifex. NACK install/po/Makefile is not created early enough when running `make rpms` from a clean checkout. # git clean -fx ... # make rpms rm -rf /rpmbuild mkdir -p /rpmbuild/BUILD mkdir -p /rpmbuild/RPMS mkdir -p /rpmbuild/SOURCES mkdir -p /rpmbuild/SPECS mkdir -p /rpmbuild/SRPMS mkdir -p dist/rpms mkdir -p dist/srpms if [ ! -e RELEASE ]; then echo 0> RELEASE; fi sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ freeipa.spec.in> freeipa.spec sed -e s/__VERSION__/2.99.0GITde16a82/ version.m4.in \ > version.m4 sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/setup.py.in \ > ipapython/setup.py sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/version.py.in \ > ipapython/version.py perl -pi -e "s:__NUM_VERSION__:2990:" ipapython/version.py perl -pi -e "s:__API_VERSION__:2.34:" ipapython/version.py sed -e s/__VERSION__/2.99.0GITde16a82/ daemons/ipa-version.h.in \ > daemons/ipa-version.h perl -pi -e "s:__NUM_VERSION__:2990:" daemons/ipa-version.h perl -pi -e "s:__DATA_VERSION__:2010061412:" daemons/ipa-version.h sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ ipa-client/ipa-client.spec.in> ipa-client/ipa-client.spec sed -e s/__VERSION__/2.99.0GITde16a82/ ipa-client/version.m4.in \ > ipa-client/version.m4 if [ "redhat" != "" ]; then \ sed -e s/SUPPORTED_PLATFORM/redhat/ ipapython/services.py.in \ > ipapython/services.py; \ fi if [ "" != "yes" ]; then \ ./makeapi --validate; \ fi make -C install/po validate-src-strings make[1]: Entering directory `/home/pviktori/freeipa/install/po' make[1]: *** No rule to make target `validate-src-strings'. Stop. make[1]: Leaving directory `/home/pviktori/freeipa/install/po' make: *** [validate-src-strings] Error 2 Updated patch attached. The fundamental problem is that we were trying to run code before configure had ever been run. We have dependencies on the output and side effects of configure. Therefore the solution is to add the bootstrap-autogen target as a dependency. FWIW, I tried an approach that did not require having bootstrap-autogen run first by having the validation occur as part of the normal "make all". But that had some undesirable properties, first we really only want to run validation for developers, not during a normal build, secondly the file generation requires a git repo (see below). But there is another reason to require running bootstrap-autogen prior to any validation (e.g. makeapi, make-lint, etc.) Those validation utilities need access to generated source files and those generated source files are supposed to be generated by the configure step. Right now we're generating them as part of updating version information. I will post a long email describing the problem on the devel list. So we've created a situation we we must run configure early on, even as part of "making the distribution" because part of "making the distribution" is validating the distribution and that requires the full content of the distribution be available. Also while trying to determine if the i18n validation step executed correctly I realized the i18n validation only emitted a message if an error was detected. That made it impossible to distinguish between empty input (a problem when not running in a development tree) and successful validation. Therefore I also updated i18n.py to output counts of messages checked which also caused me to fix some validations that were missing on plural forms. -- John Dennis Looking to carve out IT costs? www.redhat.com/carveoutcosts/ >From 9dd2046d6d617df256219162b9bb1ea5ea3b00f6 Mon Sep 17 00:00:00 2001 From: John Dennis Date: Mon, 16 Apr 2012 15:51:42 -0400 Subject: [PATCH 70-1] validate i18n strings when running "make lint" Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit * add target valid
Re: [Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"
On 03/30/2012 03:45 AM, John Dennis wrote: Translatable strings have certain requirements for proper translation and run time behaviour. We should routinely validate those strings. A recent checkin to install/po/test_i18n.py makes it possible to validate the contents of our current pot file by searching for problematic strings. However, we only occasionally generate a new pot file, far less frequently than translatable strings change in the source base. Just like other checkin's to the source which are tested for correctness we should also validate new or modified translation strings when they are introduced and not accumulate problems to fix at the last minute. This would also raise the awareness of developers as to the requirements for proper string translation. The top level Makefile should be enhanced to create a temporary pot files from the current sources and validate it. We need to use a temporary pot file because we do not want to modify the pot file under source code control and exported to Transifex. NACK install/po/Makefile is not created early enough when running `make rpms` from a clean checkout. # git clean -fx ... # make rpms rm -rf /rpmbuild mkdir -p /rpmbuild/BUILD mkdir -p /rpmbuild/RPMS mkdir -p /rpmbuild/SOURCES mkdir -p /rpmbuild/SPECS mkdir -p /rpmbuild/SRPMS mkdir -p dist/rpms mkdir -p dist/srpms if [ ! -e RELEASE ]; then echo 0 > RELEASE; fi sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ freeipa.spec.in > freeipa.spec sed -e s/__VERSION__/2.99.0GITde16a82/ version.m4.in \ > version.m4 sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/setup.py.in \ > ipapython/setup.py sed -e s/__VERSION__/2.99.0GITde16a82/ ipapython/version.py.in \ > ipapython/version.py perl -pi -e "s:__NUM_VERSION__:2990:" ipapython/version.py perl -pi -e "s:__API_VERSION__:2.34:" ipapython/version.py sed -e s/__VERSION__/2.99.0GITde16a82/ daemons/ipa-version.h.in \ > daemons/ipa-version.h perl -pi -e "s:__NUM_VERSION__:2990:" daemons/ipa-version.h perl -pi -e "s:__DATA_VERSION__:2010061412:" daemons/ipa-version.h sed -e s/__VERSION__/2.99.0GITde16a82/ -e s/__RELEASE__/0/ \ ipa-client/ipa-client.spec.in > ipa-client/ipa-client.spec sed -e s/__VERSION__/2.99.0GITde16a82/ ipa-client/version.m4.in \ > ipa-client/version.m4 if [ "redhat" != "" ]; then \ sed -e s/SUPPORTED_PLATFORM/redhat/ ipapython/services.py.in \ > ipapython/services.py; \ fi if [ "" != "yes" ]; then \ ./makeapi --validate; \ fi make -C install/po validate-src-strings make[1]: Entering directory `/home/pviktori/freeipa/install/po' make[1]: *** No rule to make target `validate-src-strings'. Stop. make[1]: Leaving directory `/home/pviktori/freeipa/install/po' make: *** [validate-src-strings] Error 2 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
