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

Add support for illumos target (0.6.x) #1294

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

pfmooney
Copy link
Contributor

With support for an illumos target on its way into rust, the cfg guards for solaris should be updated to include illumos as well.

@pfmooney
Copy link
Contributor Author

This will help untangle illumos from Solaris, as noted in #1263.

@pfmooney
Copy link
Contributor Author

Updated to require minimum working libc version bump and reflect alphabetical sorting request made in #1295.

@pfmooney pfmooney changed the title Add support for illumos target Add support for illumos target (0.6.x) Apr 15, 2020
@Thomasdezeeuw
Copy link
Collaborator

Let's focus on getting #1295 merged first, I don't know if we want to add support for a new OS to v0.6.

@jclulow
Copy link
Contributor

jclulow commented Apr 15, 2020

Hi @Thomasdezeeuw!

From our perspective in the illumos community, we're pretty keen to get 0.6 support done before 0.7 support. Here is a small shell script that collects all of the requested versions of mio from https://crates.io:

#!/bin/bash

set -o errexit
set -o pipefail

crate="mio"
url="https://crates.io/api/v1/crates/$crate/reverse_dependencies?page="

n=1
while :; do
	if ! res=$(curl -s -f "${url}${n}"); then
		printf 'ERROR: request page %d failed\n' "$n" >&2
		exit 1
	fi

	if ! len=$(jq '.versions | length' <<< "$res"); then
		exit 1
	fi

	if (( len == 0 )); then
		exit 0
	fi

	for (( i = 0; i < len; i++ )); do
		if ! name=$(jq -r ".versions[$i].crate" <<< "$res") ||
		    ! req=$(jq -r ".dependencies[$i].req" <<< "$res"); then
			exit 1
		fi

		# printf '%-24s %s\n' "$req" "$name"
		printf '%s\n' "$req"
	done

	(( n++ ))
done

Based on the output of this script, it's pretty plain that the 0.6 branch is by far and away the most common in the ecosystem:

$ ./assess.sh > /tmp/mio_versions.txt
$ wc -l /tmp/mio_versions.txt
259 /tmp/mio_versions.txt
$ grep -c '^\^0.6' /tmp/mio_versions.txt
215

215 of the 259 crates currently depend on a 0.6 version of mio, perhaps most prominently the flagship module tokio! While we're keen to help out with 0.7, as evidenced by Patrick's other PR, we'd like to be able to build and run all that existing software on our platform today.

Please let us know how we can help to move this forward!

@Thomasdezeeuw
Copy link
Collaborator

Ok we've decided to accept this for v0.6 once rustc is updated (rust-lang/rust#71145).

Please let us know how we can help to move this forward!

For the long term we need two things:

  • CI: preferably where we can run cargo test, but being able to run cargo check is sufficient. Seeing how its a new target I don't expect tier 1 support in Rustc so we can't (easily) cross check from another OS at the moment.
  • Maintainer(s): as far as I know no-one on the Mio team currently has access to a Illumos machine, so if we can ping someone for Illumos specific bug reports that would be great.

@jclulow
Copy link
Contributor

jclulow commented Apr 15, 2020

Ok we've decided to accept this for v0.6 once rustc is updated (rust-lang/rust#71145).

Thank you very much! Deeply appreciated.

For the long term we need two things:

  • CI: preferably where we can run cargo test, but being able to run cargo check is sufficient. Seeing how its a new target I don't expect tier 1 support in Rustc so we can't (easily) cross check from another OS at the moment.

We're definitely conscious of not being supported by the big CI/CD providers, and can look into what we can do for mio.

  • Maintainer(s): as far as I know no-one on the Mio team currently has access to a Illumos machine, so if we can ping someone for Illumos specific bug reports that would be great.

I have created a team, @illumos/rust, to house a list of people you can ask about issues! I've added @pfmooney and myself to start off.

@jclulow
Copy link
Contributor

jclulow commented Apr 15, 2020

Looking closer at this, I'm not sure if people outside an organisation can see a particular team. If you want to create a GitHub team in the tokio-rs organisation to track the list of maintainers you can ping for illumos issues, Patrick and I would be happy to be listed there, too.

@Thomasdezeeuw
Copy link
Collaborator

@jclulow @pfmooney I've created @tokio-rs/illumos and invited both of you. I couldn't make it a sub-team of Mio so you might get people pinging that team on other tokio repos.

As for this pr, it LGTM, but the CI is having some problems (I'm trying to fix it in #1297).

@pfmooney
Copy link
Contributor Author

As for this pr, it LGTM, but the CI is having some problems (I'm trying to fix it in #1297).

Is there something I should do to re-trigger the CI build?

@kleimkuhler
Copy link
Contributor

kleimkuhler commented Apr 20, 2020

@pfmooney If you can merge master branch in to this, the FreeBSD check was just updated and should not hang anymore.

Edit: Nevermind this is merging into 0.6 branch. May have to do the same update there. I will get that change in.

@pfmooney
Copy link
Contributor Author

Edit: Nevermind this is merging into 0.6 branch. May have to do the same update there. I will get that change in.

I'm happy to rebase my bits once the 0.6.x branch is in the desired state.

@kleimkuhler
Copy link
Contributor

@pfmooney You should be okay to rebase now. Thanks for the patience on this!

@pfmooney pfmooney force-pushed the illumos-target-0.6 branch from 3c0a75a to 6fbf73a Compare April 21, 2020 00:26
@pfmooney
Copy link
Contributor Author

@pfmooney You should be okay to rebase now. Thanks for the patience on this!

Thanks for getting the 0.6.x CI situation squared away!

Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Thomasdezeeuw should take one more look, but looks good to me.

@@ -38,7 +38,7 @@ fuchsia-zircon = "0.3.2"
fuchsia-zircon-sys = "0.3.2"

[target.'cfg(unix)'.dependencies]
libc = "0.2.42"
libc = "0.2.54"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfmooney I assume this is a sufficient up-to-date version, I didn't manually check this.

@Thomasdezeeuw Thomasdezeeuw merged commit 0aed20a into tokio-rs:v0.6.x Apr 21, 2020
@Thomasdezeeuw
Copy link
Collaborator

@pfmooney @jclulow thanks!

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.

4 participants