plantuml / plantuml

Generate diagrams from textual description
https://plantuml.com
Other
9.73k stars 881 forks source link

`GraphvizVersionFinder` fails to parse version for graphviz 3.0.0 #940

Closed carlocab closed 1 year ago

carlocab commented 2 years ago

Hi there, we're trying to update Homebrew's version of graphviz at Homebrew/homebrew-core#95933.

While testing packages that depend on graphviz, we encountered a failure in plantuml. Trying to run plantuml -testdot appears to fail because it is unable to parse the new version scheme.

This failure seems to come from here:

https://github.com/plantuml/plantuml/blob/87d472f1228e8da7f00ccc873030fe2077e9322a/src/net/sourceforge/plantuml/cucadiagram/dot/GraphvizVersionFinder.java#L75

We'd greatly appreciate it if a new jar could be published that fixes this.

arnaudroques commented 2 years ago

Ok, so we have pushed a new snapshot that should parse 3.0.0.

Having said that, we have never tested PlantUML with Graphviz 3.0 so we have no idea if this will be working. You will tell us :-)

cho-m commented 2 years ago

Can you also update these lines: https://github.com/plantuml/plantuml/blob/d38782814758038df5c52a7f8c20872b853e5b33/src/net/sourceforge/plantuml/cucadiagram/dot/GraphvizUtils.java#L198-L203

carlocab commented 2 years ago

This patch seems to work:

diff --git a/src/net/sourceforge/plantuml/cucadiagram/dot/GraphvizUtils.java b/src/net/sourceforge/plantuml/cucadiagram/dot/GraphvizUtils.java
index faebb12..95f0696 100644
--- a/src/net/sourceforge/plantuml/cucadiagram/dot/GraphvizUtils.java
+++ b/src/net/sourceforge/plantuml/cucadiagram/dot/GraphvizUtils.java
@@ -195,12 +195,12 @@ public class GraphvizUtils {
        if (s == null) {
            return -1;
        }
-       final Pattern p = Pattern.compile("\\s([12].\\d\\d)\\D");
+       final Pattern p = Pattern.compile("\\s([123])\\.(\\d\\d?)\\D");
        final Matcher m = p.matcher(s);
        if (m.find() == false) {
            return -1;
        }
-       return Integer.parseInt(m.group(1).replaceAll("\\.", ""));
+       return 100 * Integer.parseInt(m.group(1)) + Integer.parseInt(m.group(2));
    }

    public static int getDotVersion() throws IOException, InterruptedException {

Our testing is admittedly not very thorough, however. It boils down to plantuml -testdot. Does this actually do anything meaningful with dot, or does it just check that it can find dot and make sense of its version?

arnaudroques commented 2 years ago

Thanks for your patch. You can try last snapshot. It should be fine.

chenrui333 commented 1 year ago

@arnaudroques actually running into the issue again with graphviz 4.0.0 , can you create another patch? Thanks!

arnaudroques commented 1 year ago

Here is another patch. Tell us if last snapshot is not working. Thanks

chenrui333 commented 1 year ago

@arnaudroques The patch works for me, thanks!

chenrui333 commented 1 year ago

@arnaudroques we ran into the issue again, proposing a more permanent solution with #1164

arnaudroques commented 1 year ago

Out of curiosity, which version of GraphViz are you using?

carlocab commented 1 year ago

We're trying to upgrade to GraphViz 7.0.0, but are currently unable to ship it because it breaks the plantuml package that we ship.