Skip to content

Commit

Permalink
Merge pull request #54 from robgjansen/fix-build-igraph
Browse files Browse the repository at this point in the history
Fix build for igraph v0.10.x

This fixes the tgen build on systems with igraph library versions >= 0.10.0. We no longer need to use workarounds to install older version from backports or previous release repositories.

The general strategy here was to implement the tgen code according to the newer igraph api changes from v0.10.x, and then set up the messy compat definitions in tgen-igraph-compat.h that we can eventually just delete if we ever decide we no longer want to support igraph versions older than v0.10.0.

We also stop setting the C attribute table multiple times during program execution, and just sets it once at program startup as explained in #44 (comment).

Fixes #44
  • Loading branch information
robgjansen authored Dec 30, 2023
2 parents 827cdca + 8653552 commit 1116e63
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 81 deletions.
28 changes: 1 addition & 27 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,6 @@ jobs:
- 'debian:12-slim'
- 'fedora:37'
steps:
# See https://github.com/shadow/tgen/issues/44#issuecomment-1693982909
- name: Install debian:11 backports
if: ${{ matrix.container == 'debian:12-slim' }}
run: |
cat <<EOF > /etc/apt/sources.list.d/bullseye.sources
Types: deb
URIs: http://deb.debian.org/debian
Suites: bullseye bullseye-updates
Components: main
Signed-By: /usr/share/keyrings/debian-archive-keyring.gpg
Types: deb
URIs: http://deb.debian.org/debian-security
Suites: bullseye-security
Components: main
Signed-By: /usr/share/keyrings/debian-archive-keyring.gpg
EOF
- name: Update packages
run: |
case ${{ matrix.container }} in
Expand All @@ -75,14 +57,6 @@ jobs:
libglib2.0-dev \
libigraph-dev
;;
debian:12*)
# See https://github.com/shadow/tgen/issues/44#issuecomment-1693982909
apt-get install -y \
${{ matrix.cc }} \
cmake \
libglib2.0-dev \
libigraph-dev/bullseye
;;
debian*)
apt-get install -y \
${{ matrix.cc }} \
Expand Down Expand Up @@ -175,7 +149,7 @@ jobs:
python3 -m venv build/toolsenv
source build/toolsenv/bin/activate
# Drastically improves time to install requirements on platforms with
# older pip in system backages, such as debian-10.
# older pip in system packages, such as debian-10.
pip3 install --upgrade pip
pip3 install -r tools/requirements.txt
Expand Down
7 changes: 1 addition & 6 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@ include(FindPkgConfig)
find_package(RT REQUIRED)
find_package(M REQUIRED)

# We don't yet support igraph 0.10.0+, which has some API changes.
# See https://github.com/shadow/tgen/issues/44
#
# Older versions of cmake don't support the `<` operator here; only `<=`.
pkg_check_modules(IGRAPH REQUIRED igraph<=0.9.9999)

pkg_check_modules(IGRAPH REQUIRED igraph)
pkg_check_modules(GLIB REQUIRED glib-2.0)

