Skip to content

Patch to fix async_ssl build #26886

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

Merged
merged 5 commits into from
Nov 19, 2024
Merged

Conversation

raphael-proust
Copy link
Contributor

DO NOT MERGE: wait until the CI succeeds, then merge ocaml/opam-source-archives#34, then change the URL for the patch, then merge

@mseri
Copy link
Member

mseri commented Nov 14, 2024

I have merged the patches before checking this, sorry

@mseri
Copy link
Member

mseri commented Nov 14, 2024

But the build of async_ssl is fine on the whole matrix, there are only a few ssl issues that are unrelated as far as I can see

@raphael-proust
Copy link
Contributor Author

CI failures are:

  • linting errors that pre-date this fix
  • some ssl issues on fedora which seem unrelated to this fix
  • a core_unix issue on windows (probably needs an available: os != "win32" added to core_unix.v0.17.0)

AFAICT, this is good to merge.

@mseri
Copy link
Member

mseri commented Nov 15, 2024

Should we make this a patch -1 release instead? Or it will mess up with strict versioning of some packages?

@raphael-proust
Copy link
Contributor Author

Or it will mess up with strict versioning of some packages?

How so? I guess I can make a script that would break with this update, but I'm not sure there are standard tools around opam that would break.

But yeah, we might as well fix it just in case. Question is: should we mark the unpatched package as not available because it's broken several plateforms?

@mseri
Copy link
Member

mseri commented Nov 18, 2024

What I mean is if there are janestreet packages with version v0.17 requiring async_ssl with the =version flag. In this case I think we should merge as is, if not, I think we should merge a -1 patch. In the latter case I would not mark anything as unavailable because there could be still people happyly using them which do not need or want to move to the new one.

(This all in the spirit of path to the least breakage/change, it will change once the new maintainership policy is in place)

@raphael-proust
Copy link
Contributor Author

I see. Done, it's now a patch version (which makes sense because it's a patch I guess).

@mseri
Copy link
Member

mseri commented Nov 19, 2024

Tests have already run, the lint and many tests have already passed

@mseri
Copy link
Member

mseri commented Nov 19, 2024

Thanks

@mseri mseri merged commit 9f59ecb into ocaml:master Nov 19, 2024
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants