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

Stop ignoring errors in CI. #207

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

waywardmonkeys
Copy link
Contributor

No description provided.

@waywardmonkeys
Copy link
Contributor Author

This PR is to see what happens in CI if we stop doing this...

@waywardmonkeys
Copy link
Contributor Author

@SeeSpring I think you modified some of these lines last ... what do you think?

@SeeSpring
Copy link
Contributor

Looks good. I think pinning sudo apt-get install libz3-dev to whatever version bindgen is generated against would also be useful to prevent missing symbols from occurring again.

This also reminds me that

#[ignore = "See https://github.com/Z3Prover/z3/issues/5702"]
can be removed now.

@waywardmonkeys
Copy link
Contributor Author

The problem with pinning libz3-dev is that in Ubuntu, only 4.8.12 is provided ... so if we stuck with that, we can't advance to current new features and fixes.

There are already a bunch of things that have been added with new APIs since 4.8.12 that might be good to support ...

If we used GitHub Actions cache action support, we could reduce the cost of doing the Z3 compilation and make it more reasonable to always build our own Z3 by default ...

@SeeSpring
Copy link
Contributor

If we don't pin, then if a user uses the z3 crate on Ubuntu with static-link-z3 off, they might end up getting linking errors despite CI passing. Forcing users to use static linking feels unreasonable with how long the build times are (~15 minutes)

I think it boils down to synchronizing the API that z3-sys builds against with the API of the system version of z3.

Do you know how other -sys crates handle a changing C API?

@waywardmonkeys
Copy link
Contributor Author

Not offhand yet.

There are a variety of issues here ... one is that the enumerations in the past weren't always updated in a consistent way from release to release of Z3, so sometimes values got re-used or re-ordered.

I've also started looking at Z3 itself to see if perhaps the compilation could be faster.

@SeeSpring
Copy link
Contributor

SeeSpring commented Jul 27, 2022

At least in CI, it seems that

- name: Build z3-sys with statically linked Z3
run: cargo build --manifest-path z3-sys/Cargo.toml -vv --features static-link-z3
- name: Build z3 with statically linked Z3
run: cargo build --manifest-path z3/Cargo.toml -vv --features static-link-z3
results in compiling z3 twice. I'm not sure how to fix it though

@waywardmonkeys
Copy link
Contributor Author

I'm not sure how to fix it though

I'm not sure that we need to test building each one separately. Just doing a cargo build -vv --features static-link-z3 once should be fine...

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.

None yet

2 participants