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

Update CasADi to 3.6.6 and enable fatrop dependency on Linux #115

Merged

Conversation

conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I've started a version update as instructed in #114.

I'm currently searching for new versions and will update this PR shortly if I find one! Thank you for waiting!

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

conda-forge-webservices[bot] added 2 commits August 28, 2024 19:21
@agriyakhetarpal
Copy link
Member

I see that the second patch is failing to apply. CasADi changed the line from friend to inline friend, which is easy to fix.

@agriyakhetarpal
Copy link
Member

Also, we had fixed the first patch upstream, so it shouldn't be needed either: casadi/casadi#3759

@traversaro
Copy link
Contributor

Also, we had fixed the first patch upstream, so it shouldn't be needed either: casadi/casadi#3759

Sorry, I am not sure I am following, it seems that https://github.com/conda-forge-admin/casadi-feedstock/blob/conda_forge_admin_114/recipe/patches/0001-Modernize-Python-find_package.patch is not an overlap of casadi/casadi#3759, or I am missing something?

@traversaro
Copy link
Contributor

I see that the second patch is failing to apply. CasADi changed the line from friend to inline friend, which is easy to fix.

For reference, this casadi/casadi@3a30c9e, casadi/casadi#3724 and casadi/casadi#3723 are the related pointers. From what I understand we can probably just drop the patch at all.

@traversaro
Copy link
Contributor

@agriyakhetarpal for the future, if you depend on casadi and you are interested in being an additional mantainer for the feedstock to reduce the bus factor, feel free to add yourself, thanks!

@traversaro traversaro changed the title ENH: update package version Update CasADi to 3.6.6 Sep 2, 2024
@traversaro traversaro changed the title Update CasADi to 3.6.6 Update CasADi to 3.6.6 and enable fatrop dependency on Linux Sep 2, 2024
@agriyakhetarpal
Copy link
Member

agriyakhetarpal commented Sep 2, 2024

Hi @traversaro, yes, patch 0001 wasn't an overlap of casadi/casadi#3759 – I might not have looked at the entire patch, sorry. The PR fixed the issue where the Python include directory was being searched for by the CasadiGetPythonVersionMajor CMake function, so I think the relevant parts could be restored. But yes, it's easier to just say "Python 3" is the major Python version since Python 2 bindings are not being generated.

I see that the second patch is failing to apply. CasADi changed the line from friend to inline friend, which is easy to fix.

For reference, this casadi/casadi@3a30c9e, casadi/casadi#3724 and casadi/casadi#3723 are the related pointers. From what I understand we can probably just drop the patch at all.

Sure, this makes sense to me!

@agriyakhetarpal for the future, if you depend on casadi and you are interested in being an additional mantainer for the feedstock to reduce the bus factor, feel free to add yourself, thanks!

I'd love to be an additional maintainer – thanks for the offer! I will say that I don't fully understand the CasADi build procedure in depth, but I'll provide a helping hand to you and other maintainers wherever I can. I'll trigger a bot PR for this.

As for the failing Linux builds here, the location of ocp/OCPAbstract.hpp should be fixable enough, just changing to fatrop/ocp/OCPAbstract.hpp should fix it 😄 Edit: and I notice my suggestion is present in https://github.com/NixOS/nixpkgs/blob/3007f981ee958d8e7607a7c5a2de09e634cafc4c/pkgs/by-name/ca/casadi/package.nix#L69-L72 already, so we're good!

@traversaro
Copy link
Contributor

As for the failing Linux builds here, the location of ocp/OCPAbstract.hpp should be fixable enough, just changing to fatrop/ocp/OCPAbstract.hpp should fix it 😄 Edit: and I notice my suggestion is present in https://github.com/NixOS/nixpkgs/blob/3007f981ee958d8e7607a7c5a2de09e634cafc4c/pkgs/by-name/ca/casadi/package.nix#L69-L72 already, so we're good!

Thanks, I proposed a fix upstream in casadi/casadi#3832 .

@traversaro traversaro added the automerge Merge the PR when CI passes label Sep 2, 2024
@traversaro
Copy link
Contributor

@agriyakhetarpal feel free to check the PR, as far as I can see I think it is ready for merge.

@github-actions github-actions bot merged commit a187952 into conda-forge:main Sep 2, 2024
26 checks passed
Copy link
Contributor

github-actions bot commented Sep 2, 2024

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

@agriyakhetarpal
Copy link
Member

Thanks for upstreaming it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants