Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-110932: Fix regrtest for SOURCE_DATE_EPOCH#111143
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
If the SOURCE_DATE_EPOCH environment variable is defined, use itsvalue as the random seed.
vstinner commentedOct 20, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
SOURCE_DATE_EPOCH intent is tomake programsreproducible. Without this change, if SOURCE_DATE_EPOCH is set, regrtest uses arandom seed which makes testsless reproducible. With this change, the randomseed is constant and so the behavior should bemore reproducible.
|
vstinner commentedOct 20, 2023
@sobolevn: Would you mind to review this change? |
sobolevn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Generally looks good to me, just one idea.
| elserandom.getrandbits(32) | ||
| ) | ||
| if'SOURCE_DATE_EPOCH'inos.environ: | ||
| if ('SOURCE_DATE_EPOCH'inos.environ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nit
| if ('SOURCE_DATE_EPOCH'inos.environ | |
| if ( | |
| 'SOURCE_DATE_EPOCH'inos.environ |
| # SOURCE_DATE_EPOCH should be an integer, but use a string to not | ||
| # fail if it's not integer. random.seed() accepts a string. | ||
| # https://reproducible-builds.org/docs/source-date-epoch/ | ||
| self.random_seed:int|str=os.environ['SOURCE_DATE_EPOCH'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Myabe we should always usestr? There's no real reason to useint here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The reason is subtle. It avoids to import hashlib to convert a string to an integer in Random.seed(). Right now, hashlib is always imported, but I have a local branch to reduce the number of imports at Python startup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm talking about the second code path, where we create a random seed as an integer.
| # https://reproducible-builds.org/docs/source-date-epoch/ | ||
| self.random_seed:int|str=os.environ['SOURCE_DATE_EPOCH'] | ||
| elifns.random_seedisNone: | ||
| self.random_seed=random.getrandbits(32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If so, it would become:
| self.random_seed=random.getrandbits(32) | |
| self.random_seed=str(random.getrandbits(32)) |
Thanks@vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
Thanks@vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
If the SOURCE_DATE_EPOCH environment variable is defined, use itsvalue as the random seed.(cherry picked from commit7237fb5)Co-authored-by: Victor Stinner <vstinner@python.org>
If the SOURCE_DATE_EPOCH environment variable is defined, use itsvalue as the random seed.(cherry picked from commit7237fb5)Co-authored-by: Victor Stinner <vstinner@python.org>
GH-111153 is a backport of this pull request to the3.11 branch. |
GH-111154 is a backport of this pull request to the3.12 branch. |
vstinner commentedOct 21, 2023
Merged, thanks to reviewing the change@sobolevn. |
If the SOURCE_DATE_EPOCH environment variable is defined, use itsvalue as the random seed.
If the SOURCE_DATE_EPOCH environment variable is defined, use itsvalue as the random seed.
Uh oh!
There was an error while loading.Please reload this page.
If the SOURCE_DATE_EPOCH environment variable is defined, use its value as the random seed.