librespot-org
CI: Improve feature checking for binary
Simple adjustment to the ci that checks all tls features and each-feature with native-tls.
We previously had --each-feature executions, but if --include-features is provided, each-feature will only execute for the features provided by that flag. Because of that we only tested rustls-tls-native-roots and native-tls.
This should hopefully prevent compilation issues like:
- https://github.com/librespot-org/librespot/pull/1600
- https://github.com/librespot-org/librespot/pull/1630
So... it compiles now successfully, but windows takes ~4 times longer to finish the run (lin: 2-3min, win: 9-10min). Currently that will apply for each PR and push. Any thoughts about how to reduce it or if that would be fine as-is?
Maybe running the windows flow only on dev as all features can be tested on linux?
For me it’d be OK to just test each and all feature combinations on Linux and spare some cycles, unless we can think of more of a reason why we want to cover it on Windows just as defensive measure.
So, adjusted it for now to only execute windows on the default branch. But I would like to have the option to trigger a windows run for a PR on demand if we go this route.
Manuel triggering a workflow seems to not allow it for a PR or external branch.
Maybe one of you can come up with an idea. Here are the available options for triggering.
What I had thought about so far as: pull_request_review: submitted, or pull_request: labeled seem interesting. Probably labeling a PR and by that deciding which OS to additionally run is probably the best and most intuitive I think? Let me know your thoughts on the topic^^
Manuel triggering a workflow seems to not allow it for a PR or external branch.
I might misunderstand, but you can define a "base" workflow and then include it other workflows, which themselves can be manual or PR triggered. Does that help?
Hmm, I might have formulated it badly then. I mean triggering a workflow for an existing PR with additional options, like adding another os to the CI execution. So that you can still test the execution of windows on-demand. For example, this PR would be a good example to also test windows, due to the changes on the windows ci.
But I think using labels to add another os to the PR CI execution might be a good solution.
Yes, using labels sounds neat. Unless there's a obvious situation where you might add the label for book-keeping reasons but really not want the CI. But we don't really use labels otherwise so I don't think that's a real problem.
Hmm, I had it working temporarily, but seem to stopped working... you have to love ci
So, I just made a wrong assumption what the check for "is a PR" was. I also improved how the label is named, so that the label is CI: Windows and something upcoming for macos could be CI: MacOS.
So, I'm done here. Works as I want it to do. I prepared something for macOS already, but will put this in a follow-up PR. Anything that still needs to be addressed or any concerns in regards to the changes? Otherwise, I would merge it in the coming days :)