bindings: Fix and enable builds with Go bindings #1089
Conversation
| @@ -50,7 +50,7 @@ option(WITH_DNF5DAEMON_TESTS "Build dnf5daemon tests (requires configured dbus)" | |||
| option(WITH_SANITIZERS "Build with address, leak and undefined sanitizers (DEBUG ONLY)" OFF) | |||
|
|
|||
| # build options - bindings | |||
| option(WITH_GO "Build Go bindings" OFF) | |||
| option(WITH_GO "Build Go bindings" ON) | |||
There was a problem hiding this comment.
I think we probably don't want to turn it on by default just yet.
I have spent some time trying it out and I don't think the bindings actually work.
Were you able to use them?
I would like to set up some tests the way we have for Python or Perl before we enable them.
| @@ -101,7 +101,7 @@ struct Vars { | |||
| /// @param name Name of the variable | |||
| const Variable & get(const std::string & name) const { return variables.at(name); } | |||
|
|
|||
| static std::unique_ptr<std::string> detect_release(const BaseWeakPtr & base, const std::string & install_root_path); | |||
| static std::string detect_release(const BaseWeakPtr & base, const std::string & install_root_path); | |||
There was a problem hiding this comment.
We cannot do that, it would break API.
Either we could target this PR to the dnf5-5.2.0.0 branch (which is a new major release with breaking changes) or we have to find another way. Perhaps we could simply ignore the function in go bindings for now.
| @@ -33,8 +33,9 @@ namespace libdnf5 { | |||
| /// Base class for configurations objects | |||
| class Config { | |||
| public: | |||
| #ifndef SWIGGO | |||
There was a problem hiding this comment.
I would rather keep theseSWIGGO conditionals only in the interface files.
I think it could look something like:
#if defined(SWIGGO)
%ignore libdnf5::Config::opt_binds;
#endif
| @@ -53,8 +53,13 @@ class RepoCache { | |||
| public: | |||
| using RemoveStatistics = RepoCacheRemoveStatistics; | |||
|
|
|||
| /// The name of the attribute used to mark the cache as expired. | |||
| /// The name of the attribute used to mark the cache as expired. | |||
| #ifndef SWIGGO | |||
There was a problem hiding this comment.
Here as well, please keep the SWIGGO conditions only in the *.i files.
Fixes #719
Ok, disclaimer, I am neither a SWIG expert nor a dnf one but I was just poking around the codebase.
With that being clear, its a rough take on exposing the API.
std::unique_ptr, which I don't know if important or not.constexpr const char*which is a bit annoying but can be worked aroundDefinitely need some help.