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

Init GTK in NewFrontend, not init #2841

Merged
merged 3 commits into from
Aug 25, 2023
Merged

Conversation

phildrip
Copy link
Contributor

@phildrip phildrip commented Aug 23, 2023

Description

So that apps that have a headless / non-gui mode will be able to run, since they needn't call NewFrontend (which is called by CreateApp). Previously, init() would call C.gtk_init regardless of whether CreateApp was called.

Also, we call C.gtk_init_check and check the return value (and panic on failure), instead of C.gtk_init, since gtk_init just exits the process if it fails, without a sensible error message.

Fixes #2628.

Inspired by #2629, but slightly cleaner imho.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Unset DISPLAY env var
Run our app (which has a --headless flag) in headless mode, and it runs ok. Before the change, it would exit.
Run the app without the --headless flag, and see a stacktrace:

panic: failed to init GTK

goroutine 1 [running, locked to thread]:
github.com/wailsapp/wails/v2/internal/frontend/desktop/linux.NewFrontend.func1()
        /home/phil/go/pkg/mod/github.com/phildrip/wails/v2@v2.0.0-20230823104117-04b4634f4a50/internal/frontend/desktop/linux/frontend.go:143 +0x139
sync.(*Once).doSlow(0xc0001a8a20?, 0xc37e20?)
        /usr/local/go/src/sync/once.go:74 +0xbf
sync.(*Once).Do(...)
        /usr/local/go/src/sync/once.go:65
github.com/wailsapp/wails/v2/internal/frontend/desktop/linux.NewFrontend({0x159bc40?, 0xc0002d8b70}, 0xc00029d400, 0xc0000134e8, 0xc0001a8a20, {0x1599160?, 0xc0002c8380})
        /home/phil/go/pkg/mod/github.com/phildrip/wails/v2@v2.0.0-20230823104117-04b4634f4a50/internal/frontend/desktop/linux/frontend.go:134 +0x85
github.com/wailsapp/wails/v2/internal/frontend/desktop.NewFrontend(...)
        /home/phil/go/pkg/mod/github.com/phildrip/wails/v2@v2.0.0-20230823104117-04b4634f4a50/internal/frontend/desktop/desktop_linux.go:16
github.com/wailsapp/wails/v2/internal/app.CreateApp(0xc00029d400)
        /home/phil/go/pkg/mod/github.com/phildrip/wails/v2@v2.0.0-20230823104117-04b4634f4a50/internal/app/app_production.go:86 +0x589
github.com/wailsapp/wails/v2/pkg/application.(*Application).Run(0xc00026af20)
        /home/phil/go/pkg/mod/github.com/phildrip/wails/v2@v2.0.0-20230823104117-04b4634f4a50/pkg/application/application.go:57 +0x25
github.com/wailsapp/wails/v2.Run(0xc00029d400?)
        /home/phil/go/pkg/mod/github.com/phildrip/wails/v2@v2.0.0-20230823104117-04b4634f4a50/wails.go:14 +0x9a

After setting the DISPLAY env var, the app works as normal.

  • Windows
  • macOS
  • Linux

Test Configuration

output from wails doctor:

          Wails Doctor          
                                

                                                                                                                                                                                                                                                                                                                                                           
# Wails
Version         | v2.5.1                                  
Revision        | c2b36de510ce04749343fbe08396438d5244ab48
Modified        | true                                    
Package Manager | apt                                     

# System
┌─────────────────────────┐
| OS           | Ubuntu   |
| Version      | 22.04    |
| ID           | ubuntu   |
| Go Version   | go1.21.0 |
| Platform     | linux    |
| Architecture | amd64    |
└─────────────────────────┘

# Dependencies
┌──────────────────────────────────────────────────────────────────────────┐
| Dependency | Package Name          | Status    | Version                 |
| *docker    | docker.io             | Installed | 20.10.21                |
| gcc        | build-essential       | Installed | 12.9ubuntu3             |
| libgtk-3   | libgtk-3-dev          | Installed | 3.24.33-1ubuntu2        |
| libwebkit  | libwebkit2gtk-4.0-dev | Installed | 2.40.5-0ubuntu0.22.04.1 |
| npm        | npm                   | Installed | 8.19.4                  |
| *nsis      | nsis                  | Available | 3.08-2                  |
| pkg-config | pkg-config            | Installed | 0.29.2-1ubuntu3         |
└──────────────────────── * - Optional Dependency ─────────────────────────┘

# Diagnosis
Optional package(s) installation details: 
  - nsis: sudo apt install nsis

 SUCCESS  Your system is ready for Wails development!

Checklist:

  • I have updated website/src/pages/changelog.mdx with details of this PR
  • My code follows the general coding style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

So apps that have a headless / non-gui mode will be able to run, since
they needn't call `NewFrontend` (which is called by `CreateApp`).
Previously, `init` would call `C.gtk_init` regardless of whether
CreateApp was called.

Also change to call `C.gtk_init_check` with a panic, instead of
`C.gtk_init`, since `gtk_init` just exits the process if it fails,
without a sensible error message.

Fixes wailsapp#2628.
@lyimmi
Copy link
Contributor

lyimmi commented Aug 24, 2023

Could the g_set_prgname call moved before the init?
It would work better if it's set before the init.

@phildrip
Copy link
Contributor Author

I expect so, but I don't understand how that would affect things. How would it work better? It seems unrelated to the bugfix...

@lyimmi
Copy link
Contributor

lyimmi commented Aug 25, 2023

I'm sorry the prg_name would be working better if it's set before gtk_init.

It's not related to the bug at all. Just thought if the gtk_init is moved we could move set_prgname to a better place in one PR. That's all.

Edit. I'll make another PR for it when it's merged.

Copy link
Member

@leaanthony leaanthony left a comment

Choose a reason for hiding this comment

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

Thanks 🙏

@leaanthony leaanthony merged commit 427fe7e into wailsapp:master Aug 25, 2023
8 checks passed
@danjenkins
Copy link

Any idea when this may get released in a release?

@leaanthony
Copy link
Member

leaanthony commented Aug 30, 2023

Looking to do a release end of the week. Thanks for your patience. In the meantime you can always use the master branch so there shouldn't be any reason for you to be blocked. That'd also mean you can test to see if everything works as expected.

@danjenkins
Copy link

Looking to do a release end of the week. Thanks for your patience. In the meantime you can always use the master branch so there shouldn't be any reason for you to be blocked. That'd also mean you can test to see if everything works as expected.

Absolutely - I'm planning on testing it tomorrow 👍 Just wanted to know how long to expect to run off master ;) :) thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linux Wails app exits if $DISPLAY not set or valid instead of returning error.
4 participants