X Tutup
The Wayback Machine - https://web.archive.org/web/20220517174250/https://github.com/nodejs/node/pull/42865
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

build: fix indeterminacy of icu_locales value #42865

Merged
merged 1 commit into from May 6, 2022

Conversation

3ap
Copy link
Contributor

@3ap 3ap commented Apr 25, 2022

icu_locales is generated by joining values from set data structure.
However, set doesn't guarantee an order, so the result of icu_locales is not determined.
For example, the result value could be 'en,root' or 'root,en'. This fix make it deterministic.

The main reason of this fix is to restore the reproducibility of the build because the value of icu_locales is embedded into node binary.

The minimal code below could show you the issue:

default = 'root,en'
locs = set(default.split(','))
locs.add('root')
result = ','.join(str(loc) for loc in locs)

print(result)

Even after several runs of this script the problem occurs:

→ python test.py
en,root
→ python test.py
root,en
→ python test.py
en,root

@nodejs-github-bot nodejs-github-bot added build needs-ci labels Apr 25, 2022
@richardlau richardlau added the request-ci label Apr 25, 2022
`icu_locales` is generated by joining values from `set` data structure.
However, `set` doesn't guarantee an order, so the result of
`icu_locales` is not determined. For example, the result value could be
'en,root' or 'root,en'. This fix makes it deterministic.

The main reason of this fix is to restore the reproducibility of the
build because the value of `icu_locales` is embedded into `node` binary.
@github-actions github-actions bot removed the request-ci label Apr 25, 2022
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 25, 2022

lpinca
lpinca approved these changes Apr 25, 2022
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 26, 2022

@3ap
Copy link
Contributor Author

@3ap 3ap commented Apr 28, 2022

I'm not sure I understand what's wrong with CI and failed tests, could anybody explain it to me? Should or could I do something to fix it?

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 29, 2022

@3ap
Copy link
Contributor Author

@3ap 3ap commented May 6, 2022

Ping? Looks like everything is okay and ready to merge.

@richardlau richardlau added the commit-queue label May 6, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue label May 6, 2022
@nodejs-github-bot nodejs-github-bot merged commit 5e9274a into nodejs:master May 6, 2022
49 checks passed
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented May 6, 2022

Landed in 5e9274a

RafaelGSS pushed a commit that referenced this issue May 10, 2022
`icu_locales` is generated by joining values from `set` data structure.
However, `set` doesn't guarantee an order, so the result of
`icu_locales` is not determined. For example, the result value could be
'en,root' or 'root,en'. This fix makes it deterministic.

The main reason of this fix is to restore the reproducibility of the
build because the value of `icu_locales` is embedded into `node` binary.

PR-URL: #42865
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
@RafaelGSS RafaelGSS mentioned this pull request May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build needs-ci
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants
X Tutup