rpm-software-management / dnf5

Next-generation RPM package management system
Other
240 stars 79 forks source link

interface with Go programming language #719

Open malt3 opened 1 year ago

malt3 commented 1 year ago

The README states:

Note, however, that DNF5 cannot yet interface with the following programming languages:

  • Go - does not work, we are looking for contributors.

I would potentially like to work on this. Can someone explain on a high level what steps were required to add language bindings for other languages?

jan-kolarik commented 1 year ago

Hi @malt3, thank you very much for your interest!

The go bindings are actually already somehow prepared in the dnf5 project. It is just by default turned off during compilation, see the WITH_GO option in the root CMakeLists.txt. The compilation currently fails for existing code related to Go bindings.

All bindings are generated using SWIG, we are currently using the SWIG 4.0. You can see all related code in the bindings directory. In libdnf5 and libdnf5_cli directories, there are *.i SWIG interface files defining how to wrap C++ interfaces into respective scripting languages. There the Go-specific parts and directives might be missing. In bindings/go directory there are then make files to compile final SWIG libraries, you can see the other languages/directories like python3 how it is done.

So this should be basically what to do in the beginning, "simply" make it compilable with WITH_GO=ON, although it might require more effort than it seems.

Feel free to ask me for additional info if needed :slightly_smiling_face:

malt3 commented 1 year ago

Hey, I played around with this and have some bad news: The following modules have issues with swig / go interop:

All of those fail because swig has no support for std::unique_ptr wrapping in Go:

make go_base       
[  0%] Built target go_base_swig_compilation
[  0%] Built target common
[100%] Built target libdnf5
[100%] Building CXX object bindings/go/libdnf5/CMakeFiles/go_base.dir/CMakeFiles/go_base.dir/baseGO_wrap.cxx.o
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx: In function ‘std::unique_ptr<std::__cxx11::basic_string<char> >* _wrap_VarsWeakPtr_detect_release_libdnf5_4c9e9c06f3c4b176(libdnf5::WeakPtr<libdnf5::Vars, false>*, libdnf5::WeakPtr<libdnf5::Base, false>*, _gostring_)’:
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx:1365:95: error: ‘SWIGTYPE_p_std__unique_ptrT_std__string_t’ was not declared in this scope
 1365 |   _swig_go_result = SWIG_NewPointerObj(new std::unique_ptr< std::string >(std::move(result)), SWIGTYPE_p_std__unique_ptrT_std__string_t, SWIG_POINTER_OWN);
      |                                                                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx:1365:138: error: ‘SWIG_POINTER_OWN’ was not declared in this scope; did you mean ‘SWIGINTERN’?
 1365 |   _swig_go_result = SWIG_NewPointerObj(new std::unique_ptr< std::string >(std::move(result)), SWIGTYPE_p_std__unique_ptrT_std__string_t, SWIG_POINTER_OWN);
      |                                                                                                                                          ^~~~~~~~~~~~~~~~
      |                                                                                                                                          SWIGINTERN
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx:1365:21: error: ‘SWIG_NewPointerObj’ was not declared in this scope
 1365 |   _swig_go_result = SWIG_NewPointerObj(new std::unique_ptr< std::string >(std::move(result)), SWIGTYPE_p_std__unique_ptrT_std__string_t, SWIG_POINTER_OWN);
      |                     ^~~~~~~~~~~~~~~~~~
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx: In function ‘_gostring_* _wrap_LogEvent_get_spec_libdnf5_4c9e9c06f3c4b176(libdnf5::base::LogEvent*)’:
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx:2204:25: error: ‘SWIG_FromCharPtrAndSize’ was not declared in this scope
 2204 |       _swig_go_result = SWIG_FromCharPtrAndSize("", 0);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx:2206:25: error: ‘SWIG_FromCharPtrAndSize’ was not declared in this scope
 2206 |       _swig_go_result = SWIG_FromCharPtrAndSize(result->c_str(), result->size());
      |                         ^~~~~~~~~~~~~~~~~~~~~~~
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx: In function ‘_gostring_* _wrap_TransactionPackage_get_reason_change_group_id_libdnf5_4c9e9c06f3c4b176(libdnf5::base::TransactionPackage*)’:
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx:3331:25: error: ‘SWIG_FromCharPtrAndSize’ was not declared in this scope
 3331 |       _swig_go_result = SWIG_FromCharPtrAndSize("", 0);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~
/home/malte/projects/clones/dnf5/build/bindings/go/libdnf5/CMakeFiles/go_base.dir/baseGO_wrap.cxx:3333:25: error: ‘SWIG_FromCharPtrAndSize’ was not declared in this scope
 3333 |       _swig_go_result = SWIG_FromCharPtrAndSize(result->c_str(), result->size());
      |                         ^~~~~~~~~~~~~~~~~~~~~~~
At global scope:
cc1plus: note: unrecognized command-line option ‘-Wno-sometimes-uninitialized’ may have been intended to silence earlier diagnostics
make[3]: *** [bindings/go/libdnf5/CMakeFiles/go_base.dir/build.make:76: bindings/go/libdnf5/CMakeFiles/go_base.dir/CMakeFiles/go_base.dir/baseGO_wrap.cxx.o] Error 1
make[2]: *** [CMakeFiles/Makefile2:1751: bindings/go/libdnf5/CMakeFiles/go_base.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:1758: bindings/go/libdnf5/CMakeFiles/go_base.dir/rule] Error 2
make: *** [Makefile:400: go_base] Error 2

Doc on swig unique_ptr. This is what is doing the interop for python: https://github.com/swig/swig/blob/master/Lib/python/std_unique_ptr.i But as you will see, the Go variant is missing unique_ptr and shared_ptr wrappers: https://github.com/swig/swig/tree/master/Lib/go

There was an attempt to implement shared_ptr, but the PR for it was never merged and the issue was closed: https://github.com/swig/swig/issues/2030

See also: https://github.com/swig/swig/issues/2337

So in my eyes the possible ways forward are:

I may try to implement swig unique_ptr / shared_ptr support for Go upstream but I'm not sure what the scope of that feature would be. In any case, it would probably take time until dnf can depend on such a feature.

jan-kolarik commented 1 year ago

I think there are not many occurrences of std::unique_ptr on public API. Maybe we can try just ignoring these specific methods for Go in SWIG headers with some TODO comment like "not supported yet in SWIG" and then we could easily switch in the future when the support will be implemented.

jan-kolarik commented 11 months ago

@malt3 Please, do you plan to work on this? I still believe it would be beneficial to export only the parts without unique_ptr usage. If there is a specific usage crucial for some typical workflow, we can discuss it further and try to provide an alternative API for that.

malt3 commented 11 months ago

@malt3 Please, do you plan to work on this?

Realistically speaking, I'll probably not find the time in the coming weeks, so feel free to reassign this issue. My own need for this feature was solved by shelling out to dnf5 instead of using go bindings. I'm still interested in this but can't promise anything in the near future.

jan-kolarik commented 11 months ago

No problem, I'll keep it there for now and you're very welcome to continue in the future. I will unassign myself to show it as available for any other potential candidate who wishes to contribute to this.