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

[Omair Majid]

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.

>>
>> 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 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 ;-)
>

Ah, good. That's exactly what I was hoping :D

>> @@ -255,6 +276,7 @@
>>   	   ${INSTALL_DATA} -D $${files} \
>>   	   $(NETX_DIR)/net/sourceforge/jnlp/resources/$${files}; \
>>   	 done)
>> +	cp -pPR build.properties $(NETX_DIR)/net/sourceforge/jnlp/

Added an explicit copy here to copy pac-funcs.js to build dir.

>> @@ -378,6 +401,28 @@
>>   	rm -rf ${abs_top_builddir}/docs/plugin
>>   	rm -f stamps/plugin-docs.stamp
>>
>> +
>> +# check
>> +# ==========================
>> +
>> +jrunscript:
>> +if WITH_RHINO
>> +	echo '$(BOOT_DIR)/bin/java -cp $(RHINO_JAR) org.mozilla.javascript.tools.shell.Main $$@'>  jrunscript
>> +	chmod u+x jrunscript
>> +else
>> +	echo "jrunscript requires rhino support"
>> +	exit 1
>> +endif
>
> autoconf could do this too FWIW.  Look at javac.in in IcedTea.
>

I will address this in a separate patch.

>> +
>> +check-pac-functions: stamps/bootstrap-directory.stamp jrunscript
>> +	./jrunscript $(abs_top_srcdir)/tests/netx/pac/pac-funcs-test.js \
>> +	  $$(readlink -f $(abs_top_srcdir)/netx/net/sourceforge/jnlp/runtime/pac-funcs.js)
>> +
>> +clean-tests: clean-jrunscript
>> +
>> +clean-jrunscript:
>> +	rm -f jrunscript
>> +
>>   # plugin tests
>>
>>   if ENABLE_PLUGIN
>> @@ -414,6 +459,7 @@
>>   # bootstrap
>>   stamps/bootstrap-directory.stamp: stamps/native-ecj.stamp
>>   	mkdir -p $(BOOT_DIR)/bin stamps/
>> +	ln -sf $(JAVA) $(BOOT_DIR)/bin/java
>
> Wasn't this already present?
>

Surprisingly, no. Here's what my bootstrap tree looks like from a fresh 
clone:

$ find
.
./jdk1.6.0
./jdk1.6.0/bin
./jdk1.6.0/bin/jar
./jdk1.6.0/bin/javac
./jdk1.6.0/bin/javadoc
./jdk1.6.0/jre
./jdk1.6.0/jre/lib
./jdk1.6.0/jre/lib/amd64
./jdk1.6.0/jre/lib/rt.jar
./jdk1.6.0/jre/lib/jsse.jar
./jdk1.6.0/include
./jdk1.6.0/include/jvmti.h
./jdk1.6.0/include/classfile_constants.h
./jdk1.6.0/include/jvmticmlr.h
./jdk1.6.0/include/linux
./jdk1.6.0/include/jawt.h
./jdk1.6.0/include/jni.h
./jdk1.6.0/include/jdwpTransport.h


>>   	ln -sf $(JAR) $(BOOT_DIR)/bin/jar
>>   	ln -sf $(abs_top_builddir)/javac $(BOOT_DIR)/bin/javac
>>   	ln -sf $(JAVADOC) $(BOOT_DIR)/bin/javadoc
>> 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.

Yes, that should be AC_SUBST (I wonder how it worked when I was testing 
it...).

>> 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?

Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pac-support-08.patch
Type: text/x-patch
Size: 75458 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110225/3d6a2ae3/pac-support-08.patch