[rfc][icedtea-web] improvements to reproducers engine

[Danesh Dadachanji]

On 24/05/12 02:00 PM, Jiri Vanek wrote:
> On 05/23/2012 09:31 PM, Danesh Dadachanji wrote:
>> On 23/05/12 07:17 AM, Jiri Vanek wrote:
>>> On 05/22/2012 08:58 PM, Danesh Dadachanji wrote:
>>>> Hi Jiri,
>>>>
>>>> On 20/05/12 10:18 AM, Jiri Vanek wrote:
>>>>> Hi!
>>>>>
>>>>> During last week I was asked by Danesh and Saad to introduce more hierarchical server and
>>>>> whitelist for reproducers.
>>>>> The following patches should add those features. Can you guys (requesters:) mind to review if they
>>>>> match yours needs?
>>>>> During the writing i found one bug in current implementation - redundant quotes. I fixed this in
>>>>> third changeset.
>>>>> Zipped attachment is example of source and result.
>>>>>
>>>>
>>>> Thank you very much for doing this! Greatly appreciated!
>>>
>>> hhmmhmh. I'm still little bit unsure about usefulness (probably only because of lack of ideas (except sub dirs (and so lost pacakges in
>>> jars))... So I'm glad to hear that appreciated it ;)
>>>
>>
>> Definitely, I plan to use it as soon as it goes in!
>>
>> [snip]
>>
>>>>>
>>>>> *smallEnchancemtnsToreproducers-whiteList.diff*
>>>>>
>>>>
>>>> Any thoughts on reproducers that depend on other ones' jars? For example, the spaces test and the
>>>> info/title/vendor tests use simpletest1.jar to run. If simpletest1 is not whitelisted then the
>>>> engine will throw failures when running them. Are we expecting testers to know which jars depend on
>>>> which? If this "dependency" is not going to be checked by the Makefile, then we should definitely
>>>> add a note to the wiki page stating the various dependencies.
>>>
>>> I had this on mind when I wrote it. Whitelist just filter runs, not compilation and preparation. All reproducers (simple, signed and in
>>> future also special) are compiled (,signed) , deployed , resources copied, testcases compiled. But to the final list for runinning are
>>> included just those who match the regexes in list
>>>
>>> I would really like to avoid some descriptions or checks in reproducers(dependences)! They can make the engine to heavyweight.
>>
>>
>> Oh my apologies for the noise, I did not realize this was just a runtime whitelist. Awesome then I am perfectly happy with the
>> change. Feel free to push!
>>
>> [snip]
>>
>>> smallEnchancemtnsToreproducers-jarsInDirs2.diff
>>>
>>>
>>> diff -r c74bf4aa138c Makefile.am
>>> --- a/Makefile.am Wed May 23 10:42:45 2012 +0200
>>> +++ b/Makefile.am Wed May 23 13:06:33 2012 +0200
>>> @@ -544,8 +544,40 @@
>>> mkdir -p stamps&& \
>>> touch $@
>>>
>>> +stamps/change-dots-to-paths.stamp: stamps/netx-dist-tests-sign-some-reproducers.stamp
>>> + pushd $(JNLP_TESTS_SERVER_DEPLOYDIR); \
>>> + types=(simple signed); \
>>> + for which in "$${types[@]}" ; do \
>>> + . $(abs_top_srcdir)/NEW_LINE_IFS ; \
>>> + simpleReproducers=(`cat $(abs_top_builddir)/junit-jnlp-dist-$$which.txt `); \
>>> + IFS="$$IFS_BACKUP" ; \
>>> + for dir in "$${simpleReproducers[@]}" ; do \
>>> + if test "$${dir:0:1}" = "." ; then \
>>> + echo "reproducer $$dir starts with dot. It is forbidden" ; \
>>> + exit 5; \
>>> + fi; \
>>> + if test "$${dir:(-1)}" = "." ; then \
>>> + echo "reproducer $$dir ends with dot. It is forbidden" ; \
>>> + exit 5; \
>>> + fi; \
>>
>>> + q=`expr index "$$dir" .`; \
>>> + r=$$? ; \
>>> + if [ $$r = 0 ]; then \
>>
>> I don't think this if-clause is needed anymore, basename/dirname are smart enough to account for this. Let's say dir="MyAwesomeTest",
>> then path="." and file="MyAwesomeTest". Feel free to keep it there as a sanity check but I don't think it's necessary. No need to
>> repost a patch if you just remove the if (unless other issues arise).
> I had let it in. I think there is no need to walk through the block and copy itself to itself when there is no dot.
>>
>>> + slashed_dir="./$${dir//.//}" ; \
>>> + path="`dirname $$slashed_dir`" ; \
>>> + file="`basename $$slashed_dir`.jar" ; \
>>> + echo "copying $$dir.jar to $$path as $$file" ; \
>>> + mkdir --parents $$path ; \
>>> + cp $$dir".jar" "$$path"/"$$file" ; \
>>> + fi ; \
>>> + done ; \
>>> + done ; \
>>> + popd ; \
>>> + mkdir -p stamps&& \
>>> + touch $@
>>> +
>>> #this always tries to remove previous testcert
>>> -$(EXPORTED_TEST_CERT): stamps/netx-dist-tests-sign-some-reproducers.stamp netx-dist-tests-remove-cert-from-public
>>> +$(EXPORTED_TEST_CERT): stamps/change-dots-to-paths.stamp netx-dist-tests-remove-cert-from-public
>>> keytool -export -alias $(TEST_CERT_ALIAS) -file $(EXPORTED_TEST_CERT) -storepass $(PRIVATE_KEYSTORE_PASS) -keystore
>>> $(PRIVATE_KEYSTORE_NAME)
>>>
>>> stamps/netx-dist-tests-import-cert-to-public: $(EXPORTED_TEST_CERT)
>>> @@ -955,6 +987,7 @@
>>> rm -f stamps/netx-dist-tests-prepare-reproducers.stamp
>>> rm -f stamps/netx-dist-tests-compile-testcases.stamp
>>> rm -f stamps/netx-dist-tests-sign-some-reproducers.stamp
>>> + rm -f stamps/change-dots-to-paths.stamp
>>> rm -f junit-jnlp-dist-simple.txt
>>> rm -f junit-jnlp-dist-signed.txt
>>> rm -f $(REPRODUCERS_CLASS_NAMES)
>>>
>>
>> Great stuff, thanks again for doing all of this! Feel free to push to HEAD
>>
>> Cheers,
>> Danesh
>
> Tahnx a lot! Both pushed. Custom makefiles soon unless you will be first;)
>

Tried building this again with --prefix but it failed. I missed picking up on this before, thanks Omair for pointing it out!

diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2012-05-24  Danesh Dadachanji <ddadacha at redhat.com>
+
+	Fix use of src dir instead of build dir when whitelisting.
+	* Makefile.am (REPRODUCERS_CLASS_WHITELIST): Use abs_top_builddir instead
+	of abs_top_srcdir.
+
  2012-05-23  Martin Olsson <martin at minimum.se>

         * plugin/icedteanp/IcedTeaPluginUtils.cc:

Okay for HEAD?

Cheers,
Danesh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-whitelist-dir-pointer.patch
Type: text/x-patch
Size: 521 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120524/e22ffca9/fix-whitelist-dir-pointer.patch