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

[DEPRECATION] Remove references to deprecated rand.Seed #2953

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

lhardt
Copy link
Contributor

@lhardt lhardt commented Sep 30, 2024

(Note: reopened this PR after creating and squashing gofumpt commit)

Remove references to deprecated rand.Seed (#2650).

Context:

  • From Go 1.20 (Feb 2023) onwards, math/rand.Seed is deprecated, and math/rand is already seeded by default.
  • From Go 1.24 (Feb 2025?) onwards, math/rand.Seed will be a no-op, breaking the test TestPwgenNoCrand.

See method documentation in the link above for further info.

Code Changes:

  • We must have our own Rand variable, so that its value can be injected in tests;
  • I changed the test name of TestPwgenNoCrand to TestPwgenNoCrandFallback, to make the expected behaviour clearer;
  • Shred does not need to seed, nor does TestLoadFromEnv in config_test,
    • But note that TestPwgenNoCrandFallback must restore the randomness so that TestLoadFromEnv can work properly.

This also removes the side-effect that fsutil's Shred was overwriting the seed set by pwgen.

@lhardt
Copy link
Contributor Author

lhardt commented Sep 30, 2024

As mentioned above, this reopens the PR #2950, with gofumpt changes.

@lhardt
Copy link
Contributor Author

lhardt commented Sep 30, 2024

The DCO failure message is:
Expected "Leo Hardt ..." , but got "Léo Hardt ...". -- Can it be ignored or would you like me rebase and signoff again?

@dominikschulz
Copy link
Member

@lhardt I'm sorry, the check doesn't seem to handle non-ASCII nicely. Let's just ignore the check for now.

@dominikschulz dominikschulz merged commit 486c165 into gopasspw:master Sep 30, 2024
7 of 8 checks passed
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