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

MacOS: wails build -o outputName ignored. #2290

Open
Snider opened this issue Jan 10, 2023 · 17 comments
Open

MacOS: wails build -o outputName ignored. #2290

Snider opened this issue Jan 10, 2023 · 17 comments
Labels
Bug Something isn't working

Comments

@Snider
Copy link
Contributor

Snider commented Jan 10, 2023

Description

When building an application on MacOS the -o arg is ignored, and the configuration value in the code is used.

To Reproduce

macos:
    runs-on: macos-latest
    steps:
      # Checkout code
      - uses: actions/checkout@v3
        with:
          repository: letheanVPN/desktop
          submodules: recursive
      - uses: dAppServer/wails-build-action@v2
        with:
          build-name: letheanTest
          build-platform: darwin/universal
          wails-build-webview2: "embed"
          wails-version: "v2.3.1"
          go-version: ^1.18
          sign: "false"

Expected behaviour

The output name to be the same as provided with -o

Screenshots

image

Attempted Fixes

No response

System Details

https://github.com/dAppServer/wails-build-action/actions/runs/3885309312/jobs/6629024935

Additional context

linux and windows will use the -o value; not sure if you build from Linux for macOS if it builds fine or not.

https://github.com/dAppServer/wails-build-action/actions/runs/3885309312
Workflow file to test the GHA: https://github.com/dAppServer/wails-build-action/blob/11-action-fails-with-wails-231-on-macos-due-to-ditto-error/.github/workflows/lethean-desktop.yml

Workflow file to test using GHA: https://github.com/dAppServer/wails-build-action/blob/main/.github/workflows/lethean-desktop.yml

@gwynforthewyn
Copy link
Contributor

I've been taking a look at this, and I'd like to develop the fix. Here's what I've figured out so far.

Building a macOS app comes in two parts: compiling the go binary, and packaging the .app.

go binary

In v2/pkg/commands/build/build.go, in the execBuildApplication function, building the go binary has 2 logical branches for darwin; the if statement is on https://github.com/wailsapp/wails/blob/master/v2/pkg/commands/build/build.go#L266

If we are building a "universal" build (compiling for both arm64 and amd64 architectures), then on https://github.com/wailsapp/wails/blob/master/v2/pkg/commands/build/build.go#L311 the go compilation's OutputFile name is set to the value of the -o flag; if only one architecture is being built, this doesn't happen presently.

macos packaging the .app

Packaging the app for darwin takes place in https://github.com/wailsapp/wails/blob/master/v2/pkg/commands/build/packager.go#L64, and doesn't seem to check if there's a filename been set with the -o flag.

@gwynforthewyn
Copy link
Contributor

This isn't a huge check-in, but as I've been fixing this I keep getting tripped up by the difference between the Universal and the individual architecture builds, so I decided I need to add tests for the build.go file and validate I'm not introducing regressions, and well, here - gwynforthewyn@fcbefc9

It took me a few hours to figure that out : )

@Snider
Copy link
Contributor Author

Snider commented Jan 30, 2023

Thanks for looking into this @gwynforthewyn, it's a sneaky little bug

@gwynforthewyn
Copy link
Contributor

Just a note to say I’m still on this, but during the week I have work and find it hard go get the time to redirect back.

I did reach the conclusion that unit testing isn’t the best type of testing here: to run the build command you need a project layout to actually build. I think that in order to build that layout on the file system, I’d need to run main::initProject during test setup, but that can’t be called from a test inside the build package. I may just knock together a shell script to validate my file system layout as I’m tweaking things, nbd.

@Snider
Copy link
Contributor Author

Snider commented Feb 2, 2023

Just a note to say I’m still on this, but during the week I have work and find it hard go get the time to redirect back.

I did reach the conclusion that unit testing isn’t the best type of testing here: to run the build command you need a project layout to actually build. I think that in order to build that layout on the file system, I’d need to run main::initProject during test setup, but that can’t be called from a test inside the build package. I may just knock together a shell script to validate my file system layout as I’m tweaking things, nbd.

No stress, when you get time.

A shell script might be a good idea, not sure if this is helpful, I'm doing some build shenanigans that might help give some ideas to force a name mismatch: https://github.com/dAppServer/pwa-native-action/blob/main/action.yml

@gwynforthewyn
Copy link
Contributor

gwynforthewyn commented Feb 8, 2023

I have a draft PR for a fix up; I do need to finish the tests, but I wanted to ask some guidance.

The request here was for the output binary to be renamed. Renaming this means that there needs to be different information inside the Info.plist file - we cannot continue referencing the old binary name, as this isn't being generated. Then I figured that if I'm changing the name inside the Info.plist file, it makes sense that the .app name is changed too.

However, here's a plist file generated with this PR's changes. Should I change the CFBundleName, too?:

; cat build/bin/roflcopter.app/Contents/Info.plist 
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
    <dict>
        <key>CFBundlePackageType</key>
        <string>APPL</string>
        <key>CFBundleName</key>
        <string>2290_ignore_output_filename_macos</string> #should I change this?
        <key>CFBundleExecutable</key>
        <string>roflcopter</string>
        <key>CFBundleIdentifier</key>
        <string>com.wails.roflcopter</string>
        <key>CFBundleVersion</key>
        <string>1.0.0</string>
        <key>CFBundleGetInfoString</key>
        <string>Built using Wails (https://wails.io)</string>
        <key>CFBundleShortVersionString</key>
        <string>1.0.0</string>
        <key>CFBundleIconFile</key>
        <string>iconfile</string>
        <key>LSMinimumSystemVersion</key>
        <string>10.13.0</string>
        <key>NSHighResolutionCapable</key>
        <string>true</string>
        <key>NSHumanReadableCopyright</key>
        <string>Copyright.........</string>
    </dict>
</plist>

Additionally, the window that holds the wails app contains the default name in its title bar:

wails

I haven't figured out how to change that just yet, but I can try to get that updated too if you'd like?

(Also, the amount of knowledge needed to fix this is absolutely not represented by the tiny diff 😄 )

@Snider
Copy link
Contributor Author

Snider commented Feb 8, 2023

Heya @gwynforthewyn, Thanks for this.

I think the title comes from the CFBundleName setting, so updating the plist should fix the secondary issue; it's limited to 15 chars, but CFBundleDisplayName is not, so maybe adjust the display name and keep the BundleName the same as that prop is used for i18n and permissions.

I can see some edge cases where this is not ideal, but probably an accepted caveat of overriding the code-defined app name on the build depends on how @leaanthony feels about it, as there might be different side effects.

At least this brings MacOS build behaviour in line with Linux & Windows, even though some are non-ideal; once the build is constant across platforms, it's no longer a Wails issue and becomes the app developers.

https://developer.apple.com/documentation/bundleresources/information_property_list/cfbundlename

@gwynforthewyn
Copy link
Contributor

It's time to get into spooky behaviour.

I opened up Info.plist in my build app and added a CFBundleDisplayName key with a new value in it, but when I open the app with open /path/to/my/app.app the window title persists as a lingering problem. You can see in this screenshot that CFBundleName and CFBundleDisplayName look like they're used in the menu bar, not the window title. I'm a noob at this, though 🤷‍♀️

Screenshot 2023-02-09 at 6 59 37 AM

I can't actually see any strings that match my window title in the .app folder for this build. https://developer.apple.com/documentation/swiftui/configure-your-apps-navigation-titles seems to suggest I need to figure out where the navigationTitle is set.

I'll keep pecking away at this.

@gwynforthewyn
Copy link
Contributor

This looks promising https://github.com/wailsapp/wails/blob/master/v2/internal/frontend/desktop/darwin/window.go#L68

I'll need to figure out what data is being sent in there. This'll be a stretch of my skills, I think 😅

@Snider
Copy link
Contributor Author

Snider commented Feb 9, 2023

Snap! Looks like it is taken from the Tittle attr from the main.go file.

image

@gwynforthewyn
Copy link
Contributor

Yes! I just reached the same point! In that case, I'm not sure how to handle this just yet, as it'd involve overriding user-provided source code.

My thoughts are currently that I could leave this alone as the build/Run process is changing in v3, or I'm happy to try and figure out something like updating the init command to insert a check for whether an override file name was provided; that doesn't fix it for historical users, but would give anyone using v2 in future a nicer experience.

@Snider
Copy link
Contributor Author

Snider commented Feb 9, 2023

I don't think overriding the app name on the build is a good idea, but there might be cases where the app exe needs a more straightforward name.

The issue for Wails is that both Linux and Windows take the -o and MacOS doesn't, so making MacOS builds behave the same I think is perfect, tbh.

As you pointed out, anything more and your editing userland code which is not a Wails problem. :)

p.s if you add GitHub sponsors I'll make sure my project donates enough to buy a bottle of something for your trouble.

@gwynforthewyn
Copy link
Contributor

I think I'll have time to get the testing done on this over the weekend. There is a draft PR at #2358; the patch is surprisingly tiny.

p.s if you add GitHub sponsors I'll make sure my project donates enough to buy a bottle of something for your trouble.

Thank you, but I'm doing this for the love. I'm just happy to help out.

@Snider
Copy link
Contributor Author

Snider commented Feb 16, 2023

I think I'll have time to get the testing done on this over the weekend. There is a draft PR at #2358; the patch is surprisingly tiny.

p.s if you add GitHub sponsors I'll make sure my project donates enough to buy a bottle of something for your trouble.

Thank you, but I'm doing this for the love. I'm just happy to help out.

Thanks, well, we donated trees in your name instead. (it's about 100 trees in case you are wondering)

image

https://donate.trees.org/team/400781

Thanks for working on this :)

@gwynforthewyn
Copy link
Contributor

PR's open : ) #2358

@lpdswing
Copy link

lpdswing commented Apr 6, 2023

make sure matrix.build.name same with wails.json -> outputfilename

@airtonix
Copy link

airtonix commented Apr 29, 2023

I'm getting similar problem:

# Build Options

Platform(s)        | darwin/universal                                                       
Compiler           | /Users/runner/.asdf/installs/golang/1.18.3/go/bin/go                   
Skip Bindings      | false                                                                  
Build Mode         | production                                                             
Frontend Directory | /Users/runner/work/discord-image-browser/discord-image-browser/frontend
Obfuscated         | false                                                                  
Skip Frontend      | false                                                                  
Compress           | false                                                                  
Package            | true                                                                   
Clean Bin Dir      | false                                                                  
LDFlags            |                                                                        
Tags               | []                                                                     
Race Detector      | false                                                                  
Output File        | discordimagebrowser-darwin-universal 

so I'd expect that it creates a folder :

build/bin/discordimagebrowser-darwin-universal/

This is not at all what happens.

Instead it creates:

build/bin/discordimagebrowser.app/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants