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

Build on OpenBSD #79

Merged
merged 5 commits into from
Feb 25, 2022
Merged

Build on OpenBSD #79

merged 5 commits into from
Feb 25, 2022

Conversation

klemensn
Copy link
Contributor

@klemensn klemensn commented Dec 14, 2021

These are required to build on OpenBSD/amd64 7.0 -CURRENT.

Both net/tg_owt and net/tdesktop have been ported to OpenBSD and widely tested already.
tg_owt is built as shared library and I'd like to avoid carrying local patches as best as possible.

@klemensn klemensn changed the title openbsd Build on OpenBSD Dec 14, 2021
@klemensn klemensn force-pushed the openbsd branch 2 times, most recently from c2a187f to b42ece1 Compare December 15, 2021 22:37
@klemensn
Copy link
Contributor Author

All comments are addressed, this builds on OpenBSD, does not effect other non-POSIX platforms and should still build fine on POSIX platforms.

Thanks for the feedback.

@ilya-fedin
Copy link
Contributor

Looks like I found where pid_t comes from: https://man7.org/linux/man-pages/man2/gettid.2.html
Apparently, it should be

#elif defined(WEBRTC_POSIX)
#if defined(WEBRTC_MAC) || defined(WEBRTC_IOS)
typedef mach_port_t PlatformThreadId;
#elif defined(WEBRTC_LINUX)
typedef pid_t PlatformThreadId;
#else
typedef intptr_t PlatformThreadId;
#endif
typedef pthread_t PlatformThreadRef;

@klemensn
Copy link
Contributor Author

I have amended it once more, but you might as well commit this on your own in an OpenBSD unrelated commit, imho.

@klemensn
Copy link
Contributor Author

Brief reminder. Anything else that comes to mind or is this good enough to go in?

@ilya-fedin
Copy link
Contributor

I have no more suggestions, so we have to wait until @john-preston will have time to look on that

@klemensn
Copy link
Contributor Author

Thanks, good to know.

@ilya-fedin
Copy link
Contributor

Please, no unneeded changes (such as adding extra logging)

@klemensn
Copy link
Contributor Author

@john-preston any feedback on this?

I just rebased it onto master after your libvpx changes and everything builds fine on OpenBSD.
It would be great to get these merged eventually so we can stop carrying local patches.

Otherwise building as Linux results in compile errors like symbol
redifinition;  the code is already OpenBSD aware.
Same as the others.
pthread_self(3) returns a phtread_t (aka. pthread *) value.

With @ilya-fedin:  Redefine thread IDs as `intptr_t` on POSIX platforms
unless platform specific functions/types exist so as to avoid the
following warning on OpenBSD/amd64 7.0 using clang 11.1.0:
```
platform_thread_types.cc:57:10: error: cast from pointer to smaller type 'rtc::PlatformThreadId' (aka 'int') loses information
  return reinterpret_cast<PlatformThreadId>(pthread_self());
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           1 error generated.
```

At least Solaris defines `pid_t` as `long`, i.e. that warning would not
occur.

OpenBSD defines `pid_t` as `int`.

Linux uses its non-portable gettid(3) returing a `pid_t` value,
so stick with that.
@klemensn
Copy link
Contributor Author

klemensn commented Feb 9, 2022

Rebased once more after conflicts in cmake/libusrtsctp.cmake.

@ilya-fedin
Copy link
Contributor

@klemensn what's your native language?

Suggested by @ilya-fedin, thanks!

Co-authored-by: ilya-fedin <[email protected]>
@john-preston john-preston merged commit a264028 into desktop-app:master Feb 25, 2022
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