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

Add MacOS bundle distribution #1276

Merged
merged 14 commits into from
Feb 4, 2023
Merged

Add MacOS bundle distribution #1276

merged 14 commits into from
Feb 4, 2023

Conversation

Ryex
Copy link
Contributor

@Ryex Ryex commented Nov 30, 2022

Summary:

This PR adds the means to build a MacOS bundle for distribution. The goal is to enable building a dmg and or a zip of the app bundle via a github workflow for releases.

For now the approach is to use a sand-boxed miniconda environment that in placed in the bundle.
At the time of writing this environment provides python 3.10.8 and enchant 2.3.3. Python deps are the latest from pypi.

This does NOT enable distribution via homebrew.

The Appbundle is currently not codesigned and thus will get flagged and being distributed by an unidentified developer. Users can still run the bundle by right clicking / command clicking on the app in Finder and selecting the open option. When presented with the "Unknown Developer" dialog if they select "Open" an exception will be added for the bundle and it will run correctly from then on.

Related Issue(s):

#481
#867

Reviewer's Checklist:

  • The header of all files contain a reference to the repository license
  • The overall test coverage is increased or remains the same as before
  • All tests are passing
  • All flake8 checks are passing and the style guide is followed
  • Documentation (as docstrings) is complete and understandable
  • Only files that have been actively changed are committed

@vkbo
Copy link
Owner

vkbo commented Dec 1, 2022

Great!

The specific issue ticket for this is #867

@Ryex
Copy link
Contributor Author

Ryex commented Dec 1, 2022

I've had some success building the app bundle. it appears to run (I still have to test it under a fresh install).

I'm still not sure if the distribution should be done via DMG or just a zip of the app folder. different opensource apps do one or other or both it seems.

@Ryex
Copy link
Contributor Author

Ryex commented Dec 1, 2022

Well then.

I have a working workflow
I forced it to build on this branch for testing purposes, that can be easily removed. but it does seem to build properly.

you can checkout the build artifacts here:
https://github.com/Ryex/novelWriter/actions/runs/3591268873

@vkbo
Copy link
Owner

vkbo commented Dec 1, 2022

Great! I'll have a look at it later. I will test it on my Mac VM as well.

The issue #867 doesn't have to be Homebrew if there is another method that works better. It was just the approach I assumed would work the best.

@Ryex
Copy link
Contributor Author

Ryex commented Dec 1, 2022

ok well, seems there is no way around the "unidentified developer" dialog without code-signing the artifact. This is a bit of a process and requires that you have an apple developer account.

Now I need to get this to build the documentation and sample files.

@Ryex
Copy link
Contributor Author

Ryex commented Dec 1, 2022

Ok, This is simi ready. it wold be great if we could find people willing to test it on physical hardware.

For some reason the dmg has broken with the addition of the entitlement's signing? not sure what is up there but it doesn't actually add much so it could be removed up until you can sign it with a developer identity.

In the mean time distributing as a zip works great. it seems to get automatically extracted on at least macOS 10.15 (Catalina) and above; I've tested on VMs up to macOS 13 (Ventura).
Because of the lack of code signing Running the bundle at this time requires right / command clicking the app in finder and selecting "open", Then telling it to open anyway when presented with the "unknown developer id" dialog. This is still better than needing to install homebrew, the python deps, enchant etc. yourself and running form the terminal imo.

The workflow installs QT for building the translations and a TexLive environment to build the manual, both of these steps are cached so they should run much faster after the first time but that first time could take up to 30 minuets after the runner picks up the job. Neither of these are carried over to the bundle so it remains small.

Latest artifacts are here
https://github.com/Ryex/novelWriter/actions/runs/3595612422

Do you want a release trigger workflow to automatically upload the artifacts to a release when you push a tag?

@vkbo
Copy link
Owner

vkbo commented Dec 1, 2022

Thanks for all of this.

As for the signing, I'll look into it. I have the same issue with the Windows installer. I haven't found an open source friendly way of solving this yet, I don't want to pay through the nose for a certificate. The donations aren't nearly enough to cover it.

I don't want the automatic releases. I will just do that manually. I assume I can just grab the artefacts and test them on my Mac VM. I think it's also Catalina actually.

@Ryex
Copy link
Contributor Author

Ryex commented Dec 1, 2022

Thanks for all of this.

No problem, I needed something to do this week.

As for the signing, I'll look into it. I have the same issue with the Windows installer. I haven't found an open source friendly way of solving this yet, I don't want to pay through the nose for a certificate. The donations aren't nearly enough to cover it.

If it helps, from what I can tell the actual apple cert is free. you just have to apply for it and wait to be approved.

On the other hand It's a some what complicated process and if you want to do the signing in the github action you will need to make use of secrets. I'm not a Mac user or dev (outside of opensource efforts to support the platform) myself so I won't be too much help there. reading the documentation leads me to believe it may be a bit of a rabbit hole.
The secondary step of getting the bundle notarized probably doesn't help.

I don't want the automatic releases. I will just do that manually. I assume I can just grab the artefacts and test them on my Mac VM. I think it's also Catalina actually.

Correct, the build jobs will show up under https://github.com/vkbo/novelWriter/actions as artifacts for that job. with the last push the jobs will only run on tags. The artifacts by default only stick around for 90 days so you will have to download them and add them to the release yourself. I removed the codesign call so the dmg should work again too.

Do note that while the conda env only installed the English hunspell dictionary the apps seems to pick up on the system installed dictionaries so it should be good to go on that front.

I'll mark this as non draft now.

@Ryex Ryex marked this pull request as ready for review December 1, 2022 23:07
@vkbo
Copy link
Owner

vkbo commented Dec 1, 2022

I just merged in main. There is a package change in Ubuntu 22.04 that breaks the test run.

Anyway, as for the Enchant library, yes it works on MacOS as they too use aspell and hunspell and those dictionary packages Enchant supports. On Windows, not so much. I plan to add a GUI tool to download the dictionaries into a folder where Enchant can find them. I've already cloned the LibreOffice dictionaries repo for this purpose: https://github.com/vkbo/dictionaries

No problem, I needed something to do this week.

I'm already juggling this and a Python SharePoint library for work that I wrote, did some massive updates too, and is in the process of documenting. My entire days consist of Python code!

@vkbo
Copy link
Owner

vkbo commented Dec 5, 2022

Looks like I won't be able to get to this until at least another week. I'm working quite a lot, and about to go on a trip. This is why I got the 2.0 release out as I otherwise wouldn't have had time for that either in a while.

But I haven't forgotten!

@arafel
Copy link

arafel commented Jan 4, 2023

Just a note - I have an M1 Mac, if that helps for testing purposes?

@Ryex
Copy link
Contributor Author

Ryex commented Jan 4, 2023

Just a note - I have an M1 Mac, if that helps for testing purposes?

could you test one or both of the artifacts from this action?
https://github.com/Ryex/novelWriter/actions/runs/3595612422

@vkbo
Copy link
Owner

vkbo commented Jan 4, 2023

I've been very sick for the past two weeks or so, so didn't get around to doing anything on novelWriter over the break. I need to do a 2.0.3 patch release too.

@arafel
Copy link

arafel commented Jan 5, 2023

Just a note - I have an M1 Mac, if that helps for testing purposes?

could you test one or both of the artifacts from this action? https://github.com/Ryex/novelWriter/actions/runs/3595612422

Sure! I haven't given them a thorough beating but both artifacts seem to be fine - the application starts up and seems to work, dialogues coming and going as they're supposed to, all that stuff. :-) Nice work!

Thanks - Paul

@Ryex
Copy link
Contributor Author

Ryex commented Jan 5, 2023

Just a note - I have an M1 Mac, if that helps for testing purposes?

could you test one or both of the artifacts from this action? https://github.com/Ryex/novelWriter/actions/runs/3595612422

Sure! I haven't given them a thorough beating but both artifacts seem to be fine - the application starts up and seems to work, dialogues coming and going as they're supposed to, all that stuff. :-) Nice work!

Thanks - Paul

Wonderfull! If they work on an M1 then I'm relatively confident they work universally. To be perfectly honest I was a bit nervous about that. ☺️

@vkbo
Copy link
Owner

vkbo commented Jan 8, 2023

Thanks a lot for doing this, and for testing it. It looks promising.

A couple of review points:

  • Could you add the macos folder as a subfolder under setup instead? Just like I've done with debian. I would like to keep the root folder of the project as uncluttered as possible. Especially since there may be a lot of these type of folders.
  • Would it be possible to add the .plist build code as a part of setup.py? I would like to build this file on the fly as part of the mac build process. You could then also integrate with the existing functionality to extract current version number from the source instead of having to manually update the script. The more that can go into setup.py, the better. Currently it holds all packaging code.
  • Does the generated .plist file need to be a part of the repo? I would think it could just be built on the fly, similar to how I do for Debian packages.
  • All source files should have a line break at the end. See the warning symbol indicating this on the "Files changed" page on the PR.

Also, another point. This PR is against the main branch, which means it will not be included until the 2.1 release. I think it would be better to make this against the patch branch so that it can be applied to 2.0.x releases. I don't see, from a semantic versioning point of view, why new package formats should not be part of a patch release. They fall in the same category as translations in my opinion, and should be updated/added when available.

Ryex added 9 commits January 8, 2023 19:27
Signed-off-by: Rachel Powers <[email protected]>
use python 3.10
use curl over wget (pre-installed)
ensure enchant is installed
cleanup better to limit app bundle size
fix Info.plist

Signed-off-by: Rachel Powers <[email protected]>
Signed-off-by: Rachel Powers <[email protected]>
commit a1f358b6f0a193be7d3d72fa4082fa09701ebe16
Author: Rachel Powers <[email protected]>
Date:   Thu Dec 1 11:48:38 2022 -0700

    fix: reneable the res of the build

    Signed-off-by: Rachel Powers <[email protected]>

commit 8ed02b0
Author: Rachel Powers <[email protected]>
Date:   Thu Dec 1 11:41:19 2022 -0700

    fix: install qt for tools

    Signed-off-by: Rachel Powers <[email protected]>

commit d68dc29
Author: Rachel Powers <[email protected]>
Date:   Thu Dec 1 11:32:07 2022 -0700

    fix: add tex-gyre to texlive install

    Signed-off-by: Rachel Powers <[email protected]>

commit ff654a2
Author: Rachel Powers <[email protected]>
Date:   Thu Dec 1 11:08:46 2022 -0700

    fix: add required latex pkgs

    Signed-off-by: Rachel Powers <[email protected]>

commit 1a4babc
Author: Rachel Powers <[email protected]>
Date:   Thu Dec 1 10:17:31 2022 -0700

    fix: install sphinx via pip3

    Signed-off-by: Rachel Powers <[email protected]>

commit 4b602b0
Author: Rachel Powers <[email protected]>
Date:   Thu Dec 1 10:05:46 2022 -0700

    fix: ensure sphinx is properly installed

    Signed-off-by: Rachel Powers <[email protected]>

commit 59ad26f
Author: Rachel Powers <[email protected]>
Date:   Thu Dec 1 09:59:40 2022 -0700

    feat: install scheme-basic texlive pkg
    run setup.py in proper dir

    Signed-off-by: Rachel Powers <[email protected]>

commit 16cd503
Author: Rachel Powers <[email protected]>
Date:   Thu Dec 1 09:54:42 2022 -0700

    feat: build manual, translations, and sample project zip

    Signed-off-by: Rachel Powers <[email protected]>

Signed-off-by: Rachel Powers <[email protected]>
@Ryex Ryex force-pushed the macos-appbundle branch from 0e14998 to d9f5029 Compare January 9, 2023 04:08
@Ryex Ryex changed the base branch from main to patch January 9, 2023 04:08
@Ryex Ryex force-pushed the macos-appbundle branch from d9f5029 to 1a9dd1f Compare January 9, 2023 04:12
@Ryex
Copy link
Contributor Author

Ryex commented Jan 9, 2023

ok, I've moved the plist generation into the setup.py
the macos build script really shouldn't be merged with it though. far too much external interaction necessary to make that clean (the activation and usage of the miniconda env effectively requires a bash shell).

also re-based the PR onto the patch branch. should I do the same with the flatpak PR?

@Ryex Ryex force-pushed the macos-appbundle branch from 1a9dd1f to 7fb03e5 Compare January 9, 2023 05:06
@Ryex Ryex force-pushed the macos-appbundle branch from 7fb03e5 to f912d55 Compare January 9, 2023 05:09
@vkbo
Copy link
Owner

vkbo commented Jan 9, 2023

