-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Remove quarantine attribute from bindings + compiled binary #3118
Conversation
WalkthroughThe recent changes involve adding a new conditional operation in two Go source files that specifically targets macOS systems. When the code is executed on a macOS machine, it now includes a step to remove the quarantine attribute from a file. This is a security feature in macOS that can prevent files from opening or running if they were downloaded from the internet. The changes ensure compatibility with macOS by removing this attribute during the build and binding generation processes. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- v2/pkg/commands/bindings/bindings.go (1 hunks)
- v2/pkg/commands/build/build.go (1 hunks)
Additional comments: 2
v2/pkg/commands/bindings/bindings.go (1)
- 60-66: The addition of the macOS-specific code to remove the quarantine attribute from the binary using
xattr -rc
is correct and aligns with the PR's objective. The error handling is consistent with the rest of the function, which is good practice. Ensure that this change has been tested on macOS to confirm that it behaves as expected and does not introduce any side effects.v2/pkg/commands/build/build.go (1)
- 324-332: The addition of the conditional block to remove the quarantine attribute on macOS looks correct and aligns with the PR's objective. It's important to ensure that the
options.CompiledBinary
variable contains the correct path to the binary that needs the attribute removed. Additionally, the error handling and verbosity checks are in place, which is good practice.
Deploying with Cloudflare Pages
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- v2/pkg/commands/build/build.go (1 hunks)
Additional comments: 1
v2/pkg/commands/build/build.go (1)
- 324-337: The changes introduced in this hunk are well-implemented and align with the PR's objective to remove the quarantine attribute from the compiled binary on macOS. The checks for the existence of the file and verbosity level are correctly placed, and the error handling is appropriate. The code is modular, maintainable, and follows best practices.
This commit adjusts the formatting in a line of code that handles a missing binary error message. Additionally, it removes a redundant closing brace that was incorrectly situated in the code, thereby cleaning up the code structure.
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- v2/pkg/commands/build/build.go (1 hunks)
Additional comments: 1
v2/pkg/commands/build/build.go (1)
- 324-336: The changes introduced to remove the quarantine attribute from the compiled binary on macOS are correctly checking for the binary's existence before attempting to remove the attribute. This is a good practice to prevent errors from non-existent files.
stdout, stderr, err := shell.RunCommand(options.BinDirectory, "xattr", "-rc", options.CompiledBinary) | ||
if err != nil { | ||
return "", fmt.Errorf("%s - %s", err.Error(), stderr) |
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.
Consider handling the case where the xattr
command might not be available on the system. This could be done by checking the error message for a "command not found" type of error and providing a more user-friendly message or documentation reference.
Description
Some people experience an error when generating bindings related to the temporary binary having the quarantine attribute set. This unsets it.
Summary by CodeRabbit
New Features
Refactor