Modify

Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#17170 closed enhancement (fixed)

[Patch] Migrate TagInfoExtract.groovy to Java

Reported by: simon04 Owned by: simon04
Priority: normal Milestone: 19.01
Component: Core Version:
Keywords: groovy Cc: Don-vip, michael2402

Description

One Groovy script less. I spent the evening migrating this overly long Groovy script to Java. Subsequent refactoring might be advisable.

I started using JOSM's OptionParser; thus, the CLI might be different slightly. For automatic compilation w/o adapting the build scripts, I put the new class in org.openstreetmap.josm.tools.TagInfoExtract (to be discussed).

Attachments (1)

17170.patch (45.1 KB) - added by simon04 3 months ago.

Download all attachments as: .zip

Change History (16)

Changed 3 months ago by simon04

Attachment: 17170.patch added

comment:1 Changed 3 months ago by Don-vip

Wow, great! I never found the courage to convert it :) However I'm not a fan to include it in the utils package. We can keep it in the scripts directory, it doesn't prevent IDEs to compile it. And Jenkins runs this script at each build.

comment:2 Changed 3 months ago by simon04

Resolution: fixed
Status: assignedclosed

In 14634/josm:

fix #17170 - Migrate TagInfoExtract.groovy to Java

comment:3 Changed 3 months ago by simon04

In 14635/josm:

see #17170 - Refactoring of TagInfoExtract

comment:4 Changed 3 months ago by simon04

In 14636/josm:

see #17170 - Adapt build.xml to compile+run TagInfoExtract

comment:5 Changed 3 months ago by simon04

In 14639/josm:

see #17170 - Update README

comment:6 Changed 3 months ago by GerdP

Not sure if this should work on my PC? I tried

ant clean dist taginfo

and got this

...
dist:
     [echo] Revision 14642
   [delete] Deleting: C:\josm\core\dist\josm-custom.jar
      [jar] Building jar: C:\josm\core\dist\josm-custom.jar

taginfo-compile:
    [javac] Compiling 1 source file to C:\josm\core\build2

taginfo:
     [echo] Generating Taginfo for type mappaint to taginfo_style.json

BUILD FAILED
C:\josm\core\build.xml:860: The following error occurred while executing this line:
C:\josm\core\build.xml:842: java.nio.file.InvalidPathException: Illegal char <:> at index 8: way_addr:interpolation=odd.png
        at sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
        at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
        at sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
        at sun.nio.fs.WindowsPath.parse(WindowsPath.java:94)
        at sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:255)
        at sun.nio.fs.AbstractPath.resolve(AbstractPath.java:53)
        at TagInfoExtract$StyleSheet$Checker.createImage(TagInfoExtract.java:417)
        at TagInfoExtract$StyleSheet$WayChecker.findUrl(TagInfoExtract.java:474)
        at TagInfoExtract$StyleSheet.lambda$convertStyleSheet$2(TagInfoExtract.java:358)
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
        at java.util.stream.DistinctOps$1$2.accept(DistinctOps.java:175)
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
        at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
        at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
        at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:580)
        at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:270)
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
        at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
        at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
        at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
        at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
        at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
        at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
        at TagInfoExtract$StyleSheet.convertStyleSheet(TagInfoExtract.java:374)
        at TagInfoExtract$StyleSheet.run(TagInfoExtract.java:320)
        at TagInfoExtract.main(TagInfoExtract.java:102)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.apache.tools.ant.taskdefs.ExecuteJava.run(ExecuteJava.java:218)
        at org.apache.tools.ant.taskdefs.ExecuteJava.execute(ExecuteJava.java:153)
        at org.apache.tools.ant.taskdefs.Java.run(Java.java:833)
        at org.apache.tools.ant.taskdefs.Java.executeJava(Java.java:227)
        at org.apache.tools.ant.taskdefs.Java.executeJava(Java.java:136)
        at org.apache.tools.ant.taskdefs.Java.execute(Java.java:109)
        at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:293)
        at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
        at org.apache.tools.ant.Task.perform(Task.java:348)
        at org.apache.tools.ant.taskdefs.Sequential.execute(Sequential.java:68)
        at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:293)
        at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
        at org.apache.tools.ant.Task.perform(Task.java:348)
        at org.apache.tools.ant.taskdefs.MacroInstance.execute(MacroInstance.java:396)
        at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:293)
        at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
        at org.apache.tools.ant.Task.perform(Task.java:348)
        at org.apache.tools.ant.Target.execute(Target.java:435)
        at org.apache.tools.ant.Target.performTasks(Target.java:456)
        at org.apache.tools.ant.Project.executeSortedTargets(Project.java:1405)
        at org.apache.tools.ant.Project.executeTarget(Project.java:1376)
        at org.apache.tools.ant.helper.DefaultExecutor.executeTargets(DefaultExecutor.java:41)
        at org.apache.tools.ant.Project.executeTargets(Project.java:1260)
        at org.apache.tools.ant.Main.runBuild(Main.java:854)
        at org.apache.tools.ant.Main.startAnt(Main.java:236)
        at org.apache.tools.ant.launch.Launcher.run(Launcher.java:285)
        at org.apache.tools.ant.launch.Launcher.main(Launcher.java:112)

Total time: 1 minute 21 seconds

C:\josm\core>

comment:7 Changed 3 months ago by simon04

Presumably this script and its generated image filenames have never worked on Windows. Possible colons : are not escaped and thus invalid on Windows filesystems. The corresponding image from the stacktrace is https://josm.openstreetmap.de/download/taginfo/taginfo-img/way_addr:interpolation=odd.png

comment:8 Changed 3 months ago by GerdP

I wanted to check the results because it seems to be what we need to finish #17055. Do you plan to make it work on Windows? Or do you want me to do that?

comment:9 Changed 3 months ago by simon04

Please go ahead. Since I don't have a Windows development machine at hand, testing the changes is somehow cumbersome.

Providing Windows compatibility might be as simple as masking invalid NTFS characters (maybe taking care of : suffices).

comment:10 Changed 3 months ago by GerdP

OK, working on it. There are more problems, directories are missing, temp. files are not removed. All typical for unix-like programs executed on Windows ;)

comment:11 Changed 3 months ago by GerdP

Hmm, I think the program will not work on unix either unless you create directory taginfo-img manually or specify it with option --imgdir ?
Not sure if windows supports tmpdir.toFile().deleteOnExit(). In general it refuses to delete non-empty directories and it will also not delete files which are not yet closed. Closing a file can take seconds :(
Do we already have code in JOSM to handle that better?

Besides that it seems enough to add e.g.

imageName = imageName.replaceAll(":", "-");

to make it work on Windonws.
and the output doesn't seem to help with #17055. That ticket would need a simple list of all often used values (e.g. > 1000 times) for all tag keys present in the presets.

comment:12 Changed 3 months ago by simon04

In 14651/josm:

see #17170 - TagInfoExtract: create imageDir

comment:13 Changed 3 months ago by simon04

@Don-vip: Does Jenkins or the server configuration need to be updates? It seems that the script did not run yesterday/today, see https://taginfo.openstreetmap.org/projects/josm_main_mappaint_style.

comment:14 Changed 3 months ago by Don-vip

Yep, good catch:

cd /tmp/josm/josm && DISPLAY= groovy -cp /tmp/josm/josm-snapshot-`cat /tmp/josm/revision`.jar:/tmp/josm/josm/tools/spotbugs/spotbugs-annotations.jar scripts/TagInfoExtract.groovy -t mappaint --svnweb --imgurlprefix https://josm.openstreetmap.de/download/taginfo/taginfo-img -o taginfo_style.json
Caught: java.io.FileNotFoundException: /tmp/josm/josm/scripts/TagInfoExtract.groovy (/tmp/josm/josm/scripts/TagInfoExtract.groovy)
java.io.FileNotFoundException: /tmp/josm/josm/scripts/TagInfoExtract.groovy (/tmp/josm/josm/scripts/TagInfoExtract.groovy)
Makefile:185: recipe for target 'taginfo_nodep' failed
make: *** [taginfo_nodep] Error 1

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain simon04.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.