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

Rework target matrix #132

Merged
merged 1 commit into from Jun 4, 2020
Merged

Rework target matrix #132

merged 1 commit into from Jun 4, 2020

Conversation

Chronophylos
Copy link
Contributor

@Chronophylos Chronophylos commented Jun 4, 2020

  • Added a job for each platform (Linux, Windows and MacOS)
  • Added constraint to only run on pushes to master
  • Added some targets from Tier 2 for Linux
  • Added iOS targets for MacOS
  • Added MSVC targets for Windows

There is a lot of duplicate text, but I couldn't find another way to do this the way I want.

Steps using native TLS are disabled on Linux since none of them ran.
Cross is disabled on Windows as well.

Currently not all targets are working: https://github.com/Chronophylos/twitchchat/actions/runs/124750481

closes #118

@museun
Copy link
Owner

museun commented Jun 4, 2020

This is looks great. I'll take some time to read through the failures to see if there is anything on my end I can do to make it compilible/testable on those platforms first. I also need to do a quick review of the feature/test set locally to determine what could go into the 'fast' path for when things are pushed to dev.

The reason I have it test the feature combination is that I've cut some releases that were broken when used in certain combinations -- so I came up with a permutation of all the possible feature sets to ensure things are missed.

Also, we may be able to drop some platforms if its just not going to compile -- or note that certain features won't work on them. The default-features = false configuration should work everywhere, though. This gives access to the encoder/decoder/parser and all of the different message / twitch types.

As an aside on making it feature-complete on more platforms: I'm planning on merging in a local branch soon-ish (tm) that'll add a sync EventStream/Runner/Client/etc so its fully usable with just std. (I still need to think about external vs internal threading -- should I spawn the threads or set it up so the user drives everything using blocking methods.) This should only require std and probably parking_lot and various crossbeam libraries.

@museun
Copy link
Owner

museun commented Jun 4, 2020

Also: @Emilgardis is the maintainer of cross. You can probably ask them about it in the future.

And.. I'm not sure if you're aware of: https://github.com/actions-rs . This is a community effort for better Rust support with Github Actions. I don't know how useful this would be to getting OpenSSL (and other problematic libraries) to be buildable on the test targets. I haven't really checked into alternatives to native-tls (well, for Linux).

The reason I don't default to rustls is that its not had a formal audit yet. If I can find something like a BoringSSL, or LibreTLS crate that works with tokio (or wouldn't take much effort to wrap with their traits) I could conceptually replace native-tls with those on Linux. native-tls, for the unaware, wrap/bind the system TLS provider (defacto or standard) in a common interface. So, on MacOS it uses Secure Transport and Windows it uses SChannel and falls back to OpenSSL elsewhere.

@museun
Copy link
Owner

museun commented Jun 4, 2020

Some notes...

Linux MIPS (and other older archs) can't build ring . ring is a component of rustls.

tokio won't work on wasm until mio is updated/replaced. tokio-rs/tokio#1597

For platforms where its problematic to build a TLS library, we can just note that TLS isn't supported.

I only provide the connect functions out of convenience. The crate is absolutely usable by providing your own impl of std::io::Read/Write (sync) and tokio::io::AsyncRead/AsyncWrite (async). So, these features being disabled isn't a problem. The crate is intended to be abstract over those interfaces -- so one could provide a file, a socket, whatever and it'll behave the same way.

I'm going to merge it and then trim off the defective feature sets later. As long as it builds with 'tokio + futures' and 'no-default-features', I'll be satisfied. I fixed the conflict of enabling both TLS libraries already. Those feature tests could actually be removed from the "lesser" platforms/tiers.

@museun museun merged commit 017915c into museun:dev Jun 4, 2020
@Chronophylos Chronophylos deleted the build-matrix branch June 5, 2020 06:39
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.

None yet

2 participants