[rfc] [icedtea-web] make links

Jiri Vanek jvanek at redhat.com
Mon Apr 23 08:46:56 PDT 2012 

On 04/19/2012 08:54 PM, Andrew Hughes wrote:
>
>
> ----- Original Message -----
>> can I dare to ping? :)
>>
>> -------- Original Message --------
>> Subject: [rfc] [icedtea-web] make links
>> Date: Fri, 06 Apr 2012 16:55:04 +0200
>> From: Jiri Vanek<jvanek at redhat.com>
>> To: IcedTea Distro List<distro-pkg-dev at openjdk.java.net>
>>
>> Hi!
>>
>> This is third from total of 5 patches which are introducing Tets For
>> Applets[1]
>>
>> This particular one is introducing  detection of browsers, linking
>> the newly compiled plugin to
>> detected ones and forwarding this information to testengine..
>>
>> I know this linking by makefile is controversial, but it gives sense
>> to me.
>> Yes  - I can link the plugin with browsers manually before tests' run
>> (and yes, this can be done
>> once for all the runs in future), but I think this is much more
>> versatile and is much more
>> straightforward and much better controllable and also more
>> fool-proof. The question weather to
>> *UNlink* is more accurate:) ( there is no unlink right now, but
>> should be easily addable if requested.
>> Also when tests are run from makefile, then also all the possible
>> preparation should be done from
>> makefile. And this step is necessary. I have also introduced test
>> weather linking is correct see
>> private void testBrowser(Browser browser) calls in [2]
>>
>
> There are two main problems I see:
>
> 1.  You're calling global-links from the main run-netx-dist-tests
> target.  This means it'll get called by make check
No no, make check and make run-netx-dist-tests are independent targets
>.  So you're now
> requiring that test runs need root privileges and alter the installed
> plugin without reverting it. I don't think this is acceptable.
Yes... my idea was to run separate sudo make links and then make run-netx-dist-tests, and die quikly
> I have no problem with the targets being present, but they should be
> called manually.
Now they are.  Issu eis, that there isno way (except documentation) tah tests-runner should call 
them before run-netx-dist-tests (and when he does not, then probably the link will lead to old, 
installed version instead to freshly built one.

I have (in next changeset) tests to this behaviour, but it can take time for somebody inexpirienced 
to find the issue.

>
> 2.  You don't provide a way of reverting to the original status quo.
> I'd suggest:
>
> 	    mv -v ${HOME}/.mozilla/plugins/libjavaplugin.so ${HOME}/.mozilla/plugins/libjavaplugin.original.so&&  \
> 	    ln -s $(DESTDIR)$(libdir)/IcedTeaPlugin.so libjavaplugin.so&&  \
>
> and adding the inverse to revert back to the original setup after testing.
>
>              if [ -e ${HOME}/.mozilla/plugins/libjavaplugin.original.so ] ; \
>                 rm -f ${HOME}/.mozilla/plugins/libjavaplugin.so&&  \
>                 mv ${HOME}/.mozilla/plugins/libjavaplugin.original.so ${HOME}/.mozilla/plugins/libjavaplugin.so ; \
>              fi ;

I have added those targets.
I have copied the the backup to home, because browsers have been confused with duplicate entries 
(especially to two links pointing two different versions of same plugin ;)
>
> Other issues:
>
> 1.  Why is user-links never called from anywhere?  This is actually less
> dangerous (doesn't require root) and could be automated.
Yes. I want to test both linking. One of them had to  be called manually.  Now both of them have to be.
I'm not sure how to handle those linkage dependences and warnings to tester.
> 2.  You don't check that ${HOME}/.mozilla/plugins exists.
It should exists if some mozilla like browser is installed. And I'm checking for installed browsers, 
so this shouyld be ok.
> 3.  I think using ${HOME} as above may be safer than relying on tilde expansion by the shell.
fixed!
> 4.  I don't think we need the messages.  They may have been useful for debugging but can now be removed.
I have removed the messages. But then I have extended it for "unlink"  targets and it become quite 
confusing. Soi I have added them back and now is clear what are those manual targets doing. I'm for 
to kept them inside (two levels of conditional branching CAN be confusing).
> 5.  The dependencies on the new targets seem excessive.  All you need is whatever dependency ensures
> the plugin has been installed.  You don't need e.g. javaws.desktop or docs.stamp because you don't use
> them.
fied
> 6.  On that note, surely it should use the plugin from the build directory as people tend to run "make check"
> before "make install"?
50% true, 50% not ;) (If I translated it correctly)

make check and make run-netx-dist-tests are independent targets. Make check should be run before 
make install and have no linking work. But make run-netx-dist-tests MUST have icedtea-wen installed 
and MUST be run after make install, and is using those new linking targets

It give sense to me. (?)
>
>> What I'm missing is When I have choose AC_CHECK_PROGS, then it does
>> not support --with-val :-/ Is
>> there any AC_CHECK_PROGS which allows to set with-val?
>> Becasue I will probably put rather
>> --with-firefox=/my/preconfiguired/firefox instead of let it use
>> my default one. Although Lynks is quite good alternative during tests
>> runtime :)
>> Also this can be maybe intorduced as new patch later.
>>
>
> You probably want to look at mirroring something like IT_FIND_JAVAH in IcedTea.

Yap. Looks close. I would like to have this as separate patch if you agree.
>
>> Best regards J.
>>
>> [1]
>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2012-March/017799.html
>> [2]
>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120320/8ef6bdfe/browserTests-tests-0001.diff
>> (btw, this page is partially corrupted - there is html test, and
>> firefox is trying to translate it
>> as html page :-/.. so see source rather :-/)
>>
>>
>> changelog:
>> 2012-04-06  Jiri Vanek<jvanek at redhat.com>
>>
>> 	Added detection of installed browsers and added targets to create
>> 	symbolic links from install dir
>> to browsers' plugin directories. Primarily for testing purposes
>> 	*Makefile.am: (clean-local) optionally delets links' stamps
>> 	(stamps/user-links.stamp) with alias (links) - new target for
>> 	creating symlinks for all users. One
>> must be root to execute this target.
>> 	(stamps/global-links.stamp) with alias (user-links) - new target for
>> 	creating symlinks for logged
>> user only. Because opera is missing this feature, quite useless.
>> 	*configure.ac: added basic check whether and which browsers are
>> 	installed
>>
>>
>

changelog:
2012-04-23  Jiri Vanek<jvanek at redhat.com>

  	Added detection of installed browsers and added targets to create
  	symbolic links from install dir
  to browsers' plugin directories. Primarily for testing purposes
  	*Makefile.am: (clean-local) optionally delets links' stamps
  	(stamps/user-links.stamp) with alias (links) - new target for
  	creating symlinks for all users. One
  must be root to execute this target.
  	(stamps/global-links.stamp) with alias (user-links) - new target for
  	creating symlinks for logged
	(restore-global-links): target for restoring original global links. One must be root again
  user only. Because opera is missing this feature, quite useless.
	(restore-user-links): target for rfrstoring user's links
  	*configure.ac: added basic check whether and which browsers are
  	installed



Tahnx for review!
And best regards!
      J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: makeLinks2.diff
Type: text/x-patch
Size: 10643 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120423/e9d3a757/makeLinks2.diff

More information about the distro-pkg-dev mailing list