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

casadi: build on darwin #333314

Merged
merged 15 commits into from
Aug 14, 2024
Merged

casadi: build on darwin #333314

merged 15 commits into from
Aug 14, 2024

Conversation

nim65s
Copy link
Contributor

@nim65s nim65s commented Aug 8, 2024

Description of changes

Follows #331343, and require #333016. When the latter is merged, I'll mark this ready to review.
Pinocchio's example-py-casadi-quadrotor-ocp which use casadi + ipopt + mumps from python is also working on darwin now.

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/)
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Aug 8, 2024
@ofborg ofborg bot requested review from wegank, aanderse and abbradar August 9, 2024 00:02
@nim65s nim65s force-pushed the casadi-darwin branch 2 times, most recently from 3e20b32 to 316a388 Compare August 12, 2024 11:05
@nim65s
Copy link
Contributor Author

nim65s commented Aug 12, 2024

@doronbehar : thanks a lot for your review !
I think I addressed every point.

nixpkgs-review new results should be posted soon on aarch64-darwin & x86_64-linux.

doronbehar
doronbehar reviewed Aug 12, 2024
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Changes look mostly good. BTW this should be marked as ready for review as #333016 is merged.

pkgs/by-name/hp/hpipm/package.nix Show resolved Hide resolved
@nim65s nim65s marked this pull request as ready for review August 12, 2024 11:35
@nim65s nim65s marked this pull request as draft August 12, 2024 12:26
@nim65s
Copy link
Contributor Author

nim65s commented Aug 12, 2024 •
edited
Loading

some tests are failing for now

edit: ok, a commit must have been lost in history rewrite at some point.

@nim65s nim65s marked this pull request as ready for review August 12, 2024 14:49
@nim65s
Copy link
Contributor Author

nim65s commented Aug 12, 2024

Result of nixpkgs-review pr 333314 run on aarch64-darwin 1

13 packages built:
  • bonmin
  • crocoddyl
  • ipopt
  • openturns
  • pinocchio
  • python311Packages.crocoddyl
  • python311Packages.example-robot-data
  • python311Packages.openturns
  • python311Packages.pinocchio
  • python312Packages.crocoddyl
  • python312Packages.example-robot-data
  • python312Packages.openturns
  • python312Packages.pinocchio

@nim65s
Copy link
Contributor Author

nim65s commented Aug 12, 2024

Result of nixpkgs-review pr 333314 run on x86_64-linux 1

21 packages built:
  • bonmin
  • casadi
  • crocoddyl
  • hpipm
  • ipopt
  • mumps
  • openturns
  • pagmo2
  • pinocchio
  • python311Packages.casadi
  • python311Packages.crocoddyl
  • python311Packages.example-robot-data
  • python311Packages.openturns
  • python311Packages.pinocchio
  • python311Packages.pygmo
  • python312Packages.casadi
  • python312Packages.crocoddyl
  • python312Packages.example-robot-data
  • python312Packages.openturns
  • python312Packages.pinocchio
  • python312Packages.pygmo

@nim65s
Copy link
Contributor Author

nim65s commented Aug 12, 2024

No, this is still wrong, sorry for the noise. Casadi should build on darwin, that's the point of this PR…

@nim65s nim65s marked this pull request as draft August 12, 2024 14:53
@nim65s
Copy link
Contributor Author

nim65s commented Aug 12, 2024

I don't understand why nixpkgs-review does not list casadi on aarch64-darwin, but everything seems fine.

@nim65s nim65s marked this pull request as ready for review August 12, 2024 15:26
@wegank
Copy link
Member

wegank commented Aug 12, 2024

I don't understand why nixpkgs-review does not list casadi on aarch64-darwin, but everything seems fine.

Does casadi have meta.platforms set?

@nim65s
Copy link
Contributor Author

nim65s commented Aug 12, 2024

Does casadi have meta.platforms set?

no. But I can find 165/165 Test #165: example-py-casadi-quadrotor-ocp .................... Passed 7.15 sec in nix log .#python3Packages.pinocchio on aarch64-darwin, so I think this is fine.

