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

Use CMake variables for paths in pkg-config files #31671

Merged
merged 1 commit into from
Jan 17, 2023
Merged

Use CMake variables for paths in pkg-config files #31671

merged 1 commit into from
Jan 17, 2023
+2 −2

Conversation

musicinmybrain
Copy link
Contributor

Use @gRPC_INSTALL_LIBDIR@ for libdir; this fixes an incorrect -L/usr/lib on multilib Linux systems where that is the 32-bit library path and the correct path is /usr/lib64.

Use @gRPC_INSTALL_INCLUDEDIR@ for consistency.

@jtattermusch jtattermusch added kokoro:run release notes: yes Indicates if PR needs to be in release notes labels Nov 18, 2022
@jtattermusch jtattermusch requested a review from veblush November 18, 2022 09:18
@jtattermusch
Copy link
Contributor

@veblush can you please review since you've been dealing with pkgconfig recently?

@markdroth markdroth assigned veblush and unassigned markdroth Nov 18, 2022
@veblush
Copy link
Contributor

veblush commented Nov 18, 2022

distribtest.cpp_linux_x64_stretch_cmake_pkgconfig test failed because it cannot find the header file.

@musicinmybrain
Copy link
Contributor Author

In the Fedora Linux grpc package with this PR applied as a patch, the paths come out correctly as includedir=/usr/include, but the spec file is setting -DgRPC_INSTALL_BINDIR:PATH=%{_bindir}, which would be -DgRPC_INSTALL_BINDIR:PATH=/usr/include, and so on. However, I see that these are relative paths by default, so I probably need to stop doing that and see what happens. I suspect @veblush’s suggestion above will prove correct. I will revise the PR once I have a chance to do a little more local testing.

@musicinmybrain
Use CMake variables for paths in pkg-config files
beb3d7c 
Use @gRPC_INSTALL_LIBDIR@ for libdir; this fixes an incorrect
-L/usr/lib on multilib Linux systems where that is the 32-bit library
path and the correct path is /usr/lib64.

Use @gRPC_INSTALL_INCLUDEDIR@ for consistency.
@musicinmybrain
Copy link
Contributor Author

Force-pushed with changes suggested in review. Hopefully the distribtests will be happy now.

@veblush veblush self-requested a review January 17, 2023 20:50
@veblush veblush merged commit c641b82 into grpc:master Jan 17, 2023
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Jan 18, 2023
wanlin31 pushed a commit that referenced this pull request May 18, 2023
@musicinmybrain @wanlin31
Use CMake variables for paths in pkg-config files (#31671)
befa924 
Use @gRPC_INSTALL_LIBDIR@ for libdir; this fixes an incorrect
-L/usr/lib on multilib Linux systems where that is the 32-bit library
path and the correct path is /usr/lib64.

Use @gRPC_INSTALL_INCLUDEDIR@ for consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yashykt yashykt yashykt approved these changes

@veblush veblush veblush approved these changes

@jtattermusch jtattermusch Awaiting requested review from jtattermusch

@apolcyn apolcyn Awaiting requested review from apolcyn apolcyn is a code owner

Assignees

@veblush veblush

Labels
bloat/none imported Specifies if the PR has been imported to the internal repository lang/c++ per-call-memory/neutral per-channel-memory/neutral release notes: yes Indicates if PR needs to be in release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@musicinmybrain @jtattermusch @veblush @yashykt @grpc-kokoro @markdroth