Skip to content
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

wx: Add WX_MACOS_NON_GUI_APP #6113

Merged
merged 1 commit into from
Sep 29, 2022
Merged

Conversation

wojtekmach
Copy link
Contributor

We cannot depend on is_packaged_app() check because it is not a perfect way of checking if app is bundled.

It would return true when the app is bundled with a launcher like this:

import AppKit

let task = Process()
task.launchPath = "/Users/wojtek/src/otp/bin/erl"
task.arguments = ["-noshell", "-eval", "wx:new(), timer:sleep(5000), halt()"]
try task.run()
task.waitUntilExit()

It would return false for this launcher, however:

import AppKit

class AppDelegate: NSObject, NSApplicationDelegate {
    func applicationDidFinishLaunching(_ aNotification: Notification) {
        let task = Process()
        task.launchPath = "/Users/wojtek/src/otp/bin/erl"
        task.arguments = ["-noshell", "-eval", "wx:new(), timer:sleep(5000), halt()"]
        try! task.run()
    }
}

let app = NSApplication.shared
let delegate = AppDelegate()
app.delegate = delegate
app.run()

Btw, there has recently been a fix in wx [1] and we'd only be able to take advantage of it when OSXIsGUIApplication returns true (and when wx recognizes our app as properly bundled, see [2]). And so I believe to support variety of scenarios, we need precise control.

[1] wxWidgets/wxWidgets#22508
[2] #6070 (comment)

@github-actions
Copy link
Contributor

github-actions bot commented Jun 28, 2022

CT Test Results

    2 files    17 suites   3m 47s ⏱️
152 tests 148 ✔️ 4 💤 0
167 runs  163 ✔️ 4 💤 0

Results for commit 2df4e29.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@garazdawi garazdawi added the team:PS Assigned to OTP team PS label Jul 4, 2022
We cannot depend on `is_packaged_app()` check because it is not a
perfect way of checking if app is bundled.

It would return true when the app is bundled with a launcher like this:

```swift
import AppKit

let task = Process()
task.launchPath = "/Users/wojtek/src/otp/bin/erl"
task.arguments = ["-noshell", "-eval", "wx:new(), timer:sleep(5000), halt()"]
try task.run()
task.waitUntilExit()
```

It would return false for this launcher, however:

```swift
import AppKit

class AppDelegate: NSObject, NSApplicationDelegate {
    func applicationDidFinishLaunching(_ aNotification: Notification) {
        let task = Process()
        task.launchPath = "/Users/wojtek/src/otp/bin/erl"
        task.arguments = ["-noshell", "-eval", "wx:new(), timer:sleep(5000), halt()"]
        try! task.run()
    }
}

let app = NSApplication.shared
let delegate = AppDelegate()
app.delegate = delegate
app.run()
```

Btw, there has recently been a fix in wx [1] and we'd only be
able to take advantage of it when `OSXIsGUIApplication` returns true
(and when wx recognizes our app as properly bundled, see [2]).
And so I believe to support variety of scenarios, we need precise
control.

[1] https://github.com/wxWidgets/wxWidgets #22508
[2] https://github.com/erlang/otp erlang#6070
@wojtekmach
Copy link
Contributor Author

This is PR is really only useful for the 2nd approach I mentioned in the PR description, the AppDelegate one, but there are some caveats with that which I have learned since so let's hold off on this change for a moment.

@wojtekmach wojtekmach closed this Sep 1, 2022
@wojtekmach wojtekmach reopened this Sep 2, 2022
@wojtekmach
Copy link
Contributor Author

Yeah, as noted in the PR description there is a variety of scenarios for using wx so I believe more precise control is really useful here. We'd appreciate if we could land this PR so we (Livebook Desktop) could stop using our fork. :)

@dgud
Copy link
Contributor

dgud commented Sep 28, 2022

Can this be done without an env variable, i.e. in some cleaner way?

Or is this such a special case that I should just accept it as it is?

@wojtekmach
Copy link
Contributor Author

I unfortunately couldn't think of a better way of solving this issue, I think we need to override OSXIsGUIApplication but at the same time control what exactly it returns (so we can opt-out of certain behaviour in wx).

Would controlling this with application:put_env instead be an improvement?

@IngelaAndin IngelaAndin added the testing currently being tested, tag is used by OTP internal CI label Sep 28, 2022
@wojtekmach
Copy link
Contributor Author

@dgud fyi, for Livebook we are also exploring an altogether different way of building Mac apps with OTP described in #6070 (comment) and for that we wouldn't need this patch. I still think the patch is useful though as there are so many different ways of running OTP and wx and so I think it is very helpful to have some flexibility.

@dgud dgud merged commit ba72505 into erlang:maint Sep 29, 2022
@wojtekmach wojtekmach deleted the wm-WX_MACOS_NON_GUI_APP branch September 29, 2022 14:54
@wojtekmach wojtekmach restored the wm-WX_MACOS_NON_GUI_APP branch October 21, 2022 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants