[icedtea-web] RFC: add proxy auto config support
Omair Majid
omajid at redhat.com
Mon Mar 7 09:57:58 PST 2011
On 03/07/2011 10:54 AM, Dr Andrew John Hughes wrote:
> On 17:32 Fri 04 Mar , Omair Majid wrote:
>> 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.
>>
>
> Typo; that shouldn't have been "I wasn't". I'm referring to having the Rhino-requiring
> functionality be optional and that could have waited until a later commit if necessary.
>
> It's a separate issue that it depends on classes which are only
> present in IcedTea, and that does need to be resolved in the initial
> commit, as it has been.
>
>>>>>>
>>>>>> 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!
>>
>
> :-D
>
>>>>> 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.
>
> Ah ok.
>
>>
>>>>>> 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.
>
> Ok good to commit. I await the next two patches :-)
I have pushed the PAC support. Thanks again for the extensive review.
The attached two patches fix build.properties and jrunscript. They will
probably conflict with each other if applied together; I will sort that
out when I commit them.
jrunscript.patch creates a jrunscript.in file which is processed to
create jrunscript.
rhino-build-properties.patch creates a build.properties.in file which is
processed by to create build.properties.
Any thoughts or comments?
Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jrunscript.patch
Type: text/x-patch
Size: 1297 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110307/9176ec6f/jrunscript.patch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rhino-build-properties.patch
Type: text/x-patch
Size: 1439 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110307/9176ec6f/rhino-build-properties.patch
More information about the distro-pkg-dev
mailing list