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 syscall deprecation warning on macOS >= 10.12 #685

Merged
merged 1 commit into from
Jul 14, 2021
Merged

Fix syscall deprecation warning on macOS >= 10.12 #685

merged 1 commit into from
Jul 14, 2021

Conversation

z-aki
Copy link
Contributor

@z-aki z-aki commented Jul 13, 2021

Fix #185

@google-cla
Copy link

google-cla bot commented Jul 13, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Jul 13, 2021
@z-aki
Copy link
Contributor Author

z-aki commented Jul 13, 2021

@googlebot I signed it!

Copy link
Collaborator

@sergiud sergiud left a comment

Choose a reason for hiding this comment

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

Currently, it is not possible to test these changes as a corresponding Mac OS X Github workflow is missing.

src/utilities.cc Outdated Show resolved Hide resolved
src/raw_logging.cc Outdated Show resolved Hide resolved
@z-aki z-aki changed the title Fix syscall deprecation warning on macOS >= 10.12 (#1) Fix syscall deprecation warning on macOS >= 10.12 Jul 13, 2021
@z-aki z-aki requested a review from sergiud July 13, 2021 12:12
@z-aki
Copy link
Contributor Author

z-aki commented Jul 13, 2021

Err is that failure my fault ? macOS guards shouldn't affect mingw..

@sergiud
Copy link
Collaborator

sergiud commented Jul 13, 2021

The failure is very likely caused by this branch.

@z-aki
Copy link
Contributor Author

z-aki commented Jul 13, 2021

Okay, so far I was building glog as part of a downstream project.
Will check separately now.

@z-aki
Copy link
Contributor Author

z-aki commented Jul 13, 2021

Okay it was operator precedence. Fixed now.

@sergiud
Copy link
Collaborator

sergiud commented Jul 14, 2021

Thanks. Could you please rebase onto current master? CI now supports macOS.

@sergiud sergiud added this to the 0.6 milestone Jul 14, 2021
@sergiud
Copy link
Collaborator

sergiud commented Jul 14, 2021

macOS builds still issue a warning, e.g.,

/Users/runner/work/glog/glog/src/raw_logging.cc:140:3: warning: 'syscall' is deprecated: first deprecated in macOS 10.12 - syscall(2) is unsupported; please switch to a supported interface

@sergiud
Copy link
Collaborator

sergiud commented Jul 14, 2021

Warning still present.

@z-aki
Copy link
Contributor Author

z-aki commented Jul 14, 2021

Shouldn't the glog project be defining MAC_OS_X_VERSION_MAX_ALLOWED since you know which OSes it will be build built on?
If you can't I can just remove the macOS version stuff and make it dependent on OS only.

@sergiud
Copy link
Collaborator

sergiud commented Jul 14, 2021

The library doesn't care about MAC_OS_X_VERSION_MAX_ALLOWED, hence it does need to be defined.

Can't you just check the presence of pthread_threadid_np in CMake using check_function_exists and use the function if it is available instead of relying on defines?

src/config.h.cmake.in Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@z-aki
Copy link
Contributor Author

z-aki commented Jul 14, 2021

nope warning's still there..
It's sometimes useful to clear cache when dealing with cmake variables.
Also, in one of the older attempts, I just set the code to

#if (defined(OS_MACOSX))
    uint64_t tid64;
    pthread_threadid_np(NULL, &tid64);
    pid_t tid = (pid_t)tid64;
...

and it built fine.
The function was introduced in macOS 10.6 so seems safe to use now.

Copy link
Collaborator

@sergiud sergiud left a comment

Choose a reason for hiding this comment

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

I have just one minor remark. You can then squash the commits.

src/utilities.cc Outdated Show resolved Hide resolved
@z-aki z-aki requested a review from sergiud July 14, 2021 14:37
@sergiud
Copy link
Collaborator

sergiud commented Jul 14, 2021

Great. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OSX 10.12 compilation warning: syscall is deprecated
2 participants