Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: c++ bindings #41

Merged
merged 3 commits into from
Jun 10, 2024
Merged

Conversation

baszalmstra
Copy link
Contributor

@baszalmstra baszalmstra commented Jun 6, 2024

Builds on #40

This PR adds preliminary C++ bindings. There is still stuff missing with regards to documentation and a proper CMake install but the functionality is there. A test that shows how to use the bindings can be found here: https://github.com/baszalmstra/resolvo/blob/feat/cpp_bindings/cpp/tests/solve.cpp#L165

My C++ is quite rusty (pun intended) so feedback is definitely welcome!

I added a pixi.toml to quickly get started with the code. Simply run:

pixi run test-cpp

to build and run the C++ tests.

@baszalmstra baszalmstra marked this pull request as ready for review June 7, 2024 18:50
@baszalmstra baszalmstra merged commit 10d75e9 into mamba-org:main Jun 10, 2024
8 checks passed
@baszalmstra baszalmstra deleted the feat/cpp_bindings branch June 10, 2024 15:16
@baszalmstra baszalmstra mentioned this pull request Jun 10, 2024
@jerily
Copy link
Contributor

jerily commented Jun 11, 2024

I have ported most of resolvo to C++ and I can share if you think there is any value to it. I am currently struggling with DisplayUnsat though. Just a heads up.

@wolfv
Copy link
Member

wolfv commented Jun 11, 2024

Definitely, that would be amazing to see! I also know that the mamba-team is looking into this! cc @JohanMabille @jjerphan

@jerily
Copy link
Contributor

jerily commented Jun 11, 2024

Please check your email. I have sent what I have so far. I was planning to contact you when it would have been fully completed.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Jun 11, 2024

I have ported most of resolvo to C++ and I can share if you think there is any value to it. I am currently struggling with DisplayUnsat though. Just a heads up.

Amazing!

@baszalmstra
Copy link
Contributor Author

@jerily That's amazing! Great work!

But I have to wonder, wouldn't you be able to use the bindings we just merged? I would very much like to consolidate efforts instead of fragmenting. I feel like it would be more beneficial for the larger community to all focus our efforts on a single code base than to have multiple implementations. That is exactly why I created these bindings in the first place.

If you are missing things from our bindings, or if they are not accessible enough, I would be more than happy to figure out how we can make it better for your use case too! :)

@jerily
Copy link
Contributor

jerily commented Jun 11, 2024

Thanks a lot!

I started this port because there was nothing like it a few months back. I will have to check out the C++ bindings and if they suit my needs, yes, I will use them. I must say though that everything I build these days (in open source I mean) it's C/C++ so not sure where rust fits into that picture.

That said, I'm not looking to compete with resolvo. I'm glad it exists and I hope it prospers further. I just want to build the tool I am working on in way that makes sense to me.

I just shared the code in case someone finds it valuable. That's all :)

@baszalmstra
Copy link
Contributor Author

@jerily Makes sense!

You should be able to use our bindings using something akin to (from the top of my head):

include(FetchContent)
FetchContent_Declare(
  Resolvo
  GIT_REPOSITORY https://github.com/mamba-org/resolvo.git
  GIT_TAG resolvo_cpp-v0.1.0)
FetchContent_MakeAvailable(Resolvo)

You should then have a Resolvo target that you can link to. This should have all the include- and library paths set up correctly.

The only downside of this approach is that you need to have Rust installed on your system.

I am also creating a conda package that has prebuilt binaries and doesn't require any Rust. (Not sure how familiar you are using that). Should be online in a few hours!

If you try this out, please share your thoughts! I would be very happy to facilitate making the bindings easier to use.

@jerily
Copy link
Contributor

jerily commented Jun 11, 2024

Thanks! I will check it out this week. At work at the moment - Java & Python :)

@jerily
Copy link
Contributor

jerily commented Jun 15, 2024

I have tried out the C++ bindings. Here are few notes:

  • They work!!!
  • It would have been nice if there were a few examples how to use all the features. I ended up using cpp/tests/solve.cpp
  • I don't see any way to get the nice graph that explains why a problem is unsolvable, e.g.:
The following packages are incompatible
└─ asdf * can be installed with any of the following options:
   └─ asdf 1 would require
      └─ c >=2, <3, which can be installed with any of the following options:
         └─ c 2
└─ c 1 is locked, but another version is required as reported above

@jerily
Copy link
Contributor

jerily commented Jun 15, 2024

Figured out where the nice graph is. I am good!

@baszalmstra
Copy link
Contributor Author

@jerily great! If you have suggestions for the API or if you have examples you can share Id love to hear it!

@jerily
Copy link
Contributor

jerily commented Jun 15, 2024

Just getting started. A quick one though is the for loop in display_merged_solvables. It needs to be:

        bool first = true;
        for (const auto& solvable : solvables) {
            if (!first) {
                ss << " | ";
            }

            ss << candidates[solvable.id].version;
            first = false;
        }

@baszalmstra
Copy link
Contributor Author

Would you be able to open a PR?

@jerily
Copy link
Contributor

jerily commented Jun 15, 2024

Just got back from theater. I'm on it now.

@jjerphan
Copy link
Member

jjerphan commented Jul 2, 2024

Hi @jerily, would you be interested in publishing your C++ port of resolvo on GitHub? :)

@jerily
Copy link
Contributor

jerily commented Jul 2, 2024

Hi @jjerphan , yes, sure. Please note that DisplayUnsat is not finished and I turned to using C++ bindings myself. I can either place it on github now as it stands or finish it and then publish it by end of July. Please let me know what works for you best.

@jjerphan
Copy link
Member

jjerphan commented Jul 2, 2024

I would encourage you to publish it on GitHub, we might have been able to contribute to it to help you if we have time. 🙂

@jerily
Copy link
Contributor

jerily commented Jul 2, 2024

Here it is: https://github.com/jerily/resolvo-cpp

I should be able to resume work on it after July 20.

@jerily
Copy link
Contributor

jerily commented Jul 3, 2024

Renamed the project to avoid clash with resolvo to satdepsolver-cpp: https://github.com/jerily/satdepsolver-cpp

Please let me know if that's ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants