Proposal: v0.1.x lint fix, dependencies and MSRV updates

[dekellum]

I'd like to propose a set of related backports culminating in new releases, for the v0.1.x branch. If there are other changes being considered for v0.1.x, this might be a good way to organize them?

(1) Apply dyn keywords to avoid warnings on recent nightlies (MSRV 1.27.0)

These warnings are fatal with recent nightlies on a local git checkout, given the use of deny(warnings). When built as a dependency of a normal application, cargo passes --cap-lints=allow to rustc (or for cargo -vv, --cap-lints=warn at most), so these are not fatal in that case. The current azure-pipelines CI appears to only test with stable rust (e.g. 1.36.0) or older nightlies, so this hasn't broken CI, yet.

Adding the dyn keywords would change most workspace crates (see PR #1351), but given the present scope of the problem, I don't propose releases specifically for this change. Subsequent 0.1.x releases (including those proposed below) would include this though.

A related change (alternative or addition) is PR #1368 .

(2) Backport #1312: replace deprecated tempdir crate use

Effects (-fs)

(3) Backport #1324: remove last non-dev dependency on rand crate

Effects (-threadpool)

(4) Upgrade to rand 0.7.0 (dev-only dependency) (testing MSRV 1.32.0)

Effects (-fs -threadpool -timer)

(5) Backport #1298: Upgrade to parking_lot 0.9.0 (MSRV 1.31.0)

Effects (-reactor). Drops rand 0.6 deps.

(6) Backport any necessary update for crossbeam-deque

Might effect (-threadpool). See #1345. Its currently unclear if this will just be a transparent PATCH release. There is a pending security fix for crossbeam-deque → crossbeam-epoch → memoffset (from 0.2.1 to 0.5.1). The now released crossbeam-epoch-0.7.2 also upgraded to scopeguard 1.0.0 (avoiding duplicate version 0.3.3).

(7) CI change for testing and doc of updated MSRV 1.31.0

With rust 1.36.0 stable release, this is a legal MSRV update per README stated policy

(8) Releases: tokio-threadpool-0.1.16, tokio-reactor-0.1.9

No release is required for the tokio crate itself. Above changes to tokio-fs and tokio-timer are dev-dependency or test changes only, so no release is required.

Net local dev tree dependency changes, including transitive dependencies:

    Removing fuchsia-cprng v0.1.1
    Updating lock_api v0.1.5 -> v0.3.1
    Removing owning_ref v0.4.0
    Updating parking_lot v0.7.1 -> v0.9.0
    Updating parking_lot_core v0.4.0 -> v0.6.2
    Removing rand v0.4.6
    Removing rand v0.6.5
    Removing rand_chacha v0.1.1
    Removing rand_core v0.3.1
    Removing rand_core v0.4.0
    Removing rand_hc v0.1.0
    Removing rand_isaac v0.1.1
    Removing rand_jitter v0.1.4
    Removing rand_os v0.1.3
    Removing rand_pcg v0.1.2
    Removing rand_xorshift v0.1.1
    Removing rdrand v0.4.0
    Removing scopeguard v0.3.3
    Removing stable_deref_trait v1.1.1
    Removing tempdir v0.3.7

I have a branch for (1) I will independently PR. Then I have a staging branch for (2 3 4 5) so far, which I can extend, modeling on #1240, for version bumps and whatever other release prep I can offer.

Feedback and/or extensions to this plan?

cc: @LucioFranco

[LucioFranco]

I think this mostly looks good and are some good ideas. I think 2,3,4,5,6 and 8 look good.

I'm a bit cautious about bumping the MSRV since @carllerche seems to be pretty adamant about keeping it on 1.26 without real reason. Is there a way we can fix the dyn keyword warnings without updating the MSRV?

also remember to make the PR's against the v0.1.x branch.

[carllerche]

Personally, I would rather limit the 0.1 branch to bug fixes only at this point (to save time).

[carllerche]

We can silence warnings that we don't plan on fixing for CI.

[dekellum]

For me, its a question of how long I'll be using tokio 0.1.x. If that is measured in months, then I find this worthwhile. I also realize you might want to wait on this, and consider what else comes up for 0.1.x?

parking_lot 0.9.0 has bug fixes, see https://github.com/Amanieu/parking_lot/blob/master/CHANGELOG.md since parking_lot-0.7.1, and it requires MSRV 1.31.0. MSRV 1.32.0 is required to test with rand 0.7.0, but CI could be relaxed to only verify compiling on 1.31.0 (and test on 1.32.0).

My thought was that if MSRV goes to 1.27.0+ then no reason not to fix (1). I can offer that PR and you can hold off on merging, depending on how long tokio 0.1.x is deemed supported?

A possible alternative (apologies if this is too outlandish or causes other problems) would be to release this as an intermediate 0.2.x series and release master as 0.3 or 1.0.0. The later has the advantage of distinguishing MINOR from PATCH changes moving forward, which would be nice for things like MSRV increases.

[taiki-e]

Is there a way we can fix the dyn keyword warnings without updating the MSRV?

I think it is enough to remove deny(warnings). It is what the futures 0.1 do.

[dekellum]

Doing it this real way (#1351) makes sense, IMO, if any of other, MSRV raising, parts of this proposal are eventually accepted.

[taiki-e]

IIRC, we don't receive warnings from external crates other than compatibility warnings.

[dekellum]

Yes, I try to explain the --cap-lints flag and CI implications in #1351.

[taiki-e]

Oh, sorry, I missed that.

[dekellum]

CI testing already only performs cargo check type validation on the MSRV, so MSRV need only be raised to 1.31.0 for this. rust 1.32.0 is required to actually run the tests, given updated rand and other test dependencies. Edited proposed target tokio MSRV in above description.

[dekellum]

Reduced releases proposed above (8) to -threadpool and -reactor only. The other two, -fs and -timer, have dev dependency/testing only changes in this proposal so they don't require a release.

[dekellum]

Regarding the question of raising MSRV, here is a first example of dependencies deciding that for us: #1369 (to 1.28.0). I suspect that this will only continue with existing v0.1.x release dependencies, such that eventually 1.31.0+ will be the defacto MSRV. That argues in favor of accepting an MSRV increase for the good reasons described here.

[LucioFranco]

@dekellum I'm a bit against updating msrv to 1.31 for now since 0.1 is kinda in a LTS role. I think the best route here would be to apply the dyn keyword changes (msrv 1.27) and #1369 which should bump us to msrv of 1.28 which I think is acceptable for now. What do you think?

[dekellum]

I'm fine with that—making incremental progress and at least restoring CI and local tree building of v0.1.x. None of that requires a release at this time, as far as I can tell. Note that #1368 could also be applied in a similar vane, and its now effectively also a backport. I'll start a new staging/CI PR with the former, possibly including #1368 last. If that is accepted, then I'll just update this and its associated staging PR to reflect progress—I still think 1.31 and the upgrade proposed here could easily become necessary/desirably before tokio 0.1.x is EOL.

[LucioFranco]

@dekellum Perfect, let's do 1.28 for now. And I'd like to think a bit more about 1.31. I appreciate all the help!

[dekellum]

Items (1 2) have been merged as part of #1451. Remaining items (3 4 5 7) are revised and available in #1358, which is now the better successor. Closing.