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

Relax racy fairness test #495

Merged
1 commit merged into from May 19, 2020
Merged

Relax racy fairness test #495

1 commit merged into from May 19, 2020

Conversation

jonhoo
Copy link
Contributor

@jonhoo jonhoo commented May 3, 2020

Fixes #477

@ghost
Copy link

ghost commented May 3, 2020

This looks good to me, but seems like CI is broken. I'm inclined to merge anyway and take care of that later...

@taiki-e
Copy link
Member

taiki-e commented May 3, 2020

seems like CI is broken

It seems due to signal-hook's MSRV was increased to 1.31: vorner/signal-hook@e63edcf

@taiki-e
Copy link
Member

taiki-e commented May 3, 2020

It seems due to signal-hook's MSRV was increased to 1.31: vorner/signal-hook@e63edcf

So we need to either:

  • Downgrade signal-hook's patch version in MSRV CI (add cargo update -p signal-hook --precise 0.1.13):

    - name: Downgrade dependencies
    if: matrix.rust == '1.28.0'
    run: |
    cargo generate-lockfile
    cargo update -p cfg-if --precise 0.1.9

  • Bump crossbeam's MSRV to 1.31

@jonhoo
Copy link
Contributor Author

jonhoo commented May 3, 2020

One upside of the latter is that it would also mean #487 no longer needs to bump the MSRV. But that's definitely minor.

@taiki-e
Copy link
Member

taiki-e commented May 3, 2020

I have tried the first way (downgrade deps) locally, but it seems that the MSRV of so many dependencies is actually increasing, so this seems to be difficult...

@jonhoo
Copy link
Contributor Author

jonhoo commented May 3, 2020

One issue here is that the docs say:

The minimum supported Rust version is 1.28. Any change to this is considered a breaking change.

That is a very strict policy. The policy should (probably) instead be "six months" or something? But as it stands, we'd need to issue new major releases of all the crossbeam crates to do that... It also definitely gets weird since people will have to run through all sorts of hoops to actually get it running with 1.28 — so are we even really supporting MSRV 1.28?

@taiki-e
Copy link
Member

taiki-e commented May 3, 2020

I agree with the current policy needs to be changed as suggested in #444 (comment).

It also definitely gets weird since people will have to run through all sorts of hoops to actually get it running with 1.28 — so are we even really supporting MSRV 1.28?

I mentioned this in #428 (comment), but I think it is ok to set MSRV based on the compilable version (without downgrading the dependencies).

@ghost
Copy link

ghost commented May 19, 2020

Let's merge and fix CI in a follow-up PR.

@ghost ghost merged commit de59f9b into crossbeam-rs:master May 19, 2020
@jonhoo jonhoo deleted the fix_racy_fairness branch May 19, 2020 20:52
@jonhoo jonhoo mentioned this pull request May 19, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

channel: ready::fairness2 test is racy
2 participants