Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multi-edges generate error on non-strict digraph #52

Closed
LeoAlex0 opened this issue Oct 21, 2021 · 4 comments · Fixed by #57
Closed

multi-edges generate error on non-strict digraph #52

LeoAlex0 opened this issue Oct 21, 2021 · 4 comments · Fixed by #57

Comments

@LeoAlex0
Copy link

Version

github.com/goccy/go-graphviz v0.0.9

Example Code

package main

import (
	"os"

	"github.com/goccy/go-graphviz"
)

func main() {
	graph := graphviz.New()
	g, _ := graph.Graph(graphviz.Directed, graphviz.Name("G"))

	n1, _ := g.CreateNode("1")
	n2, _ := g.CreateNode("2")
	g.CreateEdge("e1", n1, n2)
	g.CreateEdge("re1", n2, n1)

	g.CreateEdge("e2", n1, n2)
	g.CreateEdge("re2", n2, n1)

	graph.Render(g, graphviz.Format(graphviz.DOT), os.Stdout)
}

Real Output

digraph G {
	graph [bb="0,0,54,108"];
	node [label="\N"];
	1	 [height=0.5,
		pos="27,90",
		width=0.75];
	2	 [height=0.5,
		pos="27,18",
		width=0.75];
	1 -> 2 [key=e1,
	pos="e,21.105,35.593 21.084,72.202 20.28,64.181 20.057,54.523 20.416,45.596"];
2 -> 1 [key=re1,
pos="e,32.916,72.202 32.895,35.593 33.709,43.586 33.942,53.236 33.593,62.176"];
}

Expect

As the graphviz doc, a non-strict graph should allow multi-edges without merging them.

so it should output something like:

digraph G {
	graph [bb="0,0,54,108"];
	node [label="\N"];
	1	 [height=0.5,
		pos="27,90",
		width=0.75];
	2	 [height=0.5,
		pos="27,18",
		width=0.75];
	1 -> 2 [key=e1,
	pos="e,21.105,35.593 21.084,72.202 20.28,64.181 20.057,54.523 20.416,45.596"];
  1 -> 2 [key=e2,
	pos="e,21.105,35.593 21.084,72.202 20.28,64.181 20.057,54.523 20.416,45.596"];
2 -> 1 [key=re1,
pos="e,32.916,72.202 32.895,35.593 33.709,43.586 33.942,53.236 33.593,62.176"];
2 -> 1 [key=re2,
pos="e,32.916,72.202 32.895,35.593 33.709,43.586 33.942,53.236 33.593,62.176"];
}
@nevivurn
Copy link

nevivurn commented Oct 29, 2021

I was facing a similar issue, I believe this is a bug in upstream graphviz that has been fixed right after 2.40.1 (in dbe61e9fe), which is what this library uses.

I'm not too familiar with writing cgo wrappers, how hard would it be to update this library to use a more recent graphviz release? Also see #42.

@triztian
Copy link

triztian commented Mar 3, 2022

@goccy This is a wonderful package, thank you!

I've also encountered this issue and would like to take a jab at updating the GraphViz version. In #42 you'd mentioned providing a make update/graphviz/<VERSION> command. Is there any documentation of how to do it? I don't see a Makefile in the master branch, alternatively if you have any general instructions or pointers of how to the the migration manually I'd be happy to go over them.

Many thanks in advance!

@mcrakhman
Copy link

Hi @goccy! The library is great, but multi-edges fix would be greatly appreciated :-) Any news on updating graphviz or making Makefile available?

Thanks!

@ankon
Copy link
Contributor

ankon commented May 14, 2022

I'd also love to have multi-edges work, but it seems updating the cgraph library here might be a major project (probably first sync with 2.50.1, and then go to 3.0.0?)

But, for the fun of it I tried to simply apply the referenced change directly, and that works just fine.

git clone https://github.com/goccy/go-graphviz
cd go-graphviz
wget -qO - https://gitlab.com/graphviz/graphviz/-/commit/dbe61e9fe.diff | patch internal/ccall/cgraph/edge.c

And in your own project modify go.mod to use a replace.

@goccy would you accept a PR applying this change?

ankon added a commit to ankon/go-graphviz that referenced this issue May 14, 2022
This applies the diff of https://gitlab.com/graphviz/graphviz/-/commit/dbe61e9fe to make
it possible to create multiple edges between the same nodes as long as the edge names are
distinct.

Fixes goccy#52
ankon added a commit to ankon/explain-cloudformation-changeset that referenced this issue May 14, 2022
graphviz/cgraph allows to have multiple edges between the same node pair in a non-strict
graph, as long as the edges have distinct names. Unfortunately the version of graphviz go-graphviz
embeds has a bug in exactly the logic for looking up the edges, so this commit also switches
out the version to a fork that has a suitable fix for the problem.

See https://gitlab.com/graphviz/graphviz/-/issues/1191 for the original upstream problem, and
goccy/go-graphviz#52 for the issue in go-graphviz.

Fixes #6
@goccy goccy closed this as completed in #57 May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants