imxieyi / waifu2x-mac

Waifu2x-ios port to macOS, still in Core ML and Metal
MIT License
448 stars 47 forks source link

Embedding CLI into Mac App bundle #6

Closed chargeflux closed 5 years ago

chargeflux commented 5 years ago

Changed the project.pbxproj file such that when the Mac app is compiled, the CLI version is also compiled and copied into the Mac App bundle into waifu2x-mac-app.app/Contents/MacOS.

CommandLineKit.framework is also embedded into the Mac app when compiling the Mac app. This also means that the user will always have to run swift package update and rake xcodeproj in the Dependencies folder. Otherwise it will fail to build since CommandLineKit is missing.

Finally I turned on Skip install for the CLI version since it wouldn't work if it is just copied into /usr/local/bin. The user can setup an alias on their own if they would like or we could add a script that specifies the creation of alias during build/install. That being said, I could be misunderstanding how Skip install works and I just know it is an option that affects Archiving.

Anyhow, this PR should allow the CLI to work 100% since it references the Frameworks in ../Frameworks and the Mac GUI App itself works as-is.

imxieyi commented 5 years ago

I think it makes sense to me since it adds little to the app bundle size. It would be great if you could add some instructions in README.md especially about how to create alias or wrapper script to the executable. BTW, I don't think it is a good idea to automatically do this during build time.

chargeflux commented 5 years ago

OK I can add some instructions in the README.md about how to make the alias for the CLI version. The alias won't be done automatically during build time.

chargeflux commented 5 years ago

I added Compilation Instructions to make sure the user knows to compile the CLI dependencies first and have instructions to create a symbolic link for the CLI if they want. I also updated the Warning for the CLI version.

imxieyi commented 5 years ago

Thanks for your help! I will merge this.