-
Notifications
You must be signed in to change notification settings - Fork 2k
[ENH] Cargo cult the ASAN from hosted. #5936
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
It is definitely not enabled, so try again.
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
Enable ASAN build path adjustments in Updates to The binary packaging step now derives the correct Key Changes• Set Affected Areas• rust/Dockerfile This summary was automatically generated by @propel-code-bot |
| if [ "$RELEASE_MODE" = "1" ]; then mv target/release/compaction_service ./compaction_service; else mv target/debug/compaction_service ./compaction_service; fi | ||
|
|
||
| build_target=$( [ "${ADDRESS_SANITIZER}" = "1" ] && echo '--target x86_64-unknown-linux-gnu' || echo '' ) && \ | ||
| if [ "$RELEASE_MODE" = "1" ]; then cargo build ${build_target} --workspace $(printf -- '--exclude %s ' $EXCLUDED_PACKAGES) --release; else cargo build ${build_target} --workspace $(printf -- '--exclude %s ' $EXCLUDED_PACKAGES); fi && \ |
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.
[Maintainability] To avoid repeating the long cargo build command, you can use command substitution to conditionally add the --release flag. This makes the line more concise and easier to maintain.
Context for Agents
To avoid repeating the long `cargo build` command, you can use command substitution to conditionally add the `--release` flag. This makes the line more concise and easier to maintain.
File: rust/Dockerfile
Line: 59There 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.
later
Description of changes
ASAN is definitely not enabled, so try again.
Test plan
Staging
Migration plan
N/A
Observability plan
N/A
Documentation Changes
N/A