-
Notifications
You must be signed in to change notification settings - Fork 23
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
#451: mac gatekeeper #453
base: main
Are you sure you want to change the base?
#451: mac gatekeeper #453
Conversation
Pull Request Test Coverage Report for Build 9818906813Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9818906469Details
💛 - Coveralls |
# Conflicts: # cli/src/main/java/com/devonfw/tools/ide/tool/LocalToolCommandlet.java # cli/src/main/java/com/devonfw/tools/ide/tool/ToolCommandlet.java # cli/src/test/java/com/devonfw/tools/ide/commandlet/EditionSetCommandletTest.java
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.
Looks good to me. I've added some small CRs and merged the latest main into this branch.
Please double check if the merge was ok.
} | ||
return new ToolInstallation(rootDir, linkDir, binDir, resolvedVersion, newInstallation); | ||
} | ||
|
||
private Path findMacApp(Path path) { |
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.
Couldn't this be moved to the MacOsHelper
class?
Path macApp = findMacApp(linkDir); | ||
if (macApp != null) { | ||
ProcessContext pc = this.context.newProcess(); | ||
pc.executable("sudo").addArgs("xattr", "-d", "com.apple.quarantine", macApp); | ||
pc.run(); |
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.
Maybe move to MacOsHelper
class?
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.
Major problem is that this approach does not yet work. Maybe we need to do this for all files recursively. Still looking for a better solution to the problem. After all MacOS Settings will do some magic and there should be some command to automate this "magic".
But I fully aggree that this should go to MacOsHelper
following SoC.
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.
There is no documentation from Apple about such features. Maybe this meta-attribute needs to be set on the executable(s) rather than on the *.all
folder. We would need more analysis on this to come up with a real solution.
I will change this PR back to draft to make this clear.
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.
We would actually need somebody to analyze this on a real mac with Gatekeeper and try various such xattr
commands for different folders and files until we know what does the magic trick.
Additionally someone must implement #415
Only after both things are addressed, we can actually implement/finish this #451 story.
Pull Request Test Coverage Report for Build 10115741272Details
💛 - Coveralls |
tries to fix #451