-
-
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 xattr via obj-c #3328
Remove xattr via obj-c #3328
Conversation
WalkthroughThis update introduces and integrates functionality for handling extended attributes (xattrs) on macOS, addressing issues with running and building applications on Mac M2 systems. It involves modifications across several files to incorporate Objective-C code through CGO for xattr removal and adjustments in build and binding generation processes to ensure compatibility and proper functioning on macOS. Changes
Assessment against linked issues
Related issues
Poem
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 your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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 (5)
- v2/internal/xattr/xattr.go (1 hunks)
- v2/pkg/commands/bindings/bindings.go (2 hunks)
- v2/pkg/commands/bindings/bindings_darwin.go (1 hunks)
- v2/pkg/commands/build/build.go (2 hunks)
- v2/pkg/commands/build/build_darwin.go (1 hunks)
Additional comments: 6
v2/pkg/commands/build/build_darwin.go (1)
- 1-9: The changes in
build_darwin.go
correctly implement platform-specific initialization for macOS, ensuring that extended attributes are handled appropriately on this platform. The use of build tags and theinit
function are in line with Go best practices.v2/pkg/commands/bindings/bindings_darwin.go (1)
- 1-9: The changes in
bindings_darwin.go
correctly set up platform-specific initialization for macOS in the bindings package, ensuring consistent handling of extended attributes across the project. This approach is consistent with Go best practices.v2/pkg/commands/bindings/bindings.go (2)
- 16-17: The introduction of the
fixupXattrs
global variable is aligned with the PR's objectives and follows best practices for conditional platform-specific logic.- 63-65: The conditional use of
fixupXattrs
in theGenerateBindings
function correctly implements the necessary logic for handling extended attributes on macOS, ensuring it only affects builds for this platform.v2/pkg/commands/build/build.go (2)
- 25-26: The introduction of the
fixupXattrs
global variable aligns with the PR's objectives to handle extended attributes on macOS and follows best practices for conditional platform-specific logic.- 332-336: The conditional use of
fixupXattrs
in theexecBuildApplication
function correctly implements the necessary logic for handling extended attributes on macOS, ensuring it only affects builds for this platform.
v2/internal/xattr/xattr.go
Outdated
package xattr | ||
|
||
/* | ||
#cgo CFLAGS: -x objective-c | ||
#cgo LDFLAGS: -framework Foundation | ||
#import <sys/xattr.h> | ||
#include <stdlib.h> | ||
#include <string.h> | ||
|
||
void removeXattrFrom(const char *path) { | ||
ssize_t xattrNamesSize = listxattr(path, NULL, 0, XATTR_NOFOLLOW); | ||
if (xattrNamesSize <= 0) return; | ||
|
||
char *xattrNames = (char *)malloc(xattrNamesSize); | ||
xattrNamesSize = listxattr(path, xattrNames, xattrNamesSize, XATTR_NOFOLLOW); | ||
|
||
ssize_t pos = 0; | ||
while (pos < xattrNamesSize) { | ||
char *name = xattrNames + pos; | ||
removexattr(path, name, XATTR_NOFOLLOW); | ||
pos += strlen(name) + 1; | ||
} | ||
|
||
free(xattrNames); | ||
} | ||
*/ | ||
import "C" | ||
import "unsafe" | ||
|
||
func RemoveXAttr(filepath string) { | ||
cpath := C.CString(filepath) | ||
defer C.free(unsafe.Pointer(cpath)) | ||
C.removeXattrFrom(cpath) | ||
} |
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.
The implementation in xattr.go
correctly uses CGO and Objective-C to remove extended attributes from files on macOS. The integration and memory management are handled appropriately.
Consider adding error handling to the removeXattrFrom
Objective-C function and the RemoveXAttr
Go function to report failures in removing extended attributes, enhancing the robustness of this feature.
Deploying wails with Cloudflare Pages
|
is there a guide on how to build and run this PR locally? |
You can follow this guide and make sure you're using v3-alpha instead of master. Let us know how you get on 👍 |
can't figure out how to build or run any of this. why would I need to install v3 if I've pulled from the PR and This was the problem I had in my app, once I upgraded (using totally stuck here |
Sorry, ignore the v3 comment. Just follow the guide. If you want to revert to a previous version, just install using Go's
|
This reverts commit 880a4e0.
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 (2)
- v2/pkg/commands/bindings/bindings.go (1 hunks)
- v2/pkg/commands/build/build.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- v2/pkg/commands/bindings/bindings.go
- v2/pkg/commands/build/build.go
Potentially Fixes: #3326
Summary by CodeRabbit