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 illumos support to v0.1 branch #245

Closed
wants to merge 1 commit into from

Conversation

jclulow
Copy link
Contributor

@jclulow jclulow commented Apr 14, 2020

illumos forked from Solaris around a decade ago, but is also currently
without the "tm_gmtoff" member.

test result: ok. 31 passed; 0 failed; 0 ignored; 0 measured;
0 filtered out

The v0.1 branch of this crate is apparently still quite widely used, so I'd love to get a new version published to crates.io with this change inside! Thanks for taking a look.

illumos forked from Solaris around a decade ago, but is also currently
without the "tm_gmtoff" member.

    test result: ok. 31 passed; 0 failed; 0 ignored; 0 measured;
    0 filtered out
@jhpratt
Copy link
Member

jhpratt commented Apr 14, 2020

As I've mentioned in other issues, there will be one final release of 0.1 at some point in the future.

I'll take a look at this eventually — a 0.1 release won't happen for a bit regardless.

@jhpratt jhpratt added C-feature-request Category: a new feature (not already implemented) v0.1 labels Apr 14, 2020
@jclulow
Copy link
Contributor Author

jclulow commented Apr 14, 2020

I'd really appreciate it if we could reassess the "unsupported" position for the v0.1 branch.

There are no less than three PRs open against it at this moment, in no small part because people are trying to support it. I suspect this is because lots of other popular crates are using the 0.1 series API, and that there does not appear to really be an upgrade path: the types and routines used by consumers from 0.1 are for whatever absent from 0.2.

If you're looking for help or resources, please let us know -- I'm sure that at least some of us, myself included, would be happy to assist in maintenance of the 0.1 branch.

@jclulow
Copy link
Contributor Author

jclulow commented Apr 15, 2020

@jhpratt To give a sense of the scale of the problem, I knocked together a script to enumerate all of the reverse dependency versions exposed at https://crates.io:

#!/bin/bash

set -o errexit
set -o pipefail

crate="time"
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

This script emits one line per crate that depends on time, stating the version on which that crate depends:

$ head versions.txt 
^0.1.39
^0.1
^0.1
^0.2.6
^0.1.42
^0.1.39
^0.1

As per the Cargo documentation on caret requirements specification:

While SemVer says there is no compatibility before 1.0.0, Cargo considers 0.x.y to be compatible with 0.x.z, where y ≥ z and x > 0.

Of the 878 crates which depend on time, it seems that the lion's share of them depend on the 0.1 branch:

$ grep -c '^\^0.1' versions.txt 
751

The full breakdown of different requested versions appears below:

$ sort versions.txt | uniq -c | sort -n
      1 ^0.1.12
      1 ^0.1.15
      1 ^0.1.17
      1 ^0.1.20
      1 ^0.1.26
      1 ~0.1.26
      1 ^0.1.3
      1 ~0.1.32
      1 ~0.1.33
      1 >= 0.1.34
      1 = 0.1.35
      1 = 0.1.36
      1 ~0.1.38
      1 ~0.1.39
      1 >= 0.1.39, < 0.2
      1 ~0.1.40
      1 ^0.1.5
      1 ~0.2.8
      2 0.*
      2 >= 0.1
      2 ^0.1.24
      2 ^0.1.30
      2 ~0.1.37
      2 ^0.1.4
      2 >= 0.1.40
      2 ^0.2.0
      2 ^0.2.2
      2 ^0.2.7
      3 ^0
      3 ^0.1.14
      3 ^0.1.31
      5 ~0.1
      5 ^0.1.33
      5 ^0.2.1
      5 ^0.2.6
      6 ^0.1.32
      7 0.1.*
      7 ^0.1.0
      7 ^0.1.25
      7 ^0.1.41
      8 ~0.1.34
      9 ^0.2.9
     18 ^0.1.36
     18 ^0.2
     20 ^0.1.37
     21 ^0.1.38
     25 ^0.1.34
     27 ^0.1.40
     40 ^0.1.39
     42 *
     48 ^0.1.35
     86 ^0.1.42
    415 ^0.1

It seems better to continue to maintain the 0.1 branch as patches come in (e.g., for new platform support or small bug fixes) than to effectively require 750+ crates to perform work to cope with the fundamentally new API exposed in time 0.2.

@jhpratt
Copy link
Member

jhpratt commented Apr 15, 2020

I'm well aware that most crates haven't upgraded. Note that some of the largest reverse dependencies have already upgraded, even if they're not released yet. While the API seems quite different, in reality the Tm struct can be replaced by OffsetDateTime without a ton of effort. I have happily contributed feedback to crates seeking to upgrade in the past, and will continue to do so in the future.

Time v0.1 has been out since before Rust 1.0. It hasn't been actively supported for years; I took the crate over because of incompatibility across the ecosystem and the fact that it was trivial to create invalid data. 0.2 is about ensuring things are correct while also decreasing memory usage and expanding support to more operating systems (by relying on the standard library).

And just FYI, the 3 open PRs regarding v0.1 are either minimal changes or backporting changes from the standard library (which isn't strictly necessary). If I had sufficient time on my hands, there would already be a 0.1.43 release and support would be over. Things move on — 0.1 can't be supported forever.

@jclulow
Copy link
Contributor Author

jclulow commented Apr 16, 2020

I'm well aware that most crates haven't upgraded.
...
Time v0.1 has been out since before Rust 1.0. It hasn't been actively supported for years; I took the crate over because of incompatibility across the ecosystem and the fact that it was trivial to create invalid data. 0.2 is about ensuring things are correct while also decreasing memory usage and expanding support to more operating systems (by relying on the standard library).

Definitely. I found issue #136 where the handover went down, and I'd like to say @jhpratt I appreciate the work you're doing both in maintaining the old branch and in pushing 0.2 to a sustainable point. Open source work can be thankless and tiring and so I want to offer the help of @pfmooney and myself here -- if you can add us to the time-rs organisation, we can do the sustaining work on 0.1 to allow you to focus exclusively on 0.2 related matters. We can tend to any CI/CD issues, checking to make sure only serious bug fixes or build changes (like this one) go in, and direct most new work to 0.2.

Things move on — 0.1 can't be supported forever.

Total agreement here, and I think the new API in 0.2 is great. We're just looking to help keep the maintenance light on until the bulk of crates have had a chance to do the work required to cut over and test and release. Rust is very exciting for us and we'd love to be able to start using it in our core OS in place of C and Python and so on. As of rust-lang/rust#71145 we're officially in the compiler and ready to broaden access to Rust amongst our user community. That's a big part of our motivation to help out here, so that folks on our OS can begin using Rust today without having to float local patches all over the place.

Let us know how you'd like to structure a joint effort, and however else we can help out!

@jhpratt
Copy link
Member

jhpratt commented Apr 17, 2020

I'll take a look into adding the two of you to the organization tomorrow and get back with (some) details. Ultimately if you're willing to merge in fixes & support changes for v0.1, it's something less I have to do 🙂 . I will add in some basic protections to make sure nothing gets accidentally pushed to the v0.2 branch, as well.

@jhpratt
Copy link
Member

jhpratt commented Apr 17, 2020

@jclulow I've just invited both you and @pfmooney to join the @time-rs/v0-1-maintainers team. This should give you push access to the v0.1 branch (and any others you may create). There's also a discussion area as part of this.

Whenever you'd like a release, just ping me somewhere and I'll quickly review things before publishing!

@pfmooney
Copy link

v0.1 illumos support merged as ae65abc

@pfmooney pfmooney closed this Apr 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: a new feature (not already implemented)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants