MSzturc / ThinkpadAssistant

An Assistant Application that allows you to use all your Function Keys on a T-Series Thinkpad Laptop
122 stars 13 forks source link

Use DMG instead of ZIP and add main.yml [enhancement] #40

Closed EETagent closed 4 years ago

EETagent commented 4 years ago

Hello, I would suggest that we use DMG instead of ZIP. DMGs are considered a standard for app distribution outside of an App Store. Their file size is similar to ZIP and you get that fancy UI for app installation. I would also suggest adding a FOSS license to the project to fix problems that arise from not having any. Main.yml was added for pull requests testing. There is currently CODE_SIGNING_REQUIRED=NO, someone with a developer certificate must take care of this ( Some good examples 1, 2, App and DMG notarization ).

ThinkpadAssistant could be distributed as both. That would be waste of space in my opinion though.

Snímek obrazovky 2020-08-01 v 19 21 26

MSzturc commented 4 years ago

@EETagent I've merged your change in the build workflow and i've added my developer account for code signing. The .app got signed but the create dmg step was not able to sign the .dmg file. Is this a problem? I was able to install new released version (1.9.2.1) on my pc without any issues.

Log:

Create DMG19s shell: /bin/bash -e {0} Run XCBUILD_PATH="build/Build/Products/Release" XCBUILD_PATH="build/Build/Products/Release"

cp LICENSE $XCBUILD_PATH/license.txt

cd $XCBUILD_PATH create-dmg ThinkpadAssistant.app || true mv .dmg ThinkpadAssistant.dmg cd - mkdir Assets cp -R ${XCBUILD_PATH}/.dmg Assets shell: /bin/bash -e {0}

EETagent commented 4 years ago

Hi @MSzturc

Thanks for giving DMG a try. I do not think that unsigned DMG is a problem, but not doing it properly would be a waste of the paid certificate. It looks like create-dmg is unable to find signing identity automatically. Both .app and .dmg should be notarized too ( I am not Xcode dev, but from what I understand, it means sending files to Apple for official AV check ). We should reuse scripts and GitHub actions from https://github.com/bjesuiter/macos-file-summoner/tree/master/scripts, they are really professional and I don’t think there is better way to do it ( It would be better if all scripts would be merged into the .yml though )

Script for creating DMGs from said repo

#!/usr/bin/env bash

###   Dependencies for this script       
### - an Apple Developer ID Certificate available inside macOS Keychain
### - https://www.npmjs.com/package/create-dmg (can be globally installed with npm)

function log() {
    echo 
    echo "${1}"
}

# Input Vars
# This env var should be filled with the secrets.APPLE_DEVELOPER_ID_NAME on gihub actions
APPLE_DEVELOPER_ID_NAME=${APPLE_DEVELOPER_ID_NAME:-Developer ID Application: Benjamin Jesuiter (BB38WRH6VJ)}

# logs the script name
log "-- $(basename ${0}) --"

DIST_DIR=${DIST_DIR:-dist}
# The resulting DMG file will have the form of 'File Summoner x.y.z.dmg'
create-dmg --overwrite ${DIST_DIR}/*.app --identity="${APPLE_DEVELOPER_ID_NAME}"

Also it would be great to implement that GitHub Action will get commit messages since last release and post them as latest release description. Like HeliPort do, but with actual changes only. I will take a look into this

EETagent commented 4 years ago

That notarization procedure is completely mad ... Currently I have this, with false values of course. If you find notarization worth it, ( It is requirement for apps outside of an App Store ) we should test it in completely different repo, to not destroy this with unnecessary tags, releases and commits.

EDIT: Added release notes. GitHub Action reads commit messages since last release and post them as a description.

c116611 Fix Release notes
377b6a9 Add Release notes
05fe2e1 Formatting fixes
78d4ab2 ThinkpadAssistant notarization placeholder
MSzturc commented 4 years ago

Do you know what the consequences are when we not notarize the dmg? Do we get a msg that it's "unsafe" that we have to accept (like in windows) or worse?

The part with the release notes will safe me much time in the future 🥇 big thx

EETagent commented 4 years ago

Worse, one need to approve app using settings. Minor inconvenience for skilled macOS user though ( Hackintosh user must be one ) ThinkpadAssistant is sandboxed Swift application, I don't see a reason why some automatized Apple check would not approve it.

Snímek obrazovky 2020-08-03 v 21 31 48 Downloaded DMG from latest release

EETagent commented 4 years ago

@MSzturc I think that new issue should be opened. Closed pull request is not the best place where to discuss it.