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

[Danesh Dadachanji]

On 25/05/12 03:54 AM, Jiri Vanek wrote:
> On 05/24/2012 09:40 PM, Danesh Dadachanji wrote:
>>
>>
>> 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?
>
> Sure. Sorry.

No problem =) Pushed:

http://icedtea.classpath.org/hg/icedtea-web/rev/9d422db340b8

Cheers,
Danesh