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

[Jiri Vanek]

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.
>
> Cheers,
> Danesh