## Parse out igraph version. Needed to work around breaking API changes in igraph.
Expand Down
79 changes: 37 additions & 42 deletions src/tgen-graph.c
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,8 @@ static GError* _tgengraph_parseGraphEdges(TGenGraph* g) {
if(!error) {
g->edgeCount = igraph_ecount(g->graph);
if(g->edgeCount != edgeCount) {
tgen_warning("igraph_vcount %d does not match iterator count %d", g->edgeCount, edgeCount);
tgen_warning("igraph_vcount %ld does not match iterator count %ld",
(long int) g->edgeCount, (long int) edgeCount);
}

tgen_info("%u graph edges ok", (guint) g->edgeCount);
Expand All @@ -416,15 +417,15 @@ static gboolean _tgengraph_hasSelfLoop(TGenGraph* g, igraph_integer_t vertexInde
TGEN_ASSERT(g);
gboolean isLoop = FALSE;

igraph_vector_t* resultNeighborVertices = g_new0(igraph_vector_t, 1);
gint result = igraph_vector_init(resultNeighborVertices, 0);
igraph_vector_int_t* resultNeighborVertices = g_new0(igraph_vector_int_t, 1);
gint result = igraph_vector_int_init(resultNeighborVertices, 0);

if(result == IGRAPH_SUCCESS) {
result = igraph_neighbors(g->graph, resultNeighborVertices, vertexIndex, IGRAPH_OUT);
if(result == IGRAPH_SUCCESS) {
glong nVertices = igraph_vector_size(resultNeighborVertices);
glong nVertices = igraph_vector_int_size(resultNeighborVertices);
for (gint i = 0; i < nVertices; i++) {
igraph_integer_t dstVertexIndex = igraph_vector_e(resultNeighborVertices, i);
igraph_integer_t dstVertexIndex = igraph_vector_int_get(resultNeighborVertices, i);
if(vertexIndex == dstVertexIndex) {
isLoop = TRUE;
break;
Expand All @@ -433,7 +434,7 @@ static gboolean _tgengraph_hasSelfLoop(TGenGraph* g, igraph_integer_t vertexInde
}
}

igraph_vector_destroy(resultNeighborVertices);
igraph_vector_int_destroy(resultNeighborVertices);
g_free(resultNeighborVertices);
return isLoop;
}
Expand All @@ -442,10 +443,10 @@ static glong _tgengraph_countIncomingEdges(TGenGraph* g, igraph_integer_t vertex
/* Count up the total number of incoming edges */

/* initialize a vector to hold the result neighbor vertices for this action */
igraph_vector_t* resultNeighborVertices = g_new0(igraph_vector_t, 1);
igraph_vector_int_t* resultNeighborVertices = g_new0(igraph_vector_int_t, 1);

/* initialize with 0 entries, since we dont know how many neighbors we have */
gint result = igraph_vector_init(resultNeighborVertices, 0);
gint result = igraph_vector_int_init(resultNeighborVertices, 0);
if(result != IGRAPH_SUCCESS) {
tgen_critical("igraph_vector_init return non-success code %i", result);
g_free(resultNeighborVertices);
Expand All @@ -456,17 +457,17 @@ static glong _tgengraph_countIncomingEdges(TGenGraph* g, igraph_integer_t vertex
result = igraph_neighbors(g->graph, resultNeighborVertices, vertexIndex, IGRAPH_IN);
if(result != IGRAPH_SUCCESS) {
tgen_critical("igraph_neighbors return non-success code %i", result);
igraph_vector_destroy(resultNeighborVertices);
igraph_vector_int_destroy(resultNeighborVertices);
g_free(resultNeighborVertices);
return -1;
}

/* handle the results */
glong totalIncoming = igraph_vector_size(resultNeighborVertices);
glong totalIncoming = igraph_vector_int_size(resultNeighborVertices);
tgen_debug("found %li incoming 1-hop neighbors to vertex %li", totalIncoming, (glong)vertexIndex);

/* cleanup */
igraph_vector_destroy(resultNeighborVertices);
igraph_vector_int_destroy(resultNeighborVertices);
g_free(resultNeighborVertices);

return totalIncoming;
Expand Down Expand Up @@ -1068,10 +1069,11 @@ static GError* _tgengraph_parseGraphVertices(TGenGraph* g) {
if(!error) {
g->vertexCount = igraph_vcount(g->graph);
if(g->vertexCount != vertexCount) {
tgen_warning("igraph_vcount %d does not match iterator count %d", g->vertexCount, vertexCount);
tgen_warning("igraph_vcount %ld does not match iterator count %ld",
(long int) g->vertexCount, (long int) vertexCount);
}

tgen_info("%u graph vertices ok", (guint) g->vertexCount);
tgen_info("%ld graph vertices ok", (long int) g->vertexCount);
}

return error;
Expand All @@ -1091,10 +1093,10 @@ static GError* _tgengraph_parseGraphProperties(TGenGraph* g) {
"igraph_is_connected return non-success code %i", result);
}

result = igraph_clusters(g->graph, NULL, NULL, &(g->clusterCount), IGRAPH_WEAK);
result = igraph_connected_components(g->graph, NULL, NULL, &(g->clusterCount), IGRAPH_WEAK);
if(result != IGRAPH_SUCCESS) {
return g_error_new(G_MARKUP_ERROR, G_MARKUP_ERROR_PARSE,
"igraph_clusters return non-success code %i", result);
"igraph_connected_components return non-success code %i", result);
}

/* it must be connected */
Expand All @@ -1109,13 +1111,13 @@ static GError* _tgengraph_parseGraphProperties(TGenGraph* g) {

/* now check list of all attributes */
igraph_strvector_t gnames, vnames, enames;
igraph_vector_t gtypes, vtypes, etypes;
igraph_vector_int_t gtypes, vtypes, etypes;
igraph_strvector_init(&gnames, 25);
igraph_vector_init(&gtypes, 25);
igraph_vector_int_init(&gtypes, 25);
igraph_strvector_init(&vnames, 25);
igraph_vector_init(&vtypes, 25);
igraph_vector_int_init(&vtypes, 25);
igraph_strvector_init(&enames, 25);
igraph_vector_init(&etypes, 25);
igraph_vector_int_init(&etypes, 25);

result = igraph_cattribute_list(g->graph, &gnames, &gtypes, &vnames, &vtypes, &enames, &etypes);
if(result != IGRAPH_SUCCESS) {
Expand All @@ -1125,16 +1127,16 @@ static GError* _tgengraph_parseGraphProperties(TGenGraph* g) {

GError* error = NULL;
gint i = 0;
for(i = 0; !error && i < igraph_strvector_size(&gnames); i++) {
gchar* name = NULL;
igraph_strvector_get(&gnames, (glong) i, &name);

#ifdef DEBUG
for(i = 0; !error && i < igraph_strvector_size(&gnames); i++) {
const gchar* name = igraph_strvector_get(&gnames, (igraph_integer_t) i);
tgen_debug("found graph attribute '%s'", name);
}
#endif

for(i = 0; !error && i < igraph_strvector_size(&vnames); i++) {
gchar* name = NULL;
igraph_strvector_get(&vnames, (glong) i, &name);
const gchar* name = igraph_strvector_get(&vnames, (igraph_integer_t) i);

tgen_debug("found vertex attribute '%s'", name);

Expand All @@ -1150,8 +1152,7 @@ static GError* _tgengraph_parseGraphProperties(TGenGraph* g) {
}

for(i = 0; !error && i < igraph_strvector_size(&enames); i++) {
gchar* name = NULL;
igraph_strvector_get(&enames, (glong) i, &name);
const gchar* name = igraph_strvector_get(&enames, (igraph_integer_t) i);

tgen_debug("found edge attribute '%s'", name);

Expand All @@ -1167,11 +1168,11 @@ static GError* _tgengraph_parseGraphProperties(TGenGraph* g) {
}

igraph_strvector_destroy(&gnames);
igraph_vector_destroy(&gtypes);
igraph_vector_int_destroy(&gtypes);
igraph_strvector_destroy(&vnames);
igraph_vector_destroy(&vtypes);
igraph_vector_int_destroy(&vtypes);
igraph_strvector_destroy(&enames);
igraph_vector_destroy(&etypes);
igraph_vector_int_destroy(&etypes);

if(error) {
tgen_warning("failed to verify graph properties and attributes");
Expand Down Expand Up @@ -1271,9 +1272,6 @@ TGenGraph* tgengraph_new(gchar* path) {
* from multiple threads at the same time. this is not a problem when shadow
* uses dlmopen to get a private namespace for each plugin. */

/* use the built-in C attribute handler */
igraph_attribute_table_t* oldHandler = igraph_set_attribute_table(&igraph_cattribute_table);

g->graph = _tgengraph_loadNewGraph(g->graphPath);
if(!g->graph) {
error = g_error_new(G_MARKUP_ERROR, G_MARKUP_ERROR_PARSE,
Expand All @@ -1290,9 +1288,6 @@ TGenGraph* tgengraph_new(gchar* path) {
if(!error) {
error = _tgengraph_parseGraphVertices(g);
}

/* replace the old handler */
igraph_set_attribute_table(oldHandler);
}

if(error) {
Expand Down Expand Up @@ -1332,10 +1327,10 @@ GQueue* tgengraph_getNextActionIDs(TGenGraph* g, TGenActionID actionID) {
igraph_integer_t srcVertexIndex = (igraph_integer_t) actionID;

/* initialize a vector to hold the result neighbor vertices for this action */
igraph_vector_t* resultNeighborVertices = g_new0(igraph_vector_t, 1);
igraph_vector_int_t* resultNeighborVertices = g_new0(igraph_vector_int_t, 1);

/* initialize with 0 entries, since we dont know how many neighbors we have */
gint result = igraph_vector_init(resultNeighborVertices, 0);
gint result = igraph_vector_int_init(resultNeighborVertices, 0);
if(result != IGRAPH_SUCCESS) {
tgen_critical("igraph_vector_init return non-success code %i", result);
g_free(resultNeighborVertices);
Expand All @@ -1346,13 +1341,13 @@ GQueue* tgengraph_getNextActionIDs(TGenGraph* g, TGenActionID actionID) {
result = igraph_neighbors(g->graph, resultNeighborVertices, srcVertexIndex, IGRAPH_OUT);
if(result != IGRAPH_SUCCESS) {
tgen_critical("igraph_neighbors return non-success code %i", result);
igraph_vector_destroy(resultNeighborVertices);
igraph_vector_int_destroy(resultNeighborVertices);
g_free(resultNeighborVertices);
return NULL;
}

/* handle the results */
glong nVertices = igraph_vector_size(resultNeighborVertices);
glong nVertices = igraph_vector_int_size(resultNeighborVertices);
tgen_debug("found %li outgoing neighbors from vertex %li", nVertices, (glong)srcVertexIndex);

/* only follow one edge of all edges with the 'weight' attribute (do a weighted choice)
Expand All @@ -1364,7 +1359,7 @@ GQueue* tgengraph_getNextActionIDs(TGenGraph* g, TGenActionID actionID) {

for (gint i = 0; i < nVertices; i++) {
/* we have source, get destination */
igraph_integer_t dstVertexIndex = igraph_vector_e(resultNeighborVertices, i);
igraph_integer_t dstVertexIndex = igraph_vector_int_get(resultNeighborVertices, i);

TGenAction* nextAction = _tgengraph_getAction(g, dstVertexIndex);
if(!nextAction) {
Expand All @@ -1378,7 +1373,7 @@ GQueue* tgengraph_getNextActionIDs(TGenGraph* g, TGenActionID actionID) {
result = igraph_get_eid(g->graph, &edgeIndex, srcVertexIndex, dstVertexIndex, IGRAPH_DIRECTED, TRUE);
if(result != IGRAPH_SUCCESS) {
tgen_critical("igraph_get_eid return non-success code %i", result);
igraph_vector_destroy(resultNeighborVertices);
igraph_vector_int_destroy(resultNeighborVertices);
g_free(resultNeighborVertices);
g_queue_free(nextActions);
g_queue_free(chooseActions);
Expand Down Expand Up @@ -1425,7 +1420,7 @@ GQueue* tgengraph_getNextActionIDs(TGenGraph* g, TGenActionID actionID) {
}

/* cleanup */
igraph_vector_destroy(resultNeighborVertices);
igraph_vector_int_destroy(resultNeighborVertices);
g_free(resultNeighborVertices);
g_queue_free(chooseActions);
g_queue_free(chooseWeights);
Expand Down
23 changes: 22 additions & 1 deletion src/tgen-igraph-compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,30 @@
#ifndef TGEN_IGRAPH_COMPAT_H_
#define TGEN_IGRAPH_COMPAT_H_

/* Renamed in in igraph 0.9.0 */
/* Renamed in igraph 0.9.0 */
#if IGRAPH_VERSION_MAJOR==0 && IGRAPH_VERSION_MINOR<9
#define igraph_set_attribute_table igraph_i_set_attribute_table
#endif

/* Changed in igraph 0.10.0 */
#if IGRAPH_VERSION_MAJOR == 0 && IGRAPH_VERSION_MINOR < 10
// Renamed
#define igraph_connected_components igraph_clusters
#define IGRAPH_ATTRIBUTE_UNSPECIFIED IGRAPH_ATTRIBUTE_DEFAULT
// Change to int types
#define igraph_vector_int_t igraph_vector_t
#define igraph_vector_int_init igraph_vector_init
#define igraph_vector_int_size igraph_vector_size
#define igraph_vector_int_get igraph_vector_e
#define igraph_vector_int_destroy igraph_vector_destroy
// Removed the name arg and return it instead
inline const char *igraph_strvector_get_compat(igraph_strvector_t *sv,
igraph_integer_t idx) {
char *name_ = NULL;
(igraph_strvector_get)(sv, idx, &name_);
return (const char *)name_;
}
#define igraph_strvector_get igraph_strvector_get_compat
#endif

#endif
4 changes: 4 additions & 0 deletions src/tgen-main.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <igraph.h>

#include "tgen.h"
#include "tgen-igraph-compat.h"

static void _tgenmain_cleanup(gint status, gpointer arg) {
if(arg) {
Expand Down Expand Up @@ -82,6 +83,9 @@ static gint _tgenmain_run(gint argc, gchar *argv[]) {
tgen_message("Set SIG_IGN for signal SIGPIPE");
}

/* use the built-in C attribute handler. this is set once and then left alone. */
igraph_set_attribute_table(&igraph_cattribute_table);

/* parse the config file */
TGenGraph* graph = tgengraph_new(argv[1]);
if (!graph) {
Expand Down
6 changes: 1 addition & 5 deletions src/tgen-markovmodel.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ static gboolean tgenmarkovmodel_attributeTypeMismatch(const igraph_t *graph, igr
tgen_warning("Internal error: igraph_cattribute_table.gettype missing; unable to validate attribute types");
return FALSE;
}
igraph_attribute_type_t type = IGRAPH_ATTRIBUTE_DEFAULT;
igraph_attribute_type_t type = IGRAPH_ATTRIBUTE_UNSPECIFIED;
if (igraph_cattribute_table.gettype(graph, &type, elemtype, name) != IGRAPH_SUCCESS) {
// The igraph documentation says it'll abort on any error, but in case
// it doesn't, error out here.
Expand Down Expand Up @@ -880,10 +880,6 @@ static igraph_t* _tgenmarkovmodel_loadGraph(FILE* graphFileStream, const gchar*
rewind(graphFileStream);

igraph_t* graph = g_new0(igraph_t, 1);

/* make sure we use the correct attribute handler */
igraph_set_attribute_table(&igraph_cattribute_table);

result = igraph_read_graph_graphml(graph, graphFileStream, 0);

if (result != IGRAPH_SUCCESS) {
Expand Down
5 changes: 5 additions & 0 deletions test/test-markovmodel.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
#include <stdint.h>

#include <glib.h>
#include <igraph.h>

#include "tgen-igraph-compat.h"
#include "tgen-log.h"
#include "tgen-markovmodel.h"

Expand Down Expand Up @@ -87,6 +89,9 @@ gint main(gint argc, gchar *argv[]) {
return EXIT_FAILURE;
}

/* use the built-in C attribute handler. this is set once and then left alone. */
igraph_set_attribute_table(&igraph_cattribute_table);

guint32 seed = (guint32)atoi(argv[1]);
gchar* path = g_strdup(argv[2]);
gchar* name = g_path_get_basename(path);
Expand Down

0 comments on commit 1116e63

Please sign in to comment.