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 pthread related build busters in glog code. #866

Merged

Conversation

acolwell
Copy link
Collaborator

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. Additionally, make sure you've done all of these things:

PR Description

What type of PR is this? (Check one of the boxes below)

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change which does not add functionality nor fixes a bug but improves Natron in some way)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • My change requires a change to the documentation
    • I have updated the documentation accordingly

What does this pull request do?

Fixes port.h to more closely match upstream glog code which also fixes build issues related to pthreads on MSYS2 builds.

The build issues are ultimately caused by different headers in the glog code including different config.h files. Some headers
like libs/glog/src/windows/port.h include the config.h that are in the same directory, but files like libs/glog/src/base/mutex.h
do not have a config.h in the same directory. This causes the compiler to fallback to searching the include paths. When this
happens, the gflags directories are scanned before the glog directories because glog depends on gflags. This results in the
gflags config.h being used sometimes. The gflag config.h defines HAVE_PTHREADS which in turn causes glogs' mutex.h to include pthread.h. The inclusion of pthread.h then causes issues with the thread related function declarations in port.h.

Without this change the build fails because we get "multiple definition" errors for the thread related definitions in port.h. This
change fixes this issue by allowing the code to just use the pthread header if HAVE_PTHREADS is defined and fallback on the alternate definitions if it is not. It also causes the code to be a closer match to upstream glog which could facilitate
unforking glog if that ever becomes desirable in the future.

Have you tested your changes (if applicable)? If so, how?

Yes. The glog code builds fine on a fresh MSYS2 install with this change.

Fixes port.h to more closely match upstream glog code which also
fixes build issues related to pthreads on MSYS2 builds.
Copy link
Member

@devernay devernay left a comment

Choose a reason for hiding this comment

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

LGTM

@devernay devernay merged commit 376747e into NatronGitHub:RB-2.5 Feb 22, 2023
@acolwell acolwell deleted the Fix_Windows_glog_build_busters branch February 23, 2023 01:42
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.

None yet

2 participants