CI: Improve workflows [WIP]

[HaHeho]

I ran into a compilation issue locally, and figured that it was not covered in the CI workflow. So far, I have done a couple of things that are outside of the scope of my initial issue. These things should be discussed if you even want them, and potentially be split into separate PRs.

• Add to not fail fast (maybe should be removed again?)
• Split macos-latest configurations into discrete versions macos-10.15, macos-11, macos-12 and compile either of them with the two latest supported xcode versions (seems useful, but maybe too explicit?)
• Fix workflow issue with build "(macos-12, Xcode_13.4.1)"
• Update from actions/checkout@v2 to actions/checkout@v3
• Update from actions/upload-artifact@v2 to actions/upload-artifact@v3
• Test a spaced path in all workflow steps of main.yml (this is what currently still fails)
• Test a spaced path in all workflow steps of pd-externals.yml (this currently fails only for macos-latest)
• Fix workflow issues when using a spaced path
• Find a nicer way (or remove changes) to run workflows with a spaced path

[HaHeho]

The current issue caused by a path containing a space can be seen in the operation e.g. here https://github.com/HaHeho/ssr/actions/runs/3101424126/jobs/5022768368#step:15:122

checking for gml/vec.hpp... no
configure: error: GML library cannot be found.  Update your Git submodules with:

    git submodule update --init

This is the exact issue I've encountered when building locally, and it happens on the CI only after introducing the path with a space. So I'm fairly confident the my horrible hack to introduce the pathed space into the CI is functioning as intended (at least up to the point of the issue).

The issue happens for all tested configurations identically. It seems like it should be trivial to fix, but I didn't find a way yet. @mgeier you probably have an idea?

[HaHeho]

In case of the pd-externals workflow, the introduced path with a space causes a fail only for macos-latest, e.g. here https://github.com/HaHeho/ssr/actions/runs/3101661894/jobs/5023239240#step:4:705

./libtool: line 5955: cd: /Users/runner/work/ssr/ssr/test: No such file or directory
./libtool: line 1901: cd: src/.libs/libsndfile.lax/libgsm.a: No such file or directory
make[2]: *** [src/libsndfile.la] Error 1

[mgeier]

Thanks for working on this!

Supporting filenames with spaces is always a chore ...

It seems like it should be trivial to fix, but I didn't find a way yet. @mgeier you probably have an idea?

I wouldn't call it trivial. It took me a few hours.

I first tried a gazillion ways of escaping the absolute path, and nothing seemed to work. I was close to giving up when I tried using a relative path instead. This meant to have different handling in configure.ac and Makefile.am, but it looked promising. I then had to escape a few more things and then it seemed to work. The result is in #294.

I've only tested it on Linux, so there might still be things left to do for macOS.

And I didn't test the Pd externals.

In case of the pd-externals workflow, the introduced path with a space causes a fail only for macos-latest

These errors happen when building libsndfile, so I guess we'll have to fix their build system?

This doesn't look very promising: libsndfile/libsndfile#350

Alternatively, we could try using their CMake-based build system instead.

[mgeier]

In case of the pd-externals workflow, the introduced path with a space causes a fail only for macos-latest

It looks like this is actually a bug in libtool.
This line doesn't have quotes:

https://github.com/autotools-mirror/libtool/blob/6132006b5fb6b95f31c30a972fbb829f93e1878b/build-aux/ltmain.in#L3287

I've created a PR at autotools-mirror/libtool#4, I hope this somehow makes it into the actual repository.

[HaHeho]

I merged some of the changes that you implemented since investigating this issue. In my understanding, everything that was a potential improvement of the workflow in this PR is now already in the main repository or other PRs, right?

The only missing thing would be the build testing the with actual spaces in the path (the way it is currently implemented here is awful of course). But it is probably not reasonable to include and implement this in a neat way into main i.e., its good enough that we fixed the build challenges with spaces for now.

Hence, I believe I can close this draft @mgeier ?

[mgeier]

Thanks for the update!

Why is the error from #293 (comment) not happening anymore?
AFAIU, the libtool bug is not yet solved, right?

The images and versions I selected are kinda random, I wanted to reduce the number of jobs a bit.
If there are some versions missing, or if there are still too many, we can of course change that.

About the "fail fast" thing: I don't really have an opinion on that, but we can change it if you want.

