search

t-oster / VisiCut

A userfriendly tool to prepare, save and send Jobs to Lasercutters
https://visicut.org
Other
235 stars 115 forks source link

Release with GitHub actions #710

Closed TheAssassin closed 8 months ago

TheAssassin commented 8 months ago

Fixes #670.

This PR introduces a GitHub actions workflow to create releases on GitHub with all the available binaries.

It is intended to replace the external VisiCut build server and make it easier for contributors to maintain their own builds without the need for external infrastructure and complicated setup procedures.

Please note that the second commit is a workaround for an NSIS bug. The generated license file contains non-ASCII characters (it is UTF-8 encoded). I cannot make NSIS accept the file as-is, it claims it violates codepage 0 (to my understanding, this is the currently active codepage; on Windows, it should be some UTF-8 compatible scheme; I have no idea what the default on Debian is, though).

A demo of the generated release can be found here: https://github.com/TheAssassin/VisiCut/releases/tag/continuous Tagged releases will generate a non-prerelease entry. The text can be customized, please see pyuploadtool's docs.

mgmax commented 8 months ago

Regarding the license UTF8 issue: I assume it worked correctly in the old build environment. What's different now? I would have expected it to work the same because it uses the same docker container as before.

TheAssassin commented 8 months ago

I have no idea. I spent a couple of hours on this already. The software stack seems to be exactly the same. There is no newer version of NSIS in the backports, for instance. Help would be appreciated.

We could install a newer or older version into the Docker container manually if we have to.

mgmax commented 8 months ago

Looks like the distribute job does not use the VisicutBuilder docker container and therefore uses a different NSIS version.

mgmax commented 8 months ago

The distribute step is currently (almost) duplicate because build.sh already builds the packages, except for AppImage. https://github.com/t-oster/VisicutBuilder/blob/master/build.sh#L66-L69

In the distribute step, a different build script is used (distribute-docker.sh) with a different container than VisicutBuilder. The distribute-docker.sh is missing some features of VisicutBuilder/build.sh, at least the windows-addons and macos-addons files. If you want to get rid of VisicutBuilder, that's fine for me, but then we should cover all features (or have a good reason when we drop features).

What I especially liked about VisicutBuilder was that you could locally with just one docker command build VisiCut for every platform (except AppImage), and the only dependency you needed was Docker.

TheAssassin commented 8 months ago

We can always change to the other image if needed. It used to work with plain Debian, though. I have no idea whether VisicutBuilder is maintained regularly and given the fact that once the external CD setup is obsolete there is no use for the image outside of this repository, that'll eliminate another dependency. Of course, one can just copy the Dockerfile from VisicutBuilder to this repopsitory, too.

The idea of the distribute step is to run the builds in parallel to a) speed things up and b) see on the actions dashboard which deployment failed (resp. whether it's a build problem if all of them failed).

distribute-docker.sh is just as easy to use as VisicutBuilder. Personally, I foud VisicutBuilder quite a bit more unintuitive because it "just runs" and "does things". The script has a help text and it also just runs a single build if needed. In local development, as long as the Dockerfile (or the base image) don't change, it's pretty much as fast as using distribute.sh directly.

I'm open to changes, please let me know what you'd like. In my opinion, the less external dependencies, the better.

TheAssassin commented 8 months ago

So I updated the branch, but unfortunately the image does not work out of the box. I'm undecided on whether it's worth improving/fixing the VisicutBuilder image, or whether it's just easier to copy the necessary portions of the VisicutBuilder Dockerfile...

t-oster commented 8 months ago

If it has the same functionality and usability in the end it's better to have everything in one repository IMHO

TheAssassin commented 8 months ago

Mind to elaborate a bit please? I need to understand your expectations and requirements. Do you plan on giving up on the external CI then?

t-oster commented 8 months ago

Well the build process should work from command line Linux with just docker installed and should be able to build all distributions. Then there would not be any need for an external visicutbuilder image. And if the GitHub distributions work, I would be fine with removing download.visicut.org if there are no objections

t-oster commented 8 months ago

However since @mgmax is mostly maintaining visicut at the moment and I am very inactive, I would say it's his decision

TheAssassin commented 8 months ago

I see. That's provided with distribute/distribute-docker.sh and the reason I added that script along distribute.sh. Things should "just work" and they should work locally, too (especially for debugging/testing). I have not run a special test, but I'd expect it to work in a random live CD VM with Docker installed.

I'll try to fix the build issue as proposed in https://github.com/t-oster/VisiCut/pull/710#issuecomment-1989228016 by copying stuff from VisicutBuilder.

TheAssassin commented 8 months ago

I just copied @t-oster's fix for the NSIS bug from VisicutBuilder and added the remaining package dependences (even though I'm not sure what they're exactly used for, for potrace especially I don't see any use at this point but it doesn't really hurt either).

@t-oster could you confirm please whether potrace and the macOS/Windows "addons" are still needed anywhere? They don't seem to play any part in the current build process and seem to be leftovers. I think we can just leave them out.

t-oster commented 8 months ago

well potrace is used for the bitmap-to-vector conversion when you use VisiCut with a camera and take a picture of what you want to cut/engrave, you can just select it and create a shape out of that image.

t-oster commented 8 months ago

see https://github.com/t-oster/VisiCut/blob/master/src/main/java/de/thomas_oster/visicut/vectorize/VectorizeDialog.java#L358

TheAssassin commented 8 months ago

So it's a runtime dependency. It's not shipped at the moment from the binaries built by distribute.sh (not sure if it ever had been). The Debian package suggests its installation at least. We'd have to add this. However, that's for another PR, and should be tracked in a separate issue.

That said, I'm not sure that feature (and the built-in vectorization of raster graphics VisiCut appears to provide) are really worth keeping in the software. Personally, I use (and prefer) to use Inkscape for editing input files. I'd like to propose removing those features to focus on the core functionality and will open an issue for that.

The build has passed, i.e., this PR is ready for merging! Feel free to squash-merge. Otherwise, please notify me and I'll clean up the history.

t-oster commented 8 months ago

yes, it's a runtime dependency and for debian we just declare it, but for osx and windows we bundle the binaries. It pretty much depends on your workflow, but if you use VisiCut with a camera attached to the lasercutter it is IMHO a good feature, because you don't have to switch software for some simple tasks and even children can draw their ideas onto paper, take a picture in visicut and cut it. Not sure how many people use it though.

TheAssassin commented 8 months ago

Since I don't have a camera attached (yet), I have never tested this, so I cannot really tell how well it works. I have access to a (proprietary, of course) Shaper Trace that works reasonably well. But as said, this is for another day. We should open an issue.

mgmax commented 8 months ago

My suggestion would be:

TheAssassin commented 8 months ago

Use a fixed release/hash of pyuploadtool and appimagecraft (I am no friend of automatically changing external dependencies)

Will do, if "latest stable release" isn't stable enough for you. But then I suggest you subscribe to both repositories to avoid missing updates.

TheAssassin commented 8 months ago

Friendly reminder, this is ready.

mgmax commented 8 months ago

Thank you for bringing this forward.

In my personal matter of taste, it could be developed further towards a trusted/reproducible build. In this PR we have introduced at least three more external URLs that are loaded without verifying a sufficiently strong checksum. Two in the GH Actions build and one indirectly in in https://github.com/TheAssassin/appimagecraft/blob/6b36fda05d50b356024bd3b12ca426ba73fe334b/appimagecraft.yml#L13 . For comparison, the existing distribute.sh script checks the hash of the downloaded JRE before extracting it. https://github.com/t-oster/VisiCut/blob/8797012c2feb29f909edd2f448dced487ed70a5a/distribute/distribute.sh#L111

I will open a separate issue for that.