@doronbehar
Copy link
Contributor

doronbehar commented Aug 13, 2024 •
edited
Loading

I don't understand why nixpkgs-review does not list casadi on aarch64-darwin, but everything seems fine.

That's indeed very peculiar.. The ofborg build attempts of the relevant attributes on x86_64-darwin were all successful, and for aarch64-linux they don't appear as green, and it is not fully clear to me why:

error: 1 dependencies of derivation '/nix/store/dwicrqshl3nmw9qc6jxaf0ksrji02smc-casadi-3.6.6.drv' failed to build
error: 1 dependencies of derivation '/nix/store/006w9vz1914vyigmnaj31ks8vhnmml2h-pinocchio-3.1.0.drv' failed to build
error: build of '/nix/store/006w9vz1914vyigmnaj31ks8vhnmml2h-pinocchio-3.1.0.drv', '/nix/store/dwicrqshl3nmw9qc6jxaf0ksrji02smc-casadi-3.6.6.drv' failed

Also, hydra-check casadi --arch aarch64-linux doesn't return any URL.. meaning that it could be an issue with any aarch64- platform and this derivation. I'd try to set platforms = lib.platforms.all and see what ofborg does with that. In anycase, the PR is generally ready IMO.

@nim65s
casadi: set meta.platforms
a3b45b9 
To try to see why why nixpkgs-review does not list casadi on aarch64-darwin

We still can see
`165/165 Test NixOS#165: example-py-casadi-quadrotor-ocp ............  Passed`
in `nix log .#python3Packages.pinocchio` on aarch64-darwin, so
everything looks fine anyways.
@nim65s
Copy link
Contributor Author

nim65s commented Aug 13, 2024

I have no idea how meta.platforms could play here, but since you both mention it, I guess we should add it 😅

@nim65s
Copy link
Contributor Author

nim65s commented Aug 13, 2024

Result of nixpkgs-review pr 333314 run on aarch64-darwin 1

20 packages built:
  • bonmin
  • casadi
  • crocoddyl
  • hpipm
  • ipopt
  • mumps
  • openturns
  • pinocchio
  • python311Packages.casadi
  • python311Packages.crocoddyl
  • python311Packages.example-robot-data
  • python311Packages.openturns
  • python311Packages.pinocchio
  • python311Packages.pygmo
  • python312Packages.casadi
  • python312Packages.crocoddyl
  • python312Packages.example-robot-data
  • python312Packages.openturns
  • python312Packages.pinocchio
  • python312Packages.pygmo

@nim65s
Copy link
Contributor Author

nim65s commented Aug 13, 2024

thanks for the 🪄 !

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Aug 13, 2024
@doronbehar
Copy link
Contributor

@ofborg build casadi

@doronbehar
Copy link
Contributor

The aarch64-linux build still appears as failing. I want to make sure it really fails.

@doronbehar
Copy link
Contributor

I OK now it is more clear what is the aarch64-linux error:

The following tests FAILED:
         66 - ProxQP::dense: test primal infeasibility solving (Failed)
Errors while running CTest

@nim65s perhaps could you make an effort to disable that test on aarch64-linux?

@nim65s
Copy link
Contributor Author

nim65s commented Aug 13, 2024

Done.
There are ~20 tests with the same name, so all are disabled, but I guess this is fine.

doronbehar
doronbehar approved these changes Aug 14, 2024
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Changes look good. CI is green now for both Linux architectures, and it was green before the last aarch64-linux commit for x86_64-darwin, and nixpkgs-review was happy as well.

@wegank wegank merged commit d4c5ef7 into NixOS:master Aug 14, 2024
24 of 26 checks passed
@nim65s nim65s deleted the casadi-darwin branch August 22, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doronbehar doronbehar doronbehar approved these changes

@wegank wegank wegank approved these changes

@aanderse aanderse Awaiting requested review from aanderse

@abbradar abbradar Awaiting requested review from abbradar

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nim65s @wegank @doronbehar