handle non ascii classpath with system classloader

[astrelsky]

fixes #1194 fixes #1222

Note that the encoding problem appears to effect anything that isn't ascii. I tested the ParseUtil with the unicode_à😎 right in Java code (utf-8) and it didn't encode the url correctly.

[astrelsky]

I'm not sure what the deal is with the remaining failed tests.

Is there something additional that needs to be done to get the new java test classes into the test runner?

[Thrameos]

I will give it a look though it is looking like Wednesday at best. Lost 16 hours to a network problem 4 days before my critical project demo. So I have a heavy workload this weekend.

[marscher]

I will give it a look though it is looking like Wednesday at best. Lost 16 hours to a network problem 4 days before my critical project demo. So I have a heavy workload this weekend.

I wish you best of strength and endurance, and of course a good demo! I will have a look as wel. Maybe we can sort this out already.

[marscher]

@astrelsky I turned on trace logging of all JVM subsystems and stumbled upon this one:
[0.459s][info ][exceptions ] Exception <a 'java/io/FileNotFoundException'{0x000000008ba44ca8}: /workspaces/jpype/test/test/classes;test/jar/late/late.jar>
This actually looks like the paths are not properly being split prior passing it to open(), what do you think?

It is also confusing that a semicolon is used as path separator on Linux, right? It should be a colon!

[astrelsky]

@astrelsky I turned on trace logging of all JVM subsystems and stumbled upon this one:
[0.459s][info ][exceptions ] Exception <a 'java/io/FileNotFoundException'{0x000000008ba44ca8}: /workspaces/jpype/test/test/classes;test/jar/late/late.jar>
This actually looks like the paths are not properly being split prior passing it to open(), what do you think?

It is also confusing that a semicolon is used as path separator on Linux, right? It should be a colon!

Oh, that's interesting. I'll take a quick look maybe it's something stupid

[astrelsky]

@marscher I think I know what happened, thank you! I must have copied the classpath from the testAddClassPath run and not seen the semicolon when setting the arguments in the new test.

But now, how could the mac tests have passed? As far as I know windows is the outlier and the only one that uses ;.

[codecov]

Codecov Report

Attention: Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.18%. Comparing base (002c616) to head (d86aa67).
Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
jpype/_core.py	97.22%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1226      +/-   ##
==========================================
+ Coverage   87.17%   87.18%   +0.01%     
==========================================
  Files         113      113              
  Lines       10267    10287      +20     
  Branches     4045     4045              
==========================================
+ Hits         8950     8969      +19     
- Misses        725      726       +1     
  Partials      592      592              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[marscher]

Maybe the mac did something clever - I cannot tell, dont use that stuff. Thanks for fixing it!
@Thrameos does this solution fit our needs?

[Thrameos]

Overall I like the changes. If we don't care about the behavior of a service then I woudl approve.

If we do care about the minor differences between before and after the JVM is started, then we would need to find the appropriate test case.

My vote would be to skip it for now and wait for a user report. We are definitely better having some support, and we can add the rest once someone locates the problematic test case.

[Thrameos]

Should we pass the class path as a different variable and then use it. The issue I am concerned about is that of those jars that specify a service.

https://medium.com/@yureshcs/services-and-service-provider-with-java-9-modules-ddd0170749b0

In that past when we did testing is was clear that services were only recognized if they were loaded on the initial class_path. I believe it is the case for certain things like databases. Unfortunately, it was a while since I last looked at the issue so I don't have a working test case.

[astrelsky]

Should we pass the class path as a different variable and then use it. The issue I am concerned about is that of those jars that specify a service.

https://medium.com/@yureshcs/services-and-service-provider-with-java-9-modules-ddd0170749b0

In that past when we did testing is was clear that services were only recognized if they were loaded on the initial class_path. I believe it is the case for certain things like databases. Unfortunately, it was a while since I last looked at the issue so I don't have a working test case.

They are still loaded on the initial classpath (loaded by the system class loader). I can figure out how to put together a service and add a test case for it tomorrow.

Services are loaded and instantiated lazily. So as long as the paths are added to the system classpath before returning from startJVM, before the user can do anything, then we should be ok.

[Thrameos]

Is this ready to go?

[astrelsky]

Is this ready to go?

Yes. Services work as expected so I think everything is good if you don't have any other objections.

I thought I had left a comment after I made the test but it looks like I didn't. I happened to find a simple test case for a service.

[Thrameos]

Thanks for the hard work on this. I will trigger test and release scripts in my next open session.

[astrelsky]

Thanks for the hard work on this. I will trigger test and release scripts in my next open session.

Sounds good, thank you.

[astrelsky]

@Thrameos I don't mean to be a pain but would #1224 be included? I only ask because without it building it in free threaded mode will fail with a compile error or unexpectedly build the normal gil mode wheel.

[Thrameos]

Sure I see no reason to exclude it.