Skip to content

Package: Add osqp v1.0.0#5510

Merged
hoodmane merged 9 commits intopyodide:stablefrom
vineetbansal:stable
Mar 24, 2025
Merged

Package: Add osqp v1.0.0#5510
hoodmane merged 9 commits intopyodide:stablefrom
vineetbansal:stable

Conversation

@vineetbansal
Copy link
Contributor

Description

Added recipe for OSQP, the widely used QP solver.

@hoodmane
Copy link
Member

Looks like we're blocked by the fact that pygame-ce is failing ever since setuptools 76 came out. I disabled it on main. I'll see if I can't fix it.

To unblock ci on this PR, try:

git checkout main -- packages/pygame-ce
git commit -m "disable pygame-ce"

@vineetbansal
Copy link
Contributor Author

Thanks @hoodmane - thanks for the pointers. I don't think the CI failures are related to our change, since I noticed they're happening in other PRs too. Let me know if I can help in any way.

@hoodmane
Copy link
Member

Yeah, there's been a lot of CI chaos recently. Several different packages have had build regressions. We pin the versions of packages but not their build dependencies, so whenever setuptools makes a release a few of our packages break...

@hoodmane
Copy link
Member

@henryiii any idea about this sisl build flake we've been getting lately:

scikit_build_core.errors.CMakeNotFoundError: Could not find CMake with version >=3.20

It doesn't reproduce locally, and it is sporadic. We have cmake 3.31.4 installed in the docker image:

$ cmake --version
cmake version 3.31.4

Could this somehow be a regression from the recent scikit-build release? I guess I could try pinning an older scikit-build version and see if the problem goes away.

@ryanking13
Copy link
Member

It doesn't reproduce locally, and it is sporadic. We have cmake 3.31.4 installed in the docker image:

FYI, we skip cmake installation in isolated build environment to proxy cmake command to our pyodide wrapper. I am not sure how it can be flaky though: https://github.com/pyodide/pyodide-build/blob/ebe83b9f3fae98df36d35eaa3ef984008d0d5b81/pyodide_build/pypabuild.py#L35-L42

@hoodmane
Copy link
Member

Well now the build is successful but it seems like it is missing dependencies on at least jinja2 and scipy and probably other things.

@henryiii
Copy link
Contributor

Do you have a link to a log with that error? There's also verbose logging settings to get more info on the search. It is supposed to find the cmake wrapper, query it with -E capabilities, and then use that.

@ryanking13
Copy link
Member

ryanking13 commented Mar 17, 2025

@henryiii

This is the link to the same error that I got in other PR:

Running source pyodide_env in /root/repo/packages/boost-histogram               
Accessing CMake timed out, ignoring                                             
2025-03-16 10:28:31,420 - scikit_build_core - WARNING - Accessing CMake timed   
out, ignoring                                                                   
Traceback (most recent call last):                                              
  File                                                                          
"/usr/local/lib/python3.12/site-packages/pyproject_hooks/_in_process/_in_process
.py", line 389, in <module>                                                     
    main()                                                                      
  File                                                                          
"/usr/local/lib/python3.12/site-packages/pyproject_hooks/_in_process/_in_process
.py", line 373, in main                                                         
    json_out["return_val"] = hook(**hook_input["kwargs"])                       
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^                       
  File                                                                          
