Build: Fix Mac app version #2421
Conversation
|
Test successful:
|
mac/deploy_mac.sh
Outdated
| brew install /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask/Formula/create-dmg@"${create_dmg_version}".rb | ||
| # Get Jamulus version | ||
| local app_version="$(grep -oP 'VERSION = \K\w[^\s\\]*' Jamulus.pro)" | ||
| local app_version="$(sed -nE 's/^VERSION *= *(.*)$/\1/p' Jamulus.pro)" |
There was a problem hiding this comment.
I think to be consistent with earlier versions and the rest of the script, this should use ${project_path}. Line 98 below doesn't actually check the directory we are running in, as ${project_path} is an absolute path, so the -f test always succeeds.
| local app_version="$(sed -nE 's/^VERSION *= *(.*)$/\1/p' Jamulus.pro)" | |
| local app_version="$(sed -nE 's/^VERSION *= *(.*)$/\1/p' ${project_path})" |
There was a problem hiding this comment.
I think to be consistent with earlier versions and the rest of the script, this should use ${project_path}
Thanks, changed. We should wait for another CI run though.
Line 98 below doesn't actually check the directory we are running in, as ${project_path} is an absolute path, so the -f test always succeeds.
Yeah, but I'd rather not touch this as part of this PR. I plan to submit my other autobuild changes really soon.
There was a problem hiding this comment.
I think to be consistent with earlier versions and the rest of the script, this should use ${project_path}
Thanks, changed. We should wait for another CI run though.
Yes, I'd like to test install the Mac artifact with the new create-dmg. Unfortunately, non-release runs don't make the artifacts available until the last job has finished, afaik.
Line 98 below doesn't actually check the directory we are running in, as ${project_path} is an absolute path, so the -f test always succeeds.
Yeah, but I'd rather not touch this as part of this PR. I plan to submit my other autobuild changes really soon.
Sure. My point was only to illustrate why it was important to use ${project_path} instead of a simple filename, as the script doesn't actually ensure we are in the correct directory.
The build logic used `grep -oP` which does not seem to be supported by macOS' `grep`. Therefore, keep using `sed` instead. Broken in: 3a63033 Co-authored-by: Tony Mountifield <tony@mountifield.org>
2735858 to
2e06d1d
Compare
softins
left a comment
There was a problem hiding this comment.
Successfully installed the dmg from this build, so happy to approve.
|
Probably a |
Short description of changes
The build logic used
grep -oPwhich does not seem to be supported by macOS'grep. Therefore, keep usingsedinstead.Broken in: 3a63033 (#2397)
Context: Fixes an issue?
Does this change need documentation? What needs to be documented and how?
No.
Status of this Pull Request
Ready.
What is missing until this pull request can be merged?
CI/Review.
Checklist
My code follows the style guide