conda-forge / onnxruntime-feedstock

A conda-smithy repository for onnxruntime.
BSD 3-Clause "New" or "Revised" License
1 stars 19 forks source link

De-vendor vendored dependencies #57

Open traversaro opened 1 year ago

traversaro commented 1 year ago

Comment:

Hello @conda-forge/onnxruntime ! I am interested in trying to fix https://github.com/conda-forge/onnxruntime-feedstock/issues/31, and if you think it may be useful become mantainer, to handle possible future Windows problems.

I started trying to tackle the Windows compilation in https://github.com/conda-forge/onnxruntime-feedstock/pull/56, but I soon realized that a possible source of problem is the various packages that are vendored by the package. So a possible way forward is to first work on de-vendoring the various dependencies currently used by the package, and then work on the Windows port. Are you ok with that plan, or you see possible problems?

For what regard de-vendoring, I saw some interesting work done on the conan side in https://github.com/conan-io/conan-center-index/pull/16849 and merged upstream in https://github.com/microsoft/onnxruntime/pull/15323 (thanks a lot and kudos to @mayeut), so a possible way forward is to adapt those patches for conda-forge .

cbourjau commented 1 year ago

Thank you very much @traversaro for looking into this and apologize for the late response: I was on holiday. A lot has changed upstream in the build-setup since #31. Most importantly, most dependencies are now fetched via cmake's FetchContent module. While not pretty/pure it actually makes packaging easier if we just accept it. Onnxruntime has been very picky with dependency versions in the past. If we de-vendor the dependencies I expect that we will have to pin quite a lot of them explicitly and maintain those pins with each release. If upstream indeed manages to get their dependencies up to date and in-tree we can consider de-vendoring.

In the meantime, it would be great to get Windows working using the vendored dependencies. I don't have experience with packaging for windows but I think this should also work with protobuf, too, but @xhochy might have much better insights on this regard.

traversaro commented 1 year ago

Thanks for the reply @cbourjau and sorry for the radio silence! I was asking about de-vendoring as in the past I had quite a lot of issues due to incompatible versions of protobuf in libraries installed via PyPI (tensorflow and opencv, in particular) and one of the reasons I use conda-forge is exactly to avoid this kind of problems. However, I totally understand your point on easy of mantainance. As suggested by you I will first try to package windows without any de-vendoring. After that, if it is ok for you, I would try at least to de-vendor some particular problematic dependency (protobuf, in my experience) and see if we can do that without introducing too much complexity.