"/usr/local/lib/python3.12/site-packages/pyproject_hooks/_in_process/_in_process
.py", line 280, in build_wheel                                                  
    return _build_backend().build_wheel(                                        
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                        
  File                                                                          
"/tmp/build-env-yut9yryo/lib/python3.12/site-packages/scikit_build_core/build/__
init__.py", line 33, in build_wheel                                             
    return _build_wheel_impl(                                                   
           ^^^^^^^^^^^^^^^^^^                                                   
  File                                                                          
"/tmp/build-env-yut9yryo/lib/python3.12/site-packages/scikit_build_core/build/wh
eel.py", line 174, in _build_wheel_impl                                         
    return _build_wheel_impl_impl(                                              
           ^^^^^^^^^^^^^^^^^^^^^^^                                              
  File                                                                          
"/tmp/build-env-yut9yryo/lib/python3.12/site-packages/scikit_build_core/build/wh
eel.py", line 246, in _build_wheel_impl_impl                                    
    cmake = CMake.default_search(version=settings.cmake.version, env=os.environ)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File                                                                          
"/tmp/build-env-yut9yryo/lib/python3.12/site-packages/scikit_build_core/cmake.py
", line 63, in default_search                                                   
    raise CMakeNotFoundError(msg)                                               
scikit_build_core.errors.CMakeNotFoundError: Could not find CMake with version  
>=3.24                                                                          
                                                                                
ERROR Backend subprocess exited when trying to invoke build_wheel               
[2025-03-16 10:28:33] Failed building package boost-histogram in 109.1 seconds. 

- osqp
requirements:
run:
- jinja2
Copy link
Member

Choose a reason for hiding this comment

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

Seems that the distribution name of Jinja2 has a capital J.

Suggested change
- jinja2
- Jinja2

@henryiii
Copy link
Contributor

henryiii commented Mar 17, 2025

Accessing CMake timed out, ignoring is the problem. We have a timeout waiting for a response from cmake -E capabilities of a few seconds. Do you know how long it's takes for it to respond?

Edit: currently it's 5 seconds (https://github.com/scikit-build/scikit-build-core/blob/73ba800de2ef71a94147375a1af85097d9dd41c4/src/scikit_build_core/program_search.py#L35-L36) (10 seconds on Windows). We've had to bump it up once due to similar flakes, I think it was 2 seconds on Unix before.

@ryanking13
Copy link
Member

Accessing CMake timed out, ignoring is the problem. We have a timeout waiting for a response from cmake -E capabilities of a few seconds. Do you know how long it's takes for it to respond?

I see. We have a wrapper for CMake: a python script that will just pass forward the cmake -E capabilities call. It is a very thin wrapper, but since the Python interpreter takes some time to bootstrap, it might get timeout...

@henryiii
Copy link
Contributor

Is there something I could detect and then set a longer timeout?

@hoodmane
Copy link
Member

When I invoke cmake -E capabilities via our wrapper directly I get:

real	0m0,117s
user	0m0,088s
sys	0m0,028s

Why would it take so much longer in CI? Maybe I'll try with rerun with ssh.

@henryiii
Copy link
Contributor

CI filesystems can be slow, probably depending on load. Maybe stick a timed call in the front to see how long that takes? Maybe we could bump the timeout a bit if CI is set.

@hoodmane
Copy link
Member

In CI via rerun with ssh it is taking ~0.25 seconds with not much variance:

$ time BUILD_ENV_SCRIPTS_DIR=xx ./repo/packages/MarkupSafe/build/pywasmcross_symlinks/cmake -E capabilities > /dev/null 

real	0m0.235s
user	0m0.189s
sys	0m0.046s

@hoodmane
Copy link
Member

No harm in trying setting the timeout higher, but it makes me wonder whether something isn't more seriously broken to make this call take 20 times as long as normal.

@agriyakhetarpal
Copy link
Member

agriyakhetarpal commented Mar 17, 2025

I think the invocation might take longer when running in a multithreaded context in CI. If we're building five packages at a time, I imagine it will be slower than 0.25s...

@ryanking13
Copy link
Member

In CI via rerun with ssh it is taking ~0.25 seconds with not much variance:

We have multiple processes running in parallel during the build. Maybe it is affecting the performance?

@henryiii
Copy link
Contributor

I've seen it take more than 2 seconds in CI, which is why it's 5 seconds. And it can be even slower on Windows. Combined with a wrapper, over 5 seconds isn't unbelievable.

@hoodmane hoodmane mentioned this pull request Mar 17, 2025
8 tasks
henryiii added a commit to scikit-build/scikit-build-core that referenced this pull request Mar 17, 2025
See pyodide/pyodide#5510.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@vineetbansal
Copy link
Contributor Author

Thanks @henryiii , @hoodmane - I still see something in our tests is timing out, though this test takes around 800ms for me locally.

FAILED osqp/test_osqp.py::simple_table[firefox] - selenium.common.exceptions.TimeoutException: Message: Timed out after 20000 ms

Do we need to introduce something like what I've seen in other tests? All other recipes have this timeout>20 though, so this may not be it.

 @pytest.mark.driver_timeout(60)
 def test_ ...

@ryanking13
Copy link
Member

Do we need to introduce something like what I've seen in other tests?

Yes, please. CI environments can sometimes be very slow.

@vineetbansal
Copy link
Contributor Author

Thanks @ryanking13 , @hoodmane . Our PR looks like its failing when building scipy:

[2025-03-18 16:11:34] Failed building package scipy in 767.5 seconds.                                                                         
ERROR: cancelling buildall due to error building scipy     

Is there something else I should do/try here? Thanks for your help.

@hoodmane
Copy link
Member

I'll rerun the workflow. It's probably a flake. Maybe you should merge stable though.

@vineetbansal vineetbansal changed the title added osqp v1.0.0b4 added osqp v1.0.0 Mar 23, 2025
@vineetbansal
Copy link
Contributor Author

@hoodmane, @ryanking13 - we seem to be getting different CI errors, unrelated to our repo AFAIK, every time I make any change to this PR, so something is still flaky. Is there something we can do here (perhaps only run tests related to our package and not all of them)? I do see some PRs have been merged with failing tests, so maybe there's some other way to get our package in?

@hoodmane
Copy link
Member

Yes we don't require all tests to be green to merge but so far the build flakiness is stopping your package from being built and tested. If some specific other package is blocking the ci then we could temporarily disable it in order to be able to see your additions working.

We successfully released from this branch just a few days ago and these tests are required to pass by the deployment pipeline. So either you're getting really unlucky or something got worse in the past week.

@hoodmane
Copy link
Member

Anyways I reran the jobs again, maybe if we're lucky it'll work this time...

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Could you please update the target branch from stable to main?

never mind. I didn't read the comment above

@ryanking13
Copy link
Member

Other than that. LGTM for the code.

@hoodmane
Copy link
Member

This time the build step succeeded so we should get a result for osqp.

@hoodmane
Copy link
Member

CI status looks good. Please add a changelog entry @vineetbansal and I will merge.

@vineetbansal
Copy link
Contributor Author

@hoodmane - added. Hopefully I did the right thing there. Thanks for your assist on this!

@hoodmane
Copy link
Member

Looks good to me.

@hoodmane hoodmane changed the title added osqp v1.0.0 Package: Add osqp v1.0.0 Mar 24, 2025
@hoodmane hoodmane merged commit 4ed95bd into pyodide:stable Mar 24, 2025
19 of 23 checks passed
hoodmane pushed a commit that referenced this pull request Mar 24, 2025
Added recipe for OSQP, the widely used QP solver.

Disabled here because it's an externally built wheel for pyodide_2024_0 and
we don't have a pyodide_2025 abi available yet. Cherry-picked from stable
where it is working.
@hoodmane
Copy link
Member

Merged into stable and cherry-picked to main as 162a0e1.

@hoodmane
Copy link
Member

Thanks for your patience @vineetbansal !

hoodmane pushed a commit to hoodmane/pyodide that referenced this pull request Mar 26, 2025
Added recipe for OSQP, the widely used QP solver.

Disabled here because it's an externally built wheel for pyodide_2024_0 and
we don't have a pyodide_2025 abi available yet. Cherry-picked from stable
where it is working.
hoodmane pushed a commit to hoodmane/pyodide that referenced this pull request Apr 3, 2025
Added recipe for OSQP, the widely used QP solver.

Disabled here because it's an externally built wheel for pyodide_2024_0 and
we don't have a pyodide_2025 abi available yet. Cherry-picked from stable
where it is working.
hoodmane pushed a commit to hoodmane/pyodide that referenced this pull request Apr 3, 2025
Added recipe for OSQP, the widely used QP solver.

Disabled here because it's an externally built wheel for pyodide_2024_0 and
we don't have a pyodide_2025 abi available yet. Cherry-picked from stable
where it is working.
hoodmane pushed a commit to hoodmane/pyodide that referenced this pull request Apr 3, 2025
Added recipe for OSQP, the widely used QP solver.

Disabled here because it's an externally built wheel for pyodide_2024_0 and
we don't have a pyodide_2025 abi available yet. Cherry-picked from stable
where it is working.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants