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

Why3 1.7.0 #24837

Merged
merged 2 commits into from
Nov 27, 2023
Merged

Why3 1.7.0 #24837

merged 2 commits into from
Nov 27, 2023

Conversation

silene
Copy link
Contributor

@silene silene commented Nov 24, 2023

No description provided.

@silene
Copy link
Contributor Author

silene commented Nov 25, 2023

None of the remaining failures are due to the added packages, so this is ready.

@haochenx
Copy link
Collaborator

@silene it's probably a good idea to up stream #23843

@silene
Copy link
Contributor Author

silene commented Nov 27, 2023

The archive already contains a fully functional configure script; there is no reason to force the users to install autoconf to produce a new one.

@haochenx
Copy link
Collaborator

Then I guess maybe the dependency to conf-autoconf should be removed?

@silene
Copy link
Contributor Author

silene commented Nov 27, 2023

No, it is needed for people who are directly building from the git repository, since we are not versioning the automatically generated configure script, hence why the dependency on conf-autoconf is only there when Opam runs in dev mode.

@haochenx
Copy link
Collaborator

Ah. So does it mean patches like #23843 is no longer needed for this new version?

@silene
Copy link
Contributor Author

silene commented Nov 27, 2023

It was not needed either for the older releases, since the configure script has always been shipped in the archive. But for some unknown reason, in some very rare cases, the configure script was considered to be out-of-date, which caused Make to try to regenerate it from scratch. (A filesystem issue?) So, yes, depending on conf-autoconf does work around the issue, but it is way overkill. It is much cheaper to just ensure that configure is up-to-date by touching it just after extraction.

@haochenx
Copy link
Collaborator

Okay I see. So if my understanding is correct these are not necessary: I will dismiss the suggestions.

Copy link
Collaborator

@haochenx haochenx left a comment

Choose a reason for hiding this comment

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

All looks good to me. CI failures are unrelated as mentioned by the author


build: [
["./autogen.sh"] {dev} # when pinning, there might be no configure file
["touch" "configure"]
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation. Should I backport this to the old why3 versions and reinstated conf-autoconf as a dev-only dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think it is worth the effort, now that Why3 1.7 is available.

@mseri mseri merged commit b2a432e into ocaml:master Nov 27, 2023
1 of 2 checks passed
@silene silene deleted the why3-1.7.0 branch November 27, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants