root @5.34.12: build fails with graphviz-devel

[ryandesign (Ryan Carsten Schmidt)]

root @5.34.12 builds fine with graphviz @2.34.0 but fails with graphviz-devel @2.35.20131119.2140:

/usr/bin/clang++ -O2 -m64 -pipe -Wshadow -W -Wall -Woverloaded-virtual -fsigned-char -fno-common -Iinclude -DR__HAVE_CONFIG   -std=c++11 -stdlib=libc++  -pthread -I/opt/local/include/graphviz  -o graf2d/gviz/src/TGraphEdge.o -c /opt/local/var/macports/build/_Users_rschmidt_macports_dports_science_root/root/work/root/graf2d/gviz/src/TGraphEdge.cxx
/opt/local/var/macports/build/_Users_rschmidt_macports_dports_science_root/root/work/root/graf2d/gviz/src/TGraphEdge.cxx:87:17: error: no matching function for call to 'agedge'
      fGVEdge = agedge(gv, n1, n2);
                ^~~~~~
/opt/local/include/graphviz/cgraph.h:286:18: note: candidate function not viable: requires 5 arguments, but 3 were provided
extern Agedge_t *agedge(Agraph_t * g, Agnode_t * t, Agnode_t * h,
                 ^
1 error generated.

[cjones051073 (Chris Jones)]

Is this new with 5.34.12, or did previous ROOT versions also fail ?

[cjones051073 (Chris Jones)]

Also, is there a known backwards incompatible change in the API of graphviz, with the 2.35 development version, w.r.t. 2.34 ? Basically is this a bug in graphviz, that should be reported to them upstream, or is this something clients of graphviz are going to have to adapt to ? If it is a planned API change, I would have hope they would change the major release number, so I am guessing(hoping) this is a graphviz bug...

[mojca (Mojca Miklavec)]

All I can say is that I have a feeling that the libraries have changed from lib/libgraph.dylib to lib/libcgraph.dylib because I had to fix the Portfile for CMake-based installation to make graphviz work in the first place, but that's already the case in graphviz 2.34 and shouldn't make a difference in 2.35 unless 2.34 contains a compatibility code and 2.35 doesn't.

See:

• ​http://stackoverflow.com/questions/16003343/problems-using-graphviz-as-a-library
• ​http://stackoverflow.com/questions/19589801/graphviz-as-library-error-too-many-arguments-to-function-agnode

My general impression was that the syntax has probably been changed indeed, but I didn't find any documentation about that yet.

[mojca (Mojca Miklavec)]

I submitted an upstream bug report:

• ​https://sft.its.cern.ch/jira/browse/ROOT-5784

For me it works if I add -DWITH_CGRAPH to the end of compile command which probably means that adding this to CXXFLAGS should be a viable workaround.

My only question is: should graphviz & graphviz-devel be two separate options of the port? That is: are the libraries compatible with each other or not?

[mojca (Mojca Miklavec)]

Ryan, if this is a problem for you, feel free to add -DWITH_CGRAPH to CXXFLAGS, test (I didn't) and commit the change (without revbump) if Chris agrees. It turns out that graphviz removed all references to that variable one month ago, see:

• ​https://github.com/ellson/graphviz/commits/master/lib/common/types.h.in

and ROOT used that variable to determine whether to treat graphviz version as "old" or "new":

void TGraphEdge::CreateGVEdge(Agraph_t *gv)
{
   // Create the GraphViz edge into the GraphViz data structure gv.           

   if (gv) {
      Agnode_t *n1 = fNode1->GetGVNode();
      Agnode_t *n2 = fNode2->GetGVNode();
#ifdef WITH_CGRAPH
      fGVEdge = agedge(gv, n1, n2, NULL, 1);
#else
      fGVEdge = agedge(gv, n1, n2);
#endif
   } else {
      Error("CreateGVEdge","Invalid graphviz graph");
   }
}

Up to the version from one month ago it would use the first definition and with the latest version it would fall back to the old one.

The ROOT developers are working on this, see my reference to the upstream report above.

(My point about whether graphviz and graphviz-devel are ABI-compatible is still valid.)

[mojca (Mojca Miklavec)]

Update: ROOT developers decided to wait what the final version of graphviz might bring and not to change anything right now. I would suggest asking the graphviz developers whether they are willing to add #define WITH_CGRAPH 1 back to types.h. That would be by far the easiest solution.

[cjones051073 (Chris Jones)]

Will the ROOT devs follow this up themselves with the graphviz devs ?

[ryandesign (Ryan Carsten Schmidt)]

I have filed a Graphviz ticket: ​http://www.graphviz.org/mantisbt/view.php?id=2404

[erg@…]

For the record, WITH_CGRAPH has been the default since 2.30, and the intention was to jettison dependencies on the older graph library. Indeed, some recent features only have a cgraph version. The change in 2.35 just removed any non-cgraph code.

We will be putting the #define back in, as that seems simplest, but developers should assume that the cgraph API is the one to use.

[mojca (Mojca Miklavec)]

Cc Me!

[mojca (Mojca Miklavec)]

Thank you, "erg".

Ryan, I didn't test yet, but if you upgrade graphviz-devel now, the problem should go away.

[ryandesign (Ryan Carsten Schmidt)]

Yes, it's fixed as of graphviz-devel @2.35.20131224.0545.