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

The outputFile was only being set for darwin universal builds. I move… #2358

Conversation

gwynforthewyn
Copy link
Contributor

@gwynforthewyn gwynforthewyn commented Feb 8, 2023

…d the

variable assignment to a location where it will take place for all builds.

The ProjectData entries are used in generating the .app plist file, so setting them here ensures that the app will run with the custom binary name.

===

#2290

I wrote a bash script that runs the test cases. It's available at https://gist.github.com/gwynforthewyn/f045187d15f8be1a25d6be52f50e7497 - I didn't know whether there was a good location in the wails source repo for this file, so it's in a gist right now. Simply copy the script and put it anywhere on your file system to run it; it manages its own state internally and only relies on wails being in PATH, and maybe a macOS userland too.

Test cases in the test script

The test case the script covers is running a wails build with the default build options, and then run wails build -o ROFLCOPTER with the and see whether the layout on the file system is the same or different. This script generates a shasum from the file system layout to validate the difference. As the macos builds need to support both universal and single architecture builds as explicit test cases, as they use different logical paths internally in the build subcommand but both should respect the -o flag, there is 1 test cases but on 2 architectures, for a total of 2 test cases.

To achieve these two test cases, the script initialises a wails project in a temporary directory, then runs in succession the following commands, capturing the state of the build/ directory after each build command:

wails build -clean -platform "darwin/amd64" . > /dev/null" #build defaults
wails build -clean -platform "darwin/amd64" -o ROFLCOPTER . > /dev/null" #override output file name

Then it does the same file system layout check for the architecture "darwin/universal".

Finally, it cleans the temporary test file system.

Usage

To use it:

  • download the script and run it with the latest install of wails from HEAD using ./tester.sh
  • download my branch, build it, then run PATH=/path/to/your/build/wails ./tester.sh

The script has a built in help message you can activate with --help

Here's the output using the current version of wails 2.3.1:

;  ./tester.sh                                                         
Path to wails /Users/gwyn/go/bin/wails
Testing individual architecture was overridden successfully...
not ok - onearch_layout.txt and override_layout.txt are equal, overrides failed
Testing universal architecture was overridden successfully...
not ok - universal_layout.txt and override_universal_layout.txt are equal, overrides failed

