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

Kissat bindings #7

Merged
merged 4 commits into from
Oct 4, 2023
Merged

Kissat bindings #7

merged 4 commits into from
Oct 4, 2023

Conversation

lixitrixi
Copy link
Contributor

Here I the kissat-rs module as cargo dependencies. This will hopefully help with modularity and size, and if we need to add to the libraries this can be changed to use our fork.

Next up is working on CI and unit testing for our SAT solver bindings. I'll try to find a public source of test DIMACS files and set up automatically running them through the bindings.

@lixitrixi
Copy link
Contributor Author

Also in this PR I've added build files generated by Minion and anything else to .gitignore, since it was an issue for me.

@ozgurakgun
Copy link
Contributor

by merging #6 I created a (trivial) merge conflict here, sorry! can you fix it please @lixitrixi?

@lixitrixi
Copy link
Contributor Author

I've added a general import rule to Cargo.toml which imports all solvers/*. Let me know if that works! The empty exclude is just to remind of the syntax in case people need to temporarily exclude a solver.

@lixitrixi
Copy link
Contributor Author

This should now be good to merge @ozgurakgun !

@niklasdewally
Copy link
Collaborator

I've added a general import rule to Cargo.toml which imports all solvers/*. Let me know if that works! The empty exclude is just to remind of the syntax in case people need to temporarily exclude a solver.

if running cargo build in the root directory tries to build minion, it should be working.

@ozgurakgun
Copy link
Contributor

not a big deal but I slightly prefer explicitly listing the solvers.

@lixitrixi
Copy link
Contributor Author

I've pulled and made the change back to explicitly listing them, but I get this error when building:

error: failed to run custom build command for `chuffed_rs v0.1.0 (/Users/felixleitner/dev/repos/conjure-oxide/solvers/chuffed)`

Caused by:
  process didn't exit successfully: `/Users/felixleitner/dev/repos/conjure-oxide/target/debug/build/chuffed_rs-0412293b45d76914/build-script-build` (exit status: 101)
  --- stdout
  cargo:rustc-rerun-if-changed=vendor
  cargo:rerun-if-changed=build.rs
  cargo:rustc-link-search=all=./solvers/chuffed/vendor/build/
  cargo:rustc-link-lib=static=chuffed
  cargo:rustc-link-lib=static=chuffed_fzn
  cargo:rustc-link-lib=dylib=c++
  stdout
  ------ BUILDING ------

  stderr
  build.sh: line 11: cmake: command not found
  build.sh: line 12: cmake: command not found


  --- stderr
  thread 'main' panicked at 'build.sh has non zero exit status', solvers/chuffed/build.rs:49:9
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace```

@niklasdewally
Copy link
Collaborator

I've pulled and made the change back to explicitly listing them, but I get this error when building:

error: failed to run custom build command for `chuffed_rs v0.1.0 (/Users/felixleitner/dev/repos/conjure-oxide/solvers/chuffed)`

Caused by:
  process didn't exit successfully: `/Users/felixleitner/dev/repos/conjure-oxide/target/debug/build/chuffed_rs-0412293b45d76914/build-script-build` (exit status: 101)
  --- stdout
  cargo:rustc-rerun-if-changed=vendor
  cargo:rerun-if-changed=build.rs
  cargo:rustc-link-search=all=./solvers/chuffed/vendor/build/
  cargo:rustc-link-lib=static=chuffed
  cargo:rustc-link-lib=static=chuffed_fzn
  cargo:rustc-link-lib=dylib=c++
  stdout
  ------ BUILDING ------

  stderr
  build.sh: line 11: cmake: command not found
  build.sh: line 12: cmake: command not found


  --- stderr
  thread 'main' panicked at 'build.sh has non zero exit status', solvers/chuffed/build.rs:49:9
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace```
brew install cmake

@ozgurakgun ozgurakgun merged commit ec06c22 into conjure-cp:main Oct 4, 2023
3 checks passed
@lixitrixi
Copy link
Contributor Author

Nooooo I was about to change it back to listing them

@ozgurakgun
Copy link
Contributor

Ooops, I thought you had already done that :) another PR?

@lixitrixi
Copy link
Contributor Author

Ooops, I thought you had already done that :) another PR?

#12 @ozgurakgun

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.

3 participants