[icedtea-web] RFC: add proxy auto config support

[Omair Majid]

On 03/03/2011 07:25 PM, Dr Andrew John Hughes wrote:
> On 19:06 Fri 25 Feb     , Omair Majid wrote:
>> Updated patch attached.
>>
>> On 02/25/2011 04:46 PM, Dr Andrew John Hughes wrote:
>>> On 10:38 Fri 25 Feb     , Omair Majid wrote:
>>>
>>> snip (this is essentially a rewrite of the build work :-)
>>>
>>
>> There are some code changes too (nothing in the actual implementation of
>> PAC parsing though). The PacEvaluator interface and PacEvaluatorFactory
>> were added to allow icedtea-web to continue building without rhino.
>>
>
> Yeah that's a nice feature to have.  I was expecting it in the initial revision.
>

Sorry for missing it out, I just wasnt sure how to implement it back 
then. I also didnt think it was necessary - the JDK had the necessary 
sun.org.mozilla.* classes.

>>>>
>>>> Any thoughts on the attached patch? Also, do you have any links to good
>>>> documentation on AC_CONFIG_FILES and good practices for it? I would like
>>>> to use it if possible for build.properties but so far I dont have much
>>>> to go on.
>>>>
>>>
>>> The autoconf manual is what I usually use.  I'm not aware of anything else.
>>> Of course, you do have the source for it... ;-)
>>>
>>
>> Yeah, but there is a big difference between making something work and
>> making it work well. Even if I read and understand all of autoconf's
>> source, I wont know about the best practices.
>>
>
> I guess you just learn by doing - I know I did!
>

You might notice that's what I am doing :) I post patches and you (very 
helpfully!) nudge me in the right direction. Thanks!

>>> I think it's just simple subsitution - i.e. @x@ is replaced with the value of x.
>>> Nothing clever.  So you'd need to create a RHINO_AVAILABLE in configure and
>>> then allow it to be substituted.
>>>
>>> Comments below.  Only issue for this patch is the IT_FIND_JAVA macro.
>>> The autoconf changes should be in a separate patch.  This one is already
>>> big enough ;-)
>>>


As you said, the autoconf changes (build.properties and jrunscript) will 
be fixed later on.


>>>> diff -r d7fee305bd4f acinclude.m4
>>>> --- a/acinclude.m4	Wed Feb 23 13:37:10 2011 -0500
>>>> +++ b/acinclude.m4	Thu Feb 24 19:34:38 2011 -0500
>>>> @@ -122,6 +122,13 @@
>>>>      fi
>>>>    ])
>>>>
>>>> +AC_DEFUN([IT_FIND_JAVA])
>>>> +[
>>>> +  AC_REQUIRE([IT_CHECK_FOR_JDK])
>>>> +  JAVA=${SYSTEM_JDK_DIR}/bin/java
>>>> +  AC_SUBS(JAVA)
>>>> +]
>>>> +
>>>
>>> This isn't the one from IcedTea.  It's a lot more complicated...
>>> BTW, AC_SUBS should be AC_SUBST?
>>>
>>
>> I somehow missed the FIND_JAVA from IcedTea6 :/ I see it there now. I
>> have added it as IT_FIND_JAVA.
>>
>
> Which version did you use?  It's now IT_FIND_JAVA in IcedTea too, and
> I did some fixing for 1.10.  Hence why I recognise the beast so well!
>
>> Yes, that should be AC_SUBST (I wonder how it worked when I was testing
>> it...).
>>
>
> I was asking myself the same question.
>

Mea culpa. There is already an existing IT_FIND_JAVA macro - it's whats 
used to run the configure checks for sun.* classes. I somehow completely 
missed it.

Since it already exists, I am taking it out of this patch. I will post a 
separate patch to update it to match IcedTea6's IT_FIND_JAVA. This is 
the only change in the patch.

>>>> diff -r d7fee305bd4f netx/net/sourceforge/jnlp/runtime/RhinoBasedPacEvaluator.java
>>>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>>>> +++ b/netx/net/sourceforge/jnlp/runtime/RhinoBasedPacEvaluator.java	Thu Feb 24 19:34:38 2011 -0500
>>>> @@ -0,0 +1,256 @@
>>
>>>> +    /**
>>>> +     * Returns the contents of file at pacUrl as a String.
>>>> +     */
>>>> +    private String getPacContents(URL pacUrl) {
>>>> +        StringBuilder contents = null;
>>>> +        try {
>>>> +            String line = null;
>>>> +            BufferedReader pacReader = new BufferedReader(new InputStreamReader(pacUrl.openStream()));
>>>> +            contents = new StringBuilder();
>>>> +            while ((line = pacReader.readLine()) != null) {
>>>> +                // System.out.println(line);
>>>> +                contents = contents.append(line).append("\n");
>>>> +            }
>>>> +        } catch (IOException e) {
>>>> +            contents = null;
>>>> +        }
>>>> +
>>>> +        System.out.println(contents);
>>
>> Removed print statement.
>>
>> Ok to commit?
>>
>
> Just update IT_FIND_JAVA from 1.10 and it's good to go.
>

Skipped for now.

> Did we say we'd replace build.properties and jrunscript in a later patch?
>

Yes, please see the comment above.

Thanks,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pac-support-08.patch
Type: text/x-patch
Size: 77662 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110304/71bf3209/pac-support-08.patch