Build this as `libonnx`

[hmaarrfk]

Comment:

It would help the pytorch builds if we had this built out as libonnx in addition to just the python-onnx package.

I can build the library, but I'm not sure how to ask python to use it.

A demo can be seen here:
https://github.com/conda-forge/staged-recipes/pull/19103/files#diff-a427d91b1af6c576ce174b36e369cc1dba4370eb5b9b6529fc19a775f0a0ac9b

[sshockwave]

If I read it correctly, this is where it links onnx to python. Switching CMake to shared lib builds should be able to expose the library while letting python link to it.

[hmaarrfk]

Gathering a bit of information:

From @traversaro on #105 (comment)

I don't really understand what I am doing wrong on windows..... help appreciated.

ONNX_API is used as export macro for both onnx_proto and onnx libraries. So, onnx_proto symbols get marked with __declspec(dllexport) also in onnx's compilation units, and so the linker complains that they are not found even if they are present in the linked import librar onnx_proto.lib .

A quick fix (I just tested locally, it seems to work) is to change the export macro for onnx-proto : traversaro/onnx@e7719fd . It is not ideal as non-CMake downstream users needs to manually set ONNX_PROTO_API , but better than not working. A better fix is to define ONNX_PROTO_API in an header, but unfortunately protobuf does not support including an arbitrary header inclusion in the generated headers, so downstream projects typically do that via external scripts, see protocolbuffers/protobuf#4198 for more details. If it is not a problem to expose all symbols of onnx_proto , an alternative simple solution is to set the WINDOWS_EXPORT_ALL_SYMBOLS CMake definition just for onnx_proto, and avoid to use any export pre-processor macro.

e7719fd89c0bd12bc54983efeeeee5d926a07563.patch.txt

[hmaarrfk]

I think I got shared libraries on unix, and static libraries on windows in #112

For shared libraries on windows

I found that in my research onnx created a switch for downstream packages to "patch" things. Not sure how long this patch will last, but pytorch uses it:

https://github.com/pytorch/pytorch/blob/e90cddb0d350df3b0447d6313da80d8676bdd915/cmake/Dependencies.cmake#L1568
https://github.com/pytorch/pytorch/blob/main/cmake/ProtoBufPatch.cmake

But I'm somewhat skeptical That it will last too long.

ChatGPT Helped me write:

file(READ ${FILENAME} content)

string(REGEX REPLACE
    "#include \"google/protobuf/port_def.inc\""
    "#include \"google/protobuf/port_def.inc\"\n\n#ifdef BUILD_DLL\n#define {ONNX_PROTO_API} __declspec(dllexport)\n#else\n#define {ONNX_PROTO_API} __declspec(dllimport)\n#endif\n"
    content
    "${content}"
)

file(WRITE ${FILENAME} "${content}")

I think it should work to include a header so that users don't need to always define things. I think we can add the visibility default stuff too make it work with unix, then we would have a single "thing" that gets done.

it also wouldn't be impossible to offer it to upstream.