-
Notifications
You must be signed in to change notification settings - Fork 73
fix: update build dependencies and enhance security flags in debian f… #1128
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
Conversation
…iles Log: Change-Id: I063521901d7b64e0a96bb4dd8887be31c9d07491
|
Warning
详情 {
"export": {
"debian/rules": {
"b": [
"export DEB_BUILD_MAINT_OPTIONS = hardening=+all",
"export DEB_CFLAGS_MAINT_APPEND = -Wall -fPIE -fstack-protector-strong -D_FORTIFY_SOURCE=1 -fPIC",
"export DEB_CXXFLAGS_MAINT_APPEND = -Wall -fPIE -fstack-protector-strong -D_FORTIFY_SOURCE=1 -fPIC",
"export DEB_LDFLAGS_MAINT_APPEND = -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,-z,noexecstack -pie"
]
}
}
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Commenting out the -fPIE and -Wl,--as-needed flags removes important security hardening—please clarify why they were removed or reapply them per-target via target_compile_options.
- Disabling CMAKE_INSTALL_RPATH may break runtime library lookups on install; if you’re switching to a different rpath strategy, please document the new approach.
- Instead of tweaking CMAKE_CXX_FLAGS globally, consider using target_compile_options to apply compiler/linker flags only to the targets that need them.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Commenting out the -fPIE and -Wl,--as-needed flags removes important security hardening—please clarify why they were removed or reapply them per-target via target_compile_options.
- Disabling CMAKE_INSTALL_RPATH may break runtime library lookups on install; if you’re switching to a different rpath strategy, please document the new approach.
- Instead of tweaking CMAKE_CXX_FLAGS globally, consider using target_compile_options to apply compiler/linker flags only to the targets that need them.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR refines the CMake build setup by removing hard-coded RPATH and explicit PIE flags, and brings Debian packaging in line with updated build dependencies and security hardening requirements. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note
详情{
"debian/control": [
{
"line": "Homepage: http://www.deepin.org",
"line_number": 49,
"rule": "S35",
"reason": "Url link | 6fe814dfb7"
}
]
} |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deepin-mozart The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
…iles
Log:
Change-Id: I063521901d7b64e0a96bb4dd8887be31c9d07491
Summary by Sourcery
Refresh Debian packaging with updated build dependencies and enhanced security hardening flags, and simplify CMakeLists by removing manual rpath and CXX_FLAGS settings
Enhancements:
Build: