Skip to content

Conversation

@oeqqwq
Copy link
Member

@oeqqwq oeqqwq commented Apr 11, 2025

No description provided.

@oeqqwq oeqqwq requested a review from huocun-ant April 11, 2025 02:40
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Hello @oeqqwq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request aims to remove what are described as useless files, specifically a set of patches related to grpc and ippcp. The patches being removed appear to address changes related to boringssl and openssl dependencies, as well as some compiler flag configurations for ippcp.

Highlights

  • Dependency Update Reversal: The patch bazel/patches/grpc_1.66.patch removes changes that switched from boringssl to openssl in the third_party/BUILD file.
  • Dependency Update Reversal: The patch bazel/patches/grpc_module_file.patch removes changes that switched from boringssl to openssl in the MODULE.bazel file.
  • Compiler Flag Reconfigurations: The patch bazel/patches/ippcp.patch removes a series of changes to compiler flags within the ippcp build configuration files, including modifications to security flags and warning suppressions.

Changelog

Click here to see the changelog
  • bazel/patches/grpc_1.66.patch
    • Removes a patch that previously switched the libssl and libcrypto aliases from @boringssl//:ssl and @boringssl//:crypto to @openssl//:ssl and @openssl//:crypto respectively in third_party/BUILD.
  • bazel/patches/grpc_module_file.patch
    • Removes a patch that previously updated the MODULE.bazel file to depend on openssl version 3.3.2.bcr.1 instead of boringssl version 0.0.0-20230215-5c22014.
  • bazel/patches/ippcp.patch
    • Removes modifications to sources/cmake/linux/GNU8.2.0.cmake, including adding -fuse-ld=bfd to LINK_FLAG_DYNAMIC_LINUX, unsetting _FORTIFY_SOURCE, and adding warning suppressions.
    • Removes modifications to sources/cmake/macosx/AppleClang11.0.0.cmake, including removing security linker flags, unsetting _FORTIFY_SOURCE, and adding warning suppressions.
    • Removes a change to sources/cmake/macosx/common.cmake that set OS_DEFAULT_COMPILER to AppleClang11.0.0.
    • Removes modifications to sources/ippcp/crypto_mb/src/cmake/linux/GNU.cmake, including adding -fuse-ld=bfd to LINK_FLAGS_DYNAMIC, unsetting _FORTIFY_SOURCE, and adding warning suppressions.
    • Removes modifications to sources/ippcp/crypto_mb/src/cmake/macosx/AppleClang.cmake, including removing security linker flags and unsetting _FORTIFY_SOURCE, and adding warning suppressions.
    • Removes modifications to sources/cmake/linux/Clang9.0.0.cmake, including unsetting _FORTIFY_SOURCE and adding warning suppressions to CMAKE_C_FLAGS_RELEASE.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


A patch, once applied with care,
Now departs, leaving code bare.
Useless, it seems,
Like forgotten dreams,
Removed to lighten the load there.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request aims to remove useless files, specifically patches related to boringssl and ippcp. The changes involve switching from boringssl to openssl and removing some compiler flag modifications. The review focuses on the correctness and potential impact of these removals.

Summary of Findings

  • boringssl to openssl switch: The patch removes boringssl and switches to openssl. This change should be carefully considered for compatibility and security implications. Ensure that all dependencies and configurations are updated accordingly.
  • Removal of compiler flag modifications: The patch removes several compiler flag modifications related to ippcp. It's important to understand the purpose of these flags and ensure that their removal doesn't negatively impact performance or security.
  • Potential impact on build process: The removal of these patches might affect the build process. It's crucial to verify that the build still works as expected after these changes.

Merge Readiness

The pull request involves significant changes, including switching from boringssl to openssl and removing compiler flag modifications. It is recommended to thoroughly test the changes and address any potential compatibility or security issues before merging. Due to the potential impact of these changes, I recommend that the pull request not be merged until these are addressed. I am unable to approve the pull request in any circumstance, and users should have others review and approve this code before merging.

Copy link
Collaborator

@huocun-ant huocun-ant left a comment

Choose a reason for hiding this comment

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

LGTM

@oeqqwq oeqqwq merged commit c30c079 into main Apr 11, 2025
13 checks passed
@oeqqwq oeqqwq deleted the oeqqwq/remove_patches branch April 11, 2025 03:18
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants