Skip to content

Dependency: Remove async_ssl #17136

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

Open
wants to merge 2 commits into
base: compatible
Choose a base branch
from
Open

Dependency: Remove async_ssl #17136

wants to merge 2 commits into from

Conversation

glyh
Copy link
Member

@glyh glyh commented May 4, 2025

async_ssl is not used at all(not even transitive dependency it seems), so we should be able to remove it.

Moreover, this package has compilation issue and it's one of the blocker for non-nix setup on Arch Linux.

EDIT:

To test this is indeed true:
In a mina's repo root:

opam switch create . 4.14.0
opam switch import --switch=. opam.export --dry-run

The later command should give you a list of all actual dependency needed. BTW, We can see that duration is not tracked by the opam.export file.
Actually I'm trying this on 4.14.2 and patched the export file to use 4.14.2 as well as I can't build 4.14.0 on Arch Linux.

@glyh
Copy link
Member Author

glyh commented May 4, 2025

!ci-build-me

@glyh glyh marked this pull request as ready for review May 4, 2025 07:03
@glyh glyh requested a review from a team as a code owner May 4, 2025 07:03
@glyh
Copy link
Member Author

glyh commented May 4, 2025

!ci-toolchain-me

@dannywillems
Copy link
Member

!ci-bypass-changelog

@dannywillems
Copy link
Member

I'm very surprised it is unused. I'm initially worried that it is used indirectly. But I'll have a deeper look later.

@glyh
Copy link
Member Author

glyh commented May 4, 2025

I guess the related complexity goes into go's libp2p stuff? Unsure

@dannywillems
Copy link
Member

!ci-nightly-me

@dannywillems
Copy link
Member

!ci-build-me

@dannywillems
Copy link
Member

Cohttp_async uses the Async library and async_ssl to handle HTTPS connections.

Source: https://github.com/mirage/ocaml-cohttp.

Copy link
Member

@dannywillems dannywillems left a comment

Choose a reason for hiding this comment

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

It seems it is used by Cohttp_async, used in our GraphQL client.

@glyh
Copy link
Member Author

glyh commented May 6, 2025

This is weird as OPAM didn't resolve this as a dependency. Is it an optional dependency?

@glyh
Copy link
Member Author

glyh commented May 6, 2025

These failing message is not very helpful

--
-- Running: [email protected]/o1labs-192920/mina-toolchain@sha256:c9f16769602bd90930e74ca61d8d303bf7533314063ae754f78b10ea481a98d4 ( sudo chown -R opam . ) --
--
docker: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?.
See 'docker run --help'.
🚨 Error: The command exited with status 125
user command error: exit status 125

@dannywillems
Copy link
Member

These failing message is not very helpful

--
-- Running: [email protected]/o1labs-192920/mina-toolchain@sha256:c9f16769602bd90930e74ca61d8d303bf7533314063ae754f78b10ea481a98d4 ( sudo chown -R opam . ) --
--
docker: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?.
See 'docker run --help'.
🚨 Error: The command exited with status 125
user command error: exit status 125

This is solved by restarting the job.

@glyh
Copy link
Member Author

glyh commented May 6, 2025

These failing message is not very helpful

--
-- Running: [email protected]/o1labs-192920/mina-toolchain@sha256:c9f16769602bd90930e74ca61d8d303bf7533314063ae754f78b10ea481a98d4 ( sudo chown -R opam . ) --
--
docker: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?.
See 'docker run --help'.
🚨 Error: The command exited with status 125
user command error: exit status 125

This is solved by restarting the job.

Not really https://buildkite.com/o-1-labs-2/mina-o-1-labs/builds/37703#0196a19f-5f9a-41fc-b318-3356581c2866

These jobs have been restarted a couple of times.

@glyh
Copy link
Member Author

glyh commented May 9, 2025

Although jobs should fail indeed. The error message makes no sense to me.

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.

3 participants