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

update(cmake): Install libraries into CMAKE_INSTALL_LIBDIR #1101

Merged
merged 1 commit into from Nov 10, 2023
Merged

update(cmake): Install libraries into CMAKE_INSTALL_LIBDIR #1101

merged 1 commit into from Nov 10, 2023

Conversation

geraldcombs
Copy link
Contributor

Install libraries into CMAKE_INSTALL_LIBDIR. This appears to be more compliant with the FHS[1] and simplifies linking when BUILD_SHARED_LIBS is enabled.

[1] https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s06.html

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap-engine-udig

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

This avoids having to mess with LD_LIBRARY_PATH or -Wl,-rpath when using shared libs.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@poiana poiana requested review from gnosek and hbrueckner May 17, 2023 22:35
@poiana poiana added the size/S label May 17, 2023
@FedeDP
Copy link
Contributor

FedeDP commented May 18, 2023

/milestone 0.12.0

@poiana poiana added this to the 0.12.0 milestone May 18, 2023
@Andreagit97 Andreagit97 modified the milestones: 0.12.0, libs-backlog Jun 7, 2023
@geraldcombs geraldcombs mentioned this pull request Jul 16, 2023
Closed
@geraldcombs
Copy link
Contributor Author

We might want to close this in favor of https://salsa.debian.org/debian/falcosecurity-libs/-/blob/master/debian/patches/fix-library-install-path.patch

@poiana
Copy link
Contributor

poiana commented Oct 14, 2023

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

@geraldcombs
Copy link
Contributor Author

We might want to close this in favor of https://salsa.debian.org/debian/falcosecurity-libs/-/blob/master/debian/patches/fix-library-install-path.patch

I replaced the original PR changes with the patch.

@geraldcombs geraldcombs force-pushed the cmake-default-libdir branch 2 times, most recently from dfb05ac to 362e4a9 Compare October 23, 2023 16:19
@geraldcombs
update(cmake): Install libraries into CMAKE_INSTALL_LIBDIR
8897a0c 
Pull in and update a patch from Dima Kogan which installs libraries into
CMAKE_INSTALL_LIBDIR:

https://salsa.debian.org/debian/falcosecurity-libs/-/blob/master/debian/patches/fix-library-install-path.patch

This simplifies things when BUILD_SHARED_LIBS is enabled since it
reduces the need to mess with LD_LIBRARY_PATH or DYLD_LIBRARY_PATH.

Signed-off-by: Gerald Combs <gerald@wireshark.org>
@poiana poiana added the lgtm label Nov 10, 2023
@poiana
Copy link
Contributor

poiana commented Nov 10, 2023

LGTM label has been added.

Git tree hash: 6d285c4fad95645e1cc985e630ea64eb3654210c

@FedeDP
Copy link
Contributor

FedeDP commented Nov 10, 2023

/milestone 0.14.0

@poiana poiana modified the milestones: TBD, 0.14.0 Nov 10, 2023
@FedeDP
Copy link
Contributor

FedeDP commented Nov 10, 2023

cc @terylt @araujof you might be interested in this one :)

FedeDP
FedeDP approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Nov 10, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, geraldcombs, gnosek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit 289df4f into falcosecurity:master Nov 10, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnosek gnosek gnosek approved these changes

@FedeDP FedeDP FedeDP approved these changes

@hbrueckner hbrueckner Awaiting requested review from hbrueckner

Assignees

@gnosek gnosek

@FedeDP FedeDP

Labels
approved area/build dco-signoff: yes kind/cleanup lgtm lifecycle/stale release-note-none size/S
Projects
None yet
Milestone
0.14.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@geraldcombs @FedeDP @poiana @gnosek @Andreagit97