Skip to content

isabelle: 2024 -> 2025 #389562

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.

Sign up for GitHub

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

Jump to bottom
Merged
wegank merged 2 commits into from
Apr 20, 2025
Merged

isabelle: 2024 -> 2025 #389562

wegank merged 2 commits into from
Apr 20, 2025

Conversation

@jvanbruegge
Copy link
Contributor

@jvanbruegge jvanbruegge commented Mar 13, 2025

https://isabelle.in.tum.de/dist/Isabelle2025/doc/NEWS.html

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 13, 2025
@nix-owners nix-owners bot requested a review from jwiegley March 13, 2025 17:32
@jvanbruegge jvanbruegge added 8.has: package (update) This PR updates a package to a newer version 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. labels Mar 13, 2025
@jvanbruegge
Copy link
Contributor Author

jvanbruegge commented Mar 13, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 389562

x86_64-linux

❌ 1 package failed to build:
  • isabelle-components.isabelle-linter
✅ 3 packages built:
  • isabelle
  • tlaps
  • vampire

@jvanbruegge
Copy link
Contributor Author

jvanbruegge commented Mar 13, 2025

The linter failure is expected it has not been updated yet

@jvanbruegge
Copy link
Contributor Author

jvanbruegge commented Mar 22, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 389562

x86_64-linux

✅ 4 packages built:
  • isabelle
  • isabelle-components.isabelle-linter
  • tlaps
  • vampire

@misuzu misuzu added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 29, 2025
@jvanbruegge jvanbruegge removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 31, 2025
@jvanbruegge
Copy link
Contributor Author

jvanbruegge commented Mar 31, 2025

Fixed the merge conflict

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@jvanbruegge
Copy link
Contributor Author

jvanbruegge commented Apr 9, 2025

Fixed the merge conflict, again. Could this please be merged?

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 9, 2025
@sempiternal-aurora
Copy link

sempiternal-aurora commented Apr 12, 2025

I too would love this merge, I've been waiting for it for a while, and pulling the changes works fine on my system

@jvanbruegge
Copy link
Contributor Author

jvanbruegge commented Apr 20, 2025

@wegank you merged the last Isabelle update, could you this one as well?

I've used it now daily for several weeks without issues.

@jvanbruegge
Copy link
Contributor Author

jvanbruegge commented Apr 20, 2025

This should be part of NixOS 25.05

@sempiternal-aurora
Copy link

sempiternal-aurora commented Apr 20, 2025

Can confirm it works well, I've been using it for the past week or so fine.

@wegank
Copy link
Member

wegank commented Apr 20, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 389562

aarch64-darwin

✅ 3 packages built:
  • isabelle
  • isabelle-components.isabelle-linter
  • tlaps

@wegank wegank merged commit e175357 into NixOS:master Apr 20, 2025
27 checks passed
@jvanbruegge jvanbruegge deleted the isabelle-2025 branch April 20, 2025 22:44
@jvanbruegge
Copy link
Contributor Author

jvanbruegge commented Apr 20, 2025

Thank you!

roberth
roberth reviewed May 28, 2025
View reviewed changes
pkgs/top-level/all-packages.nix
polyml = polyml.overrideAttrs {
pname = "polyml-for-isabelle";
version = "2024";
version = "2025";
Copy link
Member

@roberth roberth May 28, 2025

Choose a reason for hiding this comment

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

Is this override needed?

In #411760 we're making a warning about version overriding more effective, and it's triggering for this override. I suspect the best solution is to just drop this line, assuming it was only here because src was previously overridden.

Copy link
Contributor Author

@jvanbruegge jvanbruegge May 28, 2025

Choose a reason for hiding this comment

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

I don't really care what the version says, this was done in response to a review comment a while back. The rest of the override is needed, as isabelle needs those configure flags

Copy link
Member

@roberth roberth May 28, 2025

Choose a reason for hiding this comment

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

Ok, it seems that it is an intentional version override then, despite not changing the src.
#411760 will add the following:

       version = "2025";
+      __intentionallyOverridingVersion = true; # avoid a warning, no src override

No action is required on your end.
If you were to remove version or add src, you can remove the extra line.

Sorry to bother you with this. CI requires that we keep the package set free of warnings, and that's probably for the better.

Copy link
Contributor Author

@jvanbruegge jvanbruegge May 28, 2025

Choose a reason for hiding this comment

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

Yeah, no problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@roberth roberth roberth left review comments

@jwiegley jwiegley Awaiting requested review from jwiegley

Assignees

No one assigned

Labels

8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jvanbruegge @sempiternal-aurora @wegank @roberth @misuzu