ok, I've moved the plist generation into the setup.py the macos build script really shouldn't be merged with it though. far too much external interaction necessary to make that clean (the activation and usage of the miniconda env effectively requires a bash shell).

Fair enough. A few bash scripts are fine. Worst case, we can just add a command that calls it. I also call other build setups from within setup.py. I mostly just want a single point of initiating everything.

also re-based the PR onto the patch branch. should I do the same with the flatpak PR?

Great! Yeah, the flatpak one would have the same issue.

@Ryex
Copy link
Contributor Author

Ryex commented Jan 9, 2023

FYI I accidentally broke the artifact upload in the workflow because I dyslexia-ed some names, I'm fixing that right now.

Signed-off-by: Rachel Powers <[email protected]>
@vkbo
Copy link
Owner

vkbo commented Jan 30, 2023

An update: I had to rush ahead 2.0.4 due to a change in PyQt5 version 5.15.8 that broke novelWriter. I moved other issues to a 2.0.5 milestone.

The remaining issues with this PR is the use of third party unverified actions in the workflow. I need to look into how to handle that before merging. I'm reluctant to using them since we're building an artifact that users will install.

@Ryex
Copy link
Contributor Author

Ryex commented Jan 30, 2023

The remaining issues with this PR is the use of third party unverified actions in the workflow. I need to look into how to handle that before merging. I'm reluctant to using them since we're building an artifact that users will install.

Ah, I take it your referring to the jurplel/install-qt-action https://github.com/marketplace/actions/install-qt ?

It is, sadly, the "best" way to install Qt inside a github action. especially for non linux runners.

The action automates the usage of aqtinstall one of the only ways to install Qt without the usage of a UI.

aqtinstall obtains it's files from https://download.qt.io/ the official download site.

I'm familiar with several of the projects that utilize the action in their release workflow.
If you don't with to use it it can be worked around but the process would still likely use aqtinstall.
I've tried using homebrew to install qt before and it's somehow nowhere near as friendly.

Additionally, the QT environment is only ever used to generate the translation files.
The Qt environment in the bundle is installed by the miniconda environment.

@Ryex
Copy link
Contributor Author

Ryex commented Jan 31, 2023

Or if you were also talking about the https://github.com/marketplace/actions/setup-texlive-action

It automates the installation of texlive and it's env from the ctan servers
It is necessary to build the pdf documentation but other than the pdf never touches the bundle.

The only real alternative was to use brew install texlive which builds from source (which lakes a long time) and cannot be cached to speed up the workflow

while the icns *can* be generated on linux it is much preferable to generate it on macos
it is checked in as a convience

this commit fixes the appbundle not displaying the icon corrected
and corrects the app being displayed as "python" in the top bar when the bundle is run

Signed-off-by: Rachel Powers <[email protected]>
@vkbo
Copy link
Owner

vkbo commented Feb 1, 2023

I mostly meant the Qt install. The texlive one we can solve by simply adding the PDF to the repo and update it in the release commit itself before tagging.

I suppose we could also add the translation files in the same way if that avoids the whole issue. They are only about 100kB each, which is a lot less than the manual PDF.

@Ryex
Copy link
Contributor Author

Ryex commented Feb 1, 2023

Checking in the built files would be the best way to bypass the need to build them. However the setup.py code would need a way to bypass their build with a flag or something.

@vkbo
Copy link
Owner

vkbo commented Feb 2, 2023

I would just remove the automatic call in the relevant functions.

Anyway, I think I have a better idea on how to solve the problem. I don't, as a principle, like committing files that are built from source already in the repo. I'm gonna test out an approach we're using at work on GitLab. I expect GitHub has similar capabilities.

@vkbo
Copy link
Owner

vkbo commented Feb 4, 2023

I'm working on a few changes, but I'll just merge your PR first and then add mine as a new PR. Mostly because I'm probably going to generate a lot of commits to get actions working as I want, so I'm going to use squash merge in the end.

I'll tag you in the PR so you can add any insights :)

@vkbo vkbo merged commit 08bc293 into vkbo:patch Feb 4, 2023
@vkbo vkbo mentioned this pull request Feb 4, 2023
6 tasks
@vkbo vkbo added this to the Release 2.0.5 milestone Feb 10, 2023
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.

3 participants