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.

Sign up for GitHub

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

Jump to bottom

build: fix indeterminacy of icu_locales value #42865

Merged
merged 1 commit into from
May 6, 2022
Merged

build: fix indeterminacy of icu_locales value #42865

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 Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Apr 25, 2022
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 25, 2022
@3ap
build: fix indeterminacy of icu_locales value
bd69f29 
`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 Add this label to start a Jenkins CI on a PR. label Apr 25, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43687/

@github-actions github-actions bot mentioned this pull request Apr 26, 2022
Open
14 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43705/

This was referenced Apr 27, 2022
Open
Open
@3ap
Copy link
Contributor Author

3ap commented Apr 28, 2022 •
edited
Loading

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?

@github-actions github-actions bot mentioned this pull request Apr 29, 2022
Open
21 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/43772/

This was referenced Apr 30, 2022
Open
Open
@3ap
Copy link
Contributor Author

3ap commented May 6, 2022

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

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label May 6, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 6, 2022
@nodejs-github-bot nodejs-github-bot merged commit 5e9274a into nodejs:master May 6, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 5e9274a

RafaelGSS pushed a commit that referenced this pull request May 10, 2022
@3ap @RafaelGSS
build: fix indeterminacy of icu_locales value
48f4a71 
`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
Merged
juanarbol pushed a commit that referenced this pull request May 31, 2022
@3ap @juanarbol
build: fix indeterminacy of icu_locales value
e180413 
`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>
@juanarbol juanarbol mentioned this pull request May 31, 2022
Closed
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
@3ap @danielleadams
build: fix indeterminacy of icu_locales value
69ac672 
`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>
targos pushed a commit that referenced this pull request Jul 12, 2022
@3ap @targos
build: fix indeterminacy of icu_locales value
f03d5d0 
`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>
targos pushed a commit that referenced this pull request Jul 31, 2022
@3ap @targos
build: fix indeterminacy of icu_locales value
fba6e96 
`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>
@targos targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@3ap @guangwong
build: fix indeterminacy of icu_locales value
958aca0 
`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: nodejs/node#42865
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
@3ap 3ap deleted the patch-1 branch October 31, 2022 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mesteery Mesteery Mesteery approved these changes

@richardlau richardlau richardlau approved these changes

@lpinca lpinca lpinca approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@3ap @nodejs-github-bot @lpinca @richardlau @Mesteery