Here's the output with the build from my branch:
```bash
;  PATH=~/Developer/wails/v2/cmd/wails/ ./tester.sh               
Path to wails /Users/gwyn/Developer/wails/v2/cmd/wails//wails
Testing individual architecture was overridden successfully...
ok - onearch_layout.txt and override_layout.txt are not equal, override succeeded
Testing universal architecture was overridden successfully...
ok - universal_layout.txt and override_universal_layout.txt are not equal, override succeeded

If you want to visually inspect a record of the data on the temporary file system, you can prevent the script from cleaning up the temporary file system it uses. This is detailed in the help message, but worth mentioning:

 CLEANUP=false ./tester.sh 
Path to wails /Users/gwyn/go/bin/wails
Testing individual architecture was overridden successfully...
not ok - onearch_layout.txt and override_layout.txt are equal, overrides failed
Testing universal architecture was overridden successfully...
not ok - universal_layout.txt and override_universal_layout.txt are equal, overrides failed
Top dir is /var/folders/s6/f8cljwsd5dj5m1sycmc2b_xm0000gn/T/tmp.s2B97o3b
``

Unfortunately, I couldn't see how to use unit tests for verifying this fix cf. https://github.com/wailsapp/wails/issues/2290#issuecomment-1414251083

This script provides a quick sanity check, which I'd been combining with manual inspection:

### Manual test cases
- the .app launches (`open path/to/app.app` in bash)
- verify the override file string is used

### All tests
[/] the amd64 build respects the override flag correctly
[/] the universal build respects the override flag correctly
[/] the amd64 build launches
[/] the universal build launches

@gwynforthewyn gwynforthewyn marked this pull request as ready for review February 20, 2023 01:34
@leaanthony
Copy link
Member

Thanks for this @gwynforthewyn! @Snider - have you been able to test this? Thanks.

@gwynforthewyn
Copy link
Contributor Author

I'm very happy to add suggested test cases to the testing script, or to rewrite in a more useful format.

@leaanthony
Copy link
Member

Appreciate that @gwynforthewyn - this is a small change so I'm ok merging. Edge cases will appear quickly if there are any 👍 Thanks for taking the time to do this 🙏

@gwynforthewyn
Copy link
Contributor Author

@leaanthony that's great news!! For what it's worth, checking the wails master commits (https://github.com/wailsapp/wails/commits/master) it looks like this hasn't been merged just yet.

@leaanthony
Copy link
Member

Any chance you could update the CHANGELOG in the website directory?

…d the

variable assignment to a location where it will take place for all builds.

The ProjectData entries are used in generating the .app plist file, so
setting them here ensures that the app will run with the custom binary name.
@gwynforthewyn gwynforthewyn force-pushed the bugfix/2290_MacOS_wails_build_-o_outputName_ignored branch from 7dcde48 to 6325a44 Compare February 24, 2023 14:00
@gwynforthewyn
Copy link
Contributor Author

Any chance you could update the CHANGELOG in the website directory?

@leaanthony done! See what you think

@leaanthony leaanthony merged commit 017ce1e into wailsapp:master Feb 24, 2023
@leaanthony
Copy link
Member

Awesome! Thank you so much 🙏

@leaanthony
Copy link
Member

@gwynforthewyn - looks like the change affects removal of the windows syso file which seems odd! https://github.com/wailsapp/wails/actions/runs/4266942411/jobs/7428093337#step:6:99

@airtonix
Copy link

Does this fix the problem that I'm having here:

# 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 👈🏻 what I want

I want build/bin/discordimagebrowser-darwin-universal

  • Compiling application: Building AMD64 Target: /Users/runner/work/discord-image-browser/discord-image-browser/build/bin/discordimagebrowser-darwin-universal-amd64
 INFO  Build command: go build -tags desktop,wv2runtime.download,production -ldflags "-w -s" -o /Users/runner/work/discord-image-browser/discord-image-browser/build/bin/discordimagebrowser-darwin-universal-amd64

...


 INFO  Build command: go build -tags desktop,wv2runtime.download,production -ldflags "-w -s" -o /Users/runner/work/discord-image-browser/discord-image-browser/build/bin/discordimagebrowser-darwin-universal-arm64

k, universal means two binaries. amd64 and arm64.

  • Packaging application: Done.
Built '/Users/runner/work/discord-image-browser/discord-image-browser/build/bin/discord-image-browser.app/Contents/MacOS/discord-image-browser' in 3m56.152s.

8931a1cea7d324acfb490ee24a0183a5

Where did build/bin/discord-image-browser.app come from ???

i asked for build/bin/discordimagebrowser-darwin-universal

  • if this needs to be a folder, then ok.
  • if this needs to have .app on the end then let another tool do that

Seems like there's some boundary here that needs to be walked back or redefined:

  • compiling.
  • packaging.

If we look over in the https://github.com/dAppServer/wails-build-action we see that there are infact many more steps required to properly package a windows or macosx app.

So either:

  • do all those steps in wails (lol)
  • respect my outputfilename and don't add .app to the folder/file created.

I actually had to make my own version of https://github.com/dAppServer/wails-build-action:

Comment on lines +267 to +268
options.ProjectData.Name = outputFile
options.ProjectData.Info.ProductName = outputFile
Copy link

Choose a reason for hiding this comment

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

Why are these two being set here? I'd want my structure to be /Applications/{{.Name}}.app/Contents/MacOS/{{.Outputfilename}}. This is because I want the app name to be capitalized and the executable name to be lower case. So /Applications/MyApp.app/Contents/MacOS/myapp.

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.

None yet

4 participants