We can still try to use paths with spaces. But I think it would be nicer to use a variable for that instead of repeating "test path with spaces" all over the place.
If you want to do that in this PR, that's fine, but you can also close this one and create a new one.

For the future, I would recommend not making so many different changes in one PR but multiple smaller PRs instead.

[mgeier]

I've looked a bit more into the libtool issue, and I found a little hint in your log (https://github.com/HaHeho/ssr/actions/runs/3101661894/jobs/5023239240#step:4:705), line 152:

configure: WARNING: Libtool does not cope well with whitespace in `pwd`

So this seems to be a known problem, I just don't understand why nobody tries to fix it ...

I thought that since my attempt in autotools-mirror/libtool#4 doesn't seem to be noticed, maybe we could try to bring this patch to Homebrew?
However, I tried filing an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose and creating an issue there is quite complicated.

Maybe we can provide a patch, but I don't know yet how that would work.

For future reference, here is the relevant (I think) file from Homebrew:
https://github.com/Homebrew/homebrew-core/blob/master/Formula/libtool.rb

[HaHeho]

Why is the error from #293 (comment) not happening anymore? AFAIU, the libtool bug is not yet solved, right?

I have no idea. It did not occur in the last workflow run. The run log from the above comment is not available anymore, therefore it is difficult for me to investigate what makes the difference here. The error occurred in "step:4:705", while the latest macos -> step:4 (Build dependencies) doesn't even have so many lines anymore.
libsndfile is now at version 1.1.0, with the GitHub release dating to Mar-27. Not sure if the older run above used a version before that, and they managed to improve some pathing issues?
Actually version 1.2.0 is now availabe on Github since Dec-25, which was not yet used in the latest run.

The images and versions I selected are kinda random, I wanted to reduce the number of jobs a bit. If there are some versions missing, or if there are still too many, we can of course change that.

Seems fair. Exhaustive testing won't be possible anyways. The existing configurations have some coverage and macos-10.15 workflow runners will be unsupported from January. macos-12 could be run with Xcode 14.2 (Xcode 14.0.1 (default)) and macos-11 with only the latest configuration Xcode 13.2.1 (default). macos-13 (just released on hardware) is not yet available in workflows.

About the "fail fast" thing: I don't really have an opinion on that, but we can change it if you want.

Its probably good to keep failing fast for sustainability reason. Not doing it was just helpful for debugging different issues on different configurations here.

We can still try to use paths with spaces. But I think it would be nicer to use a variable for that instead of repeating "test path with spaces" all over the place. If you want to do that in this PR, that's fine, but you can also close this one and create a new one.

For sure this should be implemented better than here. But since I'm the first one to encounter this issue, it does not seem to be very relevant (or Linux people rather know to dodge paths with spaces to begin with).

For the future, I would recommend not making so many different changes in one PR but multiple smaller PRs instead.

Agreed. This was a draft mostly for testing with the existing GitHub workflows.

[mgeier]

Thanks for the hint about CI compiler versions, see #321.

I'm not sure if you meant to include Xcode 14.0.1, or if you just wanted to mention that it's the default.
I've left it out for now, should I add it?

(or Linux people rather know to dodge paths with spaces to begin with)

Yes, indeed. I've (painfully) learned to avoid spaces many years ago.

BTW, in the meantime I've found out that even if configuring and compiling (mostly) works with a path with spaces, there are still problems when installing (with make install).
Normally, there are no spaces in the install prefix (e.g. /usr/local), but it can happen when installing into a temporary directory during packaging and when building the app bundle (./configure --enable-app-bundle && make && make dmg).

I'm currently not interested in supporting this use case, but if you want (or anyone else wants) to fix this, I'm of course open for PRs.

[HaHeho]

I'm not sure if you meant to include Xcode 14.0.1, or if you just wanted to mention that it's the default. I've left it out for now, should I add it?

No, that was just to recall what the current choices were and what the current GitHub default per OS is.

The current hard coded OS + compiler combinations will not be the most recent any more a some point. Only using latest with its default compiler is not really providing a broad coverage. I'm sure the current implementation is fine if looked over from time to time.

One Apple M1 runners are available, this would for sure be one to make sure it is included in the workflows. But there seems to be no announcement or ETA for it yet (actions/runner-images#2187). Jack and SSR should work fine on Apple silicon, as investigated in #254.