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

Added placeholders for MADV defines #10881

Closed
wants to merge 2 commits into from

Conversation

kwek20
Copy link
Contributor

@kwek20 kwek20 commented Oct 26, 2022

Cross compiling rocksdb with rust bindings to android leads to an error since 7.4.0 (Incusion of madvise)
This is due to missing placeholders for non-linux platforms.

This PR adds the missing placeholders.

See rust-rocksdb/rust-rocksdb#697 for the specific error thrown.

I have just completed the CLA :)

Copy link
Contributor

@ajkr ajkr left a comment

Choose a reason for hiding this comment

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

LGTM

@facebook-github-bot
Copy link
Contributor

@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kwek20
Copy link
Contributor Author

kwek20 commented Oct 26, 2022

What about the build error/timeout @ajkr

@ajkr
Copy link
Contributor

ajkr commented Oct 26, 2022

It looks unrelated. I will rerun it.

@kwek20
Copy link
Contributor Author

kwek20 commented Oct 31, 2022

LGTM @ajkr. Whats your sprint size like for a new release? So i can give an ETA to my team

@facebook-github-bot
Copy link
Contributor

@kwek20 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@ajkr has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@ajkr
Copy link
Contributor

ajkr commented Oct 31, 2022

This should appear in the 7.9 release, which will likely be tagged around the beginning of December.

rriski added a commit to rriski/rocksdb that referenced this pull request Nov 18, 2023
Changes from facebook#10881 broke FreeBSD builds with:

    env/io_posix.h:39:9: error: 'POSIX_MADV_NORMAL' macro redefined [-Werror,-Wmacro-redefined]

This commit fixes FreeBSD builds by ignoring MADV defines.
rriski added a commit to rriski/rocksdb that referenced this pull request Nov 18, 2023
Fixes facebook#11218

Changes from facebook#10881 broke FreeBSD builds with:

    env/io_posix.h:39:9: error: 'POSIX_MADV_NORMAL' macro redefined [-Werror,-Wmacro-redefined]

This commit fixes FreeBSD builds by ignoring MADV defines.
rriski added a commit to rriski/rocksdb that referenced this pull request Nov 20, 2023
Fixes facebook#11218

Changes from facebook#10881 broke FreeBSD builds with:

    env/io_posix.h:39:9: error: 'POSIX_MADV_NORMAL' macro redefined [-Werror,-Wmacro-redefined]

This commit fixes FreeBSD builds by ignoring MADV defines.
facebook-github-bot pushed a commit that referenced this pull request Nov 20, 2023
Summary:
Fixes #11218

Changes from #10881 broke FreeBSD builds with:

    env/io_posix.h:39:9: error: 'POSIX_MADV_NORMAL' macro redefined [-Werror,-Wmacro-redefined]

This commit fixes FreeBSD builds by ignoring MADV defines.

Pull Request resolved: #12078

Reviewed By: cbi42

Differential Revision: D51452802

Pulled By: ajkr

fbshipit-source-id: 0a1f5a90954e7d257a95794277a843ac77f3a709
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.

3 participants