Sitelet https://github.com/libfive/libfive/pull/357
Skip to content

Adds GitHub-Actions for cross-platform building / testing / artifact-production - #357

Open
TyOverby wants to merge 104 commits into
libfive:masterfrom
TyOverby:github-actions
Open

TyOverby wants to merge 104 commits into
libfive:masterfrom
TyOverby:github-actions

Conversation

@TyOverby

@TyOverby TyOverby commented Jul 26, 2020 •

Copy link
Copy Markdown

If you aren't familiar with GitHub Actions, they're a lot like other continuous-integration solutions out there.

This pull-request implements actions for building + testing + producing artifacts for Linux, OSX and windows.

There are a few things to make note of:

  1. The tests don't pass. It looks like there's a test that fails (that isn't expected to fail), and because of that, I needed to mark the testing stage of the build as "continue-on-error"
  2. I don't actually know which files are useful to zip up in the artifacts package. For the osx build, I managed to get the whole "Studio.app" directory, which is pretty cool, but I don't have a mac to test it on. Likewise for linux, I only use WSL, so I can't make sure that the studio binary could actually be run after having been downloaded.
  3. Please ignore the number of commits that it took me to get this thing working...

@TyOverby

TyOverby commented Sep 11, 2020 •

Copy link
Copy Markdown
Author

Thanks, this seems like a good idea!

Thanks!

The deploy.sh is necessary to pack up everything in an actual working macOS app; otherwise, it's looking for libraries that may or not be installed on the target system.

Yep, this totally makes sense. Sadly I don't have a machine with OSX on it, so I don't think I'll be able to iterate on it here. I think it's probably best to merge this even though all of the artifacts aren't built properly so that at least we get cross-platform build validation.

I'm in the same boat as @omgitsraven when it comes to Linux distributions (in terms of not knowing much). It seems like folks prefer to have it built from source in a package manager, though Snaps are attempting to be a common package format (maybe?). I don't know if it's worth trying to deploy on Linux, vs just building + testing.

Agreed. Right now the artifacts are still zipped up (this might be useful to help debugging this workflow file), but I don't expect that they'll be particularly useful to linux end-users.

It would make the history cleaner if the commits were squashed (with a meaningful commit message), and it looks like this PR also accidentally adds a few binary files. You can use git filter-branch to remove those files from the commits, so they don't take up space in repo history.

When you click the down-arrow on the "merge pull-request" button, there's an option to squash commits, which uses the pull-request text as the commit message. This would be a lot easier for me (I'd rather not learn how to rewrite history in Git), so if it's cool with you, we could work on making my pull-request text a good commit message, and work from there.

@TyOverby

Copy link
Copy Markdown
Author

Oh, also, I managed to get windows actions building!

@TyOverby

Copy link
Copy Markdown
Author

ping @mkeeter

@mkeeter

mkeeter commented Sep 17, 2020 •

Copy link
Copy Markdown
Member

Hi – I'm relatively overloaded right now, so no promises when I'll look at this!

What's the purpose of changing the CMake version detection? On master, the new command (git describe --dirty="+" --tags --always) gives a relatively nonsensical name based on an old branch:

➜  libfive git:(master) git describe --dirty="+" --tags --always
rip-dmc-1685-ge1da2fb6

@TyOverby

Copy link
Copy Markdown
Author

@mkeeter no worries! I just wanted to make sure that you saw it.

What's the purpose of changing the CMake version detection

I couldn't get "bash -c" to run on windows without installing WSL, which doesn't appear to be possible on the machines that github uses to run this code. Shame about the issue though... Looks like the only use for the "bash -c" is so that you can use || echo '+', do you know if this possible to do in CMake without using bash?

@doug-moen

doug-moen commented Sep 18, 2020 via email

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants