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.

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

stp: 2.3.3 -> 2.3.4 #348447

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

stp: 2.3.3 -> 2.3.4 #348447

wants to merge 1 commit into from
+194 −45

Conversation

numinit
Copy link
Contributor

@numinit numinit commented Oct 14, 2024 •
edited
Loading

Bump the version, enable checkPhase, and create an installCheckPhase.

Use cadical by default, though support useCadical = false in an override if users want to disable it.

Fix the python bindings.

Write a patch to avoid infinite loops on aarch64-linux.

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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

@numinit numinit requested a review from McSinyx October 14, 2024 05:46
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Oct 14, 2024
@numinit numinit requested a review from pbsds October 19, 2024 21:09
@numinit numinit force-pushed the stp-2.3.4 branch 2 times, most recently from cc20f12 to 33126a0 Compare October 19, 2024 21:15
@numinit
Copy link
Contributor Author

numinit commented Oct 19, 2024

Moved substituteInPlace shenanigans to postPatch.

@numinit
Copy link
Contributor Author

numinit commented Oct 19, 2024

Use the 2.3.4_cadical branch and compile with Cadical support, since that coincides with the actual STP release:

https://github.com/stp/stp/releases/tag/2.3.4_cadical

@numinit
Copy link
Contributor Author

numinit commented Oct 20, 2024

Looks like Darwin timed out but it did here too

@numinit
Copy link
Contributor Author

numinit commented Oct 21, 2024

@ofborg build stp

@numinit
Copy link
Contributor Author

numinit commented Oct 22, 2024

Looks like the tests time out on aarch64-linux...

@numinit
Copy link
Contributor Author

numinit commented Oct 22, 2024

@ofborg build stp

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 10, 2024
@numinit numinit added the backport release-24.11 Backport PR automatically label Nov 17, 2024
@numinit
Copy link
Contributor Author

numinit commented Nov 17, 2024

@RossComputerGuy Another one for you. Probably no reason why this should break on aarch64-linux, seemed like it was ofborg breaking.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 17, 2024
@RossComputerGuy
Copy link
Member

Result of nixpkgs-review pr 348447 --extra-nixpkgs-config '{ packageOverrides = pkgs: with pkgs; { stp = stp.override { stdenv = zigStdenv; }; }; }' run on aarch64-linux 1

1 package failed to build:
  • stp

@RossComputerGuy
Copy link
Member

Result of nixpkgs-review pr 348447 run on aarch64-linux 1

1 package built:
  • stp

RossComputerGuy
RossComputerGuy requested changes Nov 18, 2024
View reviewed changes
Copy link
Member

@RossComputerGuy RossComputerGuy left a comment

Choose a reason for hiding this comment

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

Looks good, it builds. Probably should add a dev output so $out/include and $out/lib/cmake gets split out into it.

@numinit numinit mentioned this pull request Nov 18, 2024
Open
pbsds
pbsds reviewed Nov 18, 2024
View reviewed changes
pkgs/by-name/st/stp/package.nix Outdated

cmakeFlags =
let
onOff = var: flag: lib.singleton "-D${var}=${if flag then "ON" else "OFF"}";
Copy link
Member

Choose a reason for hiding this comment

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

Please use lib.cmakeBool

@numinit numinit marked this pull request as draft November 29, 2024 06:09
@numinit
Copy link
Contributor Author

numinit commented Nov 29, 2024

Drafting since this probably needs an upstream fix.

@numinit numinit mentioned this pull request Dec 26, 2024
Open
3 tasks
@numinit numinit mentioned this pull request Jan 12, 2025
Merged
@numinit
Copy link
Contributor Author

numinit commented Jan 20, 2025

Working with upstream on it, but found the source of the infinite loop in tests on aarch64 community builder.

@numinit numinit force-pushed the stp-2.3.4 branch 4 times, most recently from 11544dc to e129ccf Compare January 23, 2025 07:12
@github-actions github-actions bot added 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Jan 23, 2025
@numinit numinit force-pushed the stp-2.3.4 branch 2 times, most recently from 3a323a2 to 960e074 Compare January 23, 2025 08:17
@numinit numinit marked this pull request as ready for review January 23, 2025 08:18
@numinit
Copy link
Contributor Author

numinit commented Jan 23, 2025

Working now on all platforms.

@numinit numinit force-pushed the stp-2.3.4 branch 3 times, most recently from da4d713 to 0e56dcf Compare January 23, 2025 09:00
@numinit
stp: 2.3.3 -> 2.3.4
2fee3e4 
Bump the version, enable checkPhase, and create an installCheckPhase.
Turn on Darwin support (it works) and fix the Python libs.

Use cadical by default, though support `useCadical = false` in an
override if users want to disable it.
@numinit
Copy link
Contributor Author

numinit commented Jan 23, 2025

And fixed the Python bindings.

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

@McSinyx McSinyx Awaiting requested review from McSinyx

@RossComputerGuy RossComputerGuy Awaiting requested review from RossComputerGuy

@pbsds pbsds Awaiting requested review from pbsds

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes backport release-24.11 Backport PR automatically
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@numinit @RossComputerGuy @pbsds @wegank