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

Fix synchronization of datetime tests #867

Merged
merged 3 commits into from
Apr 11, 2020

Conversation

kngwyu
Copy link
Member

@kngwyu kngwyu commented Apr 11, 2020

As reported by @programmerjake by this comment, the lock! macro used in the datetime tests doesn't work.
This PR

  • Fixes the synchronization
    and
  • Remove nighly feature from parking_lot

@davidhewitt
I'm sorry that this PR conflicts with #866. I wanted to fix my fault myself.

@kngwyu
Copy link
Member Author

kngwyu commented Apr 11, 2020

I can still see an odd error 🤔
image

};
}
// TODO(kngwyu): Remove this variable
const MUTEX: RawMutex = RawMutex::INIT;
Copy link
Member

Choose a reason for hiding this comment

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

I think a const is actually not a real variable until the location it's used, so this still creates multiple copies of the mutex.

I think that the suggestion to use static on Travis is correct, as then there will only be one shared mutex:

Suggested change
const MUTEX: RawMutex = RawMutex::INIT;
static Mutex: RawMutex = RawMutex::INIT;

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, thank you.

@kngwyu kngwyu force-pushed the datetime-test-fix-try2 branch from e70ed61 to 145b917 Compare April 11, 2020 07:32
@davidhewitt
Copy link
Member

Looks like something weird has happened to travis again. I wonder if we should be migrating to github actions to simplify the CI?

@kngwyu
Copy link
Member Author

kngwyu commented Apr 11, 2020

Looks like something weird has happened to travis again.

No worry, it was because of a bug.

@kngwyu kngwyu merged commit 5e285fd into PyO3:master Apr 11, 2020
@kngwyu kngwyu deleted the datetime-test-fix-try2 branch April 11, 2020 10:10
@davidhewitt davidhewitt mentioned this pull request Apr 12, 2020
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.

2 participants