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

Fix libuv build #857

Merged
merged 3 commits into from
Jul 4, 2022
Merged

Fix libuv build #857

merged 3 commits into from
Jul 4, 2022

Conversation

vpalmisano
Copy link
Contributor

Fixes a build error:

subprojects/libuv-v1.43.0/libuv.a.p/src_unix_stream.c.o.d -o subprojects/libuv-v1.43.0/libuv.a.p/src_unix_stream.c.o -c ../../../subprojects/libuv-v1.43.0/src/unix/stream.c
../../../subprojects/libuv-v1.43.0/src/unix/stream.c: In function ‘uv__write’:
../../../subprojects/libuv-v1.43.0/src/unix/stream.c:929:3: error: C++ style comments are not allowed in ISO C90

Fixes a build error:
```
subprojects/libuv-v1.43.0/libuv.a.p/src_unix_stream.c.o.d -o subprojects/libuv-v1.43.0/libuv.a.p/src_unix_stream.c.o -c ../../../subprojects/libuv-v1.43.0/src/unix/stream.c
../../../subprojects/libuv-v1.43.0/src/unix/stream.c: In function ‘uv__write’:
../../../subprojects/libuv-v1.43.0/src/unix/stream.c:929:3: error: C++ style comments are not allowed in ISO C90
```
@nazar-pc
Copy link
Collaborator

nazar-pc commented Jul 4, 2022

Hm... this must be regression in Meson, c_std=c99 is already specified in libuv wrap: https://github.com/mesonbuild/wrapdb/blob/master/subprojects/packagefiles/libuv/meson.build#L4

Can you try bumping libuv wrap to 1.44.1 instead? It switched from c89 to c99, maybe that is related somehow...
mesonbuild/wrapdb#418

@vpalmisano
Copy link
Contributor Author

Can you try bumping libuv wrap to 1.44.1 instead? It switched from c89 to c99, maybe that is related somehow...
mesonbuild/wrapdb#418

It works :)

Copy link
Collaborator

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Looks like something wasn't accounted for in older versions of Meson, but now it is properly checked

@nazar-pc nazar-pc merged commit e8d6d53 into versatica:v3 Jul 4, 2022
@ibc
Copy link
Member

ibc commented Jul 4, 2022

I forgot to say that PRs must include an entry in CHANGELOG.md. I'll add it in v3 directly.

@dsdolzhenko
Copy link
Contributor

Hm... this must be regression in Meson

@nazar-pc Am I right that the regression is in Meson version 0.63.0 and the build started failing right after it was released?

What do you think about specifying exact version in the Makefile to make builds more reproducible?

@nazar-pc
Copy link
Collaborator

nazar-pc commented Jul 4, 2022

The idea was that newer versions of Meson should not regress and bring improvements, but I tend to agree that we can lock the version until Meson reached 1.0 to prevent such breakage in the future.

@dsdolzhenko
Copy link
Contributor

I tend to agree that we can lock the version until Meson reached 1.0

Personally, I would've prefer to keep it locked even after 1.0 release and update manually whenever it's needed. Strictly speaking no one promises that after 1.0 there won't be any regressions ;)

Shall I create a PR?

@nazar-pc
Copy link
Collaborator

nazar-pc commented Jul 4, 2022

Shall I create a PR?

Yes, please do

@eli-schwartz
Copy link

This isn't really a Meson bug, it's a bug in the libuv wrap itself which was fixed at the end of April. :p

The WrapDB does get sanity-checked with new prereleases of Meson (available in advance of any new feature release, you can install this in CI to catch errors early), so updating wraps should likely be the first thing to check (and do for general maintenance?)

In this case, Meson used to totally ignore the std of the subproject and use the one from the main project, which for mediasoup was a lot newer than c89. The new Meson release actually permits subprojects to specify their own std: https://mesonbuild.com/Release-notes-for-0-63-0.html#compiler-options-can-be-set-per-subproject

libuv just happened to specify one that did not actually work. :(

prlanzarin pushed a commit to mconf/mediasoup that referenced this pull request Jul 5, 2022
hkirat pushed a commit to gathertown/mediasoup that referenced this pull request Jul 5, 2022
ibc added a commit that referenced this pull request Jul 11, 2022
* Fix release contents by including `meson_options.txt` (PR #863).
* Pin the version of meson PR #859).
* Fix libuv build (PR #857).
uwueviee pushed a commit to uwueviee/mediasoup that referenced this pull request Jul 18, 2022
harkiratsing pushed a commit to harkiratsing/mediasoup that referenced this pull request Jul 20, 2022
harkiratsing pushed a commit to harkiratsing/mediasoup that referenced this pull request Jul 20, 2022
* Upgrade libuv to 1.44.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants