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.

Sign up for GitHub

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

Jump to bottom

Librdkafka snappy #7469

Merged
merged 6 commits into from
Aug 8, 2019
Merged

Librdkafka snappy #7469

merged 6 commits into from
Aug 8, 2019

Conversation

rotrida
Copy link
Contributor

@rotrida rotrida commented Jul 30, 2019

No description provided.

@myd7349 myd7349 mentioned this pull request Jul 30, 2019
Closed
@myd7349
Copy link
Contributor

myd7349 commented Aug 1, 2019 •
edited
Loading

According the log, the failure is caused by the WITH_SSL this time. It is defined as:
https://github.com/edenhill/librdkafka/blob/d443b4f025058172d2c8568376e36e54aba0b8b4/src/win32_config.h#L36

In vcpkg, librdkafka doesn't need OpenSSL by default(it is an optional feature), so we should not turn off WITHOUT_WIN32_CONFIG (My fault again). So I think your original change is good.

myd7349
myd7349 reviewed Aug 1, 2019
View reviewed changes
ports/librdkafka/CONTROL Outdated

Feature: snappy
Description: Build with snappy
Build-Depends: snappy
Copy link
Contributor

@myd7349 myd7349 Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Please consider removing this line as librdkafka does not need Google's snappy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I will change it.

myd7349
myd7349 suggested changes Aug 1, 2019
View reviewed changes
ports/librdkafka/portfile.cmake Outdated
@@ -17,6 +17,7 @@ vcpkg_check_features(
ssl WITH_SSL
zlib WITH_ZLIB
zstd WITH_ZSTD
snappy WITH_SNAPPY
Copy link
Contributor

Choose a reason for hiding this comment

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

Please consider using spaces instead of tabs here to keep consistency.

myd7349
myd7349 reviewed Aug 1, 2019
View reviewed changes
ports/librdkafka/CONTROL Outdated
@@ -1,5 +1,5 @@
Source: librdkafka
Version: 1.1.0
Version: 1.1.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Bump the version number as 1.1.0-1.

@LilyWangL
Copy link
Contributor

Related issue: #7454

@cbezault cbezault merged commit 73fa039 into microsoft:master Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@myd7349 myd7349 myd7349 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rotrida @myd7349 @LilyWangL @cbezault