[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: darcula and dependency libiconloader-java



tony mancill <tmancill@debian.org> writes:

> Hi Felix,

hello Tony,

thanks for looking into this.

> On Sat, Oct 19, 2019 at 08:13:34PM +0200, Felix Natter wrote:
>> hello Tony,
>> 
>> as discussed, here are two more dependencies of freeplane-1.7.10
>> (darcula and its dependency iconloader from the same author).
>> 
>> 1. https://salsa.debian.org/java-team/libiconloader-java
>> 
>> The upstream is at https://github.com/bulenkov/iconloader.git, which is
>> not active and has no releases, so I packaged git HEAD (@Markus:
>> unfortunately there is no alternative to this).
>> 
>> It uses a simple ant build system.
>> 
>> JDK11 does not have sun.reflect.ConstructorAccessor (at compile time),
>> so I replaced it with new String(...) (which is used by upstream if
>> ConstructorAccessor cannot be instantiated). Could you please review
>> this patch [1]?
>> I also disabled apple code (third patch).
>> 
>> [1] https://salsa.debian.org/java-team/libiconloader-java/blob/master/debian/patches/02_replace_ConstructorAccessor.patch
>
> In order to preserve the semantics of createShared(), maybe it's better
> to go ahead and instantiate an array of char[] and then return that cast
> to a String?  (I'm not sure what the original code was hoping to
> accomplish.)  Then the underlying storage is mutable if some application
> really expects the underlying bytes to be shared.  Or we could go with
> your patch and adjust if we encounter any problems.

I did it this way because this seemed to be the behavior of
createShared() when ourConstructorAccessor.newInstance fails to execute.

Now I see you refer to "backed by 'char' array".

But (from [1]):
String​(char[] value)
Allocates a new String so that it represents the sequence of characters
currently contained in the character array argument.
-> thus my solution is not correct.

"String.class.getDeclaredConstructor(char[].class, boolean.class)"
-> I cannot find the mentioned String(char[], boolean) constructor in [1].

[1] https://docs.oracle.com/javase/9/docs/api/java/lang/String.html

-> I don't know how the code can work, and how to make the patch
create a String backed by the array. A cast from char[] to String
does not work.

> One other comment about libiconloader-java...  It would be nice to add
> .idea to Files-Excluded in debian/copyright and regenerate the
> orig.tar.gz.  The .idea files can be regenerated IntelliJ if needed.

I've removed (repacked) them. We only package the L&F anyway so far.

Thanks and Best Regards,
-- 
Felix Natter
debian/rules!


Reply to: