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

add downloading z3 4.13.3 and 4.8.5 to the everest script #119

Merged
merged 3 commits into from
Dec 13, 2024

Conversation

nikswamy
Copy link
Contributor

Preparing for F* upgrading to z3-4.13.3 in FStarLang/FStar#3631

This PR copies FStar/bin/get_fstar_z3.sh, which downloads the right versions of Z3 for each platform

And updates the everest script to call get_fstar_z3.sh instead of relying on FStarLang/binaries

everest Outdated
ln -sf $new_z3_folder z3
./get_fstar_z3.sh $z3_versions_dest
magenta "Automatically customize $EVEREST_ENV_DEST_FILE with the z3 path $z3_versions_dest? [Yn]"
prompt_yes "write_z3_env_dest_file $z3versions_dest" true
Copy link
Member

Choose a reason for hiding this comment

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

Typo here? z3versions_dest -> z3_versions_dest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks!

@gebner
Copy link
Contributor

gebner commented Dec 13, 2024

If we have a copy of get_fstar_z3.sh here, then it should be automatically updated. I believe that would go into everest-move-fun.sh but I would only put it in there once FStarLang/FStar#3631 is merged.

@mtzguido
Copy link
Member

Agree, but I'll merge this to fix the build for now, since Pulse already requires 4.13.3.

@mtzguido mtzguido merged commit 9154805 into master Dec 13, 2024
2 of 3 checks passed
if [ -f "$destination_file_name" ]; then
echo ">>> Z3 $z3_ver already downloaded to $destination_file_name"
elif [ -z "$url" ]; then
echo ">>> Z3 $z3_ver not available for this architecture, skipping..."
Copy link
Member

Choose a reason for hiding this comment

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

On Apple Silicon, this declines to install 4.8.5. However, 4.8.5 can run on OSX, via its x64 emulation facilities (a.k.a. Rosetta 2). This is actually required for HACL* and other projects which have not yet upgraded to the latest z3.

I would suggest special-casing 4.8.5 / aarch64 to download the x64 version, and printing out a notice that Rosetta 2 ought to be installed on this machine.

A few more remarks:

  • you'll probably have a better time using [[ for you tests, since this is lexed and parsed by bash itself, meaning it can deal with variables that expand to empty, rather than [, which has to obey command-line argument parsing as if it were calling the actual /usr/bin/[, and therefore will yield unexpected errors if owing to any circumstances some of your variables are actually empty
  • you could use the local keyword in a few of your functions for local variables
  • your script requires bash >= 4 (associative arrays) but does not check for it -- this is pertinent for OSX, which ships bash 3 (the last GPLv2 version), but not bash 4; note that for now this is probably fine since this is only ever called via the everest script, which does run proper checks.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! See FStarLang/FStar#3645

Copy link
Contributor

Choose a reason for hiding this comment

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

you'll probably have a better time using [[ for you tests, since this is lexed and parsed by bash itself, meaning it can deal with variables that expand to empty, rather than [, which has to obey command-line argument parsing as if it were calling the actual /usr/bin/[, and therefore will yield unexpected errors if owing to any circumstances some of your variables are actually empty

[ works just fine with empty variables. You need to quote variables, but you should always quote them anyhow.

your script requires bash >= 4 (associative arrays) but does not check for it -- this is pertinent for OSX, which ships bash 3

I didn't test it at all on macos, thanks for serving as the guinea pig! FStarLang/FStar#3646 (I see Guido beat me to it.)

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