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

set python to 3.7 in docker container and release action #2879

Merged
merged 11 commits into from
Nov 16, 2023

Conversation

remi-delmas-3000
Copy link
Contributor

@remi-delmas-3000 remi-delmas-3000 commented Nov 14, 2023

An attempt to fix a release build issue due to cbmc-viewer depending on voluptuous which now depends on python 3.7 for type annotations.

This adds a ppa in the dockerfile and github release action for ubuntu 18.04 and installs python 3.7.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@remi-delmas-3000 remi-delmas-3000 requested a review from a team as a code owner November 14, 2023 22:54
@feliperodri
Copy link
Contributor

Is there a way to also upgrade the "workaround" for Ubuntu 18.04.

pub fn setup_python_deps_on_ubuntu_18_04(pyroot: &Path, pkg_versions: &[&str]) -> Result<()> {

@feliperodri
Copy link
Contributor

We also need to update our dependencies list in the Installation guidelines.

* **Python version 3.6 or newer** and the package installer `pip`.

@remi-delmas-3000
Copy link
Contributor Author

Is there a way to also upgrade the "workaround" for Ubuntu 18.04.

I am not modifying the OS hacks 18.04 because they are meant to be applied only when the default pip 9.x and python 3.6.x versions are detected on the host. Since we now require python 3.7 they should not have to be applied anymore.

Copy link
Contributor

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we now require python 3.7 they should not have to be applied anymore.

Why not remove the hacks then?

.github/workflows/release.yml Show resolved Hide resolved
@remi-delmas-3000
Copy link
Contributor Author

Why not remove the hacks then?
we would need to add a python version check too

@jaisnan
Copy link
Contributor

jaisnan commented Nov 16, 2023

Why not remove the hacks then?
we would need to add a python version check too

As discussed offline, I am adding a check for the python version directly, and removing the hacks. @remi-delmas-3000 can you confirm if this is the right plan?

Copy link
Contributor

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Python version checks will abort the installation if the minimum version isn't met. But the setup code isn't expected to do this, what it should do in any case is attempt to solve these problems for you.

@jaisnan
Copy link
Contributor

jaisnan commented Nov 16, 2023

The Python version checks will abort the installation if the minimum version isn't met. But the setup code isn't expected to do this, what it should do in any case is attempt to solve these problems for you.

I did attempt to solve the problem, but even upgrading to 3.7 is a tough job to accomplish through rust's interface since even python 3.7's reached end-of-life support ☹️. Should we skip 3.7 and directly upgrade the user's python to 3.8 or the latest default version of python?

I assumed the answer to be no, since anyone using python 3.6 or older is likely doing it for very specific reasons, and just informing them seemed like a good idea. Happy to change this behavior, however.

@adpaco-aws
Copy link
Contributor

I assumed the answer to be no, since anyone using python 3.6 or older is likely doing it for very specific reasons, and just informing them seemed like a good idea.

I agree that we shouldn't upgrade python in that case. But here you're not "just informing" the users, you're failing the installation...

Again, why not just remove the hacks? You can leave improvements for later.

.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
@jaisnan jaisnan merged commit 4ddd50b into model-checking:main Nov 16, 2023
20 checks passed
jaisnan added a commit that referenced this pull request Nov 17, 2023
Upgrade version number to 0.41.0

The CHANGELOG changes are:

```
# What's Changed
* Fix setup for `aarch64-unknown-linux-gnu` platform by @adpaco-aws in #2864
* Do not override `std` library during playback by @celinval in #2852
* Fix docker build step in release workflow by @adpaco-aws in #2854
* Fix path-dependent failures in `assess-artifacts` test by @adpaco-aws in #2849
* Automatic toolchain upgrade to nightly-2023-11-11 by @github-actions in #2870

**Full Changelog**: kani-0.40.0...kani-0.41.0

### Breaking Changes

* Set minimum python to 3.7 in docker container and release action by @remi-delmas-3000 in #2879
* Delete `any_slice` which has been deprecated since Kani 0.38.0. by @zhassan-aws in #2860
```

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
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.

4 participants