Skip to content

doc: clarify ICU source layout for --with-icu-source - #62011

Open
skdas20 wants to merge 2 commits into
nodejs:mainfrom
skdas20:docs/clarify-icu-source-layout-60947
Open

doc: clarify ICU source layout for --with-icu-source#62011
skdas20 wants to merge 2 commits into
nodejs:mainfrom
skdas20:docs/clarify-icu-source-layout-60947

Conversation

@skdas20

@skdas20 skdas20 commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #60947

Summary

Clarifies --with-icu-source build docs to match current ICU release packaging and expected directory layout:

  • Updated example archive name from icu4c-##.#-src.tgz to icu4c-##.#-sources.tgz.
  • Clarified that --with-icu-source must point to an extracted ICU source archive layout.
  • Added note that a plain ICU Git checkout is not sufficient unless prepared into that layout.

Validation

Ran markdown lint on the changed file:

node tools/lint-md/lint-md.mjs BUILDING.md

Result: exit code 0.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/build
  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Feb 26, 2026
Comment thread BUILDING.md Outdated
Comment on lines +996 to +999
The directory passed to `--with-icu-source` must contain the extracted ICU
source layout (for example, `source/data/in/icudt*l.dat`). A plain Git checkout
of the ICU repository is not sufficient unless it has been prepared to match
this layout.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure this is incredibly clear with respect to what the paths or "prepared to match" mean. Converting from the source code tree to the compiled source archive is an active build step (make dist), not just a "layout"; I don't think we need to mention what to do with a checked-out source tree, only that it's not the way to do it.

Suggested change
The directory passed to `--with-icu-source` must contain the extracted ICU
source layout (for example, `source/data/in/icudt*l.dat`). A plain Git checkout
of the ICU repository is not sufficient unless it has been prepared to match
this layout.
(Note that simply cloning the icu4c repository will not work here; the source
directory must have been unpacked from the compiled `-sources` archive.)

Comment thread BUILDING.md
Comment on lines 1008 to 1021

```bash
./configure --with-intl=full-icu --with-icu-source=http://url/to/icu.tgz
```

#### Windows

First unpack latest ICU to `deps/icu`
[icu4c-**##.#**-src.tgz](https://icu.unicode.org/download) (or `.zip`)
as `deps/icu` (You'll have: `deps/icu/source/...`)

```powershell
.\vcbuild full-icu
```

@Renegade334 Renegade334 Feb 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also need to update the archive names here.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

@github-actions github-actions Bot added the stale label Jul 28, 2026
@skdas20

skdas20 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Renegade334 — applied both. Used your wording verbatim for the icu4c note, since you're right that make dist is an active build step rather than a matter of layout, and saying only that a clone isn't the way to do it is clearer than describing what to do with one. Also updated the archive name in the Windows section to -sources.tgz to match.

@skdas20
skdas20 force-pushed the docs/clarify-icu-source-layout-60947 branch from 34e0949 to 8bac4b8 Compare August 27, 2026 15:32
Signed-off-by: skdas20 <skdas5405@gmail.com>
Use the reviewer's wording for the note about cloning icu4c, and update
the archive name in the Windows section to match.

Signed-off-by: skdas20 <skdas5405@gmail.com>
@skdas20
skdas20 force-pushed the docs/clarify-icu-source-layout-60947 branch from 8bac4b8 to f47d9df Compare August 27, 2026 15:33
@skdas20

skdas20 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Pushed fixes for both failing lint jobs.

lint-js-and-md was flagging the note itself:

BUILDING.md
1010:2-1010:11 warning Use "<nothing>" instead of "Note that" prohibited-strings remark-lint

@Renegade334 — that's on your suggested wording rather than anything you could have known about; prohibited-strings bans "Note that" repo-wide. I've kept your phrasing and just dropped the opening:

(Simply cloning the icu4c repository will not work here; the source directory
must have been unpacked from the compiled -sources archive.)

lint-commit-message was failing on not ok 6 signed-off-by. That was true of the original commit on this PR as well, so it had been failing since the branch was opened — both commits now carry a Signed-off-by trailer.

@Renegade334 Renegade334 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 27, 2026
Comment thread BUILDING.md
@@ -992,21 +978,24 @@ that works for both your host and target environments.

You can find other ICU releases at
[the ICU homepage](https://icu.unicode.org/download).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
[the ICU homepage](https://icu.unicode.org/download).
[the ICU homepage](https://unicode-org.github.io/icu/download/).

With the https://unicode-org.github.io/icu/download/ page redirecting you elsewhere we should probably target the new location.

@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 27, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Commit Queue failed

   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
   ✘  Requested Changes: 1
   ✘  - Stewart X Addison (@sxa): https://github.com/nodejs/node/pull/62011#pullrequestreview-5043250790

The pull request was removed from the Commit Queue and labeled commit-queue-failed An error occurred while landing this pull request using GitHub Actions. . After resolving the failure, remove that label and add commit-queue Add this label to land a pull request using GitHub Actions. to retry.

Full Commit Queue output
- Loading data for nodejs/node/pull/62011
✔  Done loading data for nodejs/node/pull/62011
----------------------------------- PR info ------------------------------------
Title      doc: clarify ICU source layout for --with-icu-source (#62011)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     skdas20:docs/clarify-icu-source-layout-60947 -> nodejs:main
Labels     doc, build, commit-queue, commit-queue-squash, stale
Commits    2
 - doc: clarify ICU source layout for configure
 - doc: apply review feedback on ICU source archive wording
Committers 1
 - Sumit Kumar Das <151006536+skdas20@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/62011
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/62011
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 26 Feb 2026 18:36:34 GMT
   ✘  Requested Changes: 1
   ✘  - Stewart X Addison (@sxa): https://github.com/nodejs/node/pull/62011#pullrequestreview-5043250790
   ✔  Approvals: 1
   ✔  - René (@Renegade334): https://github.com/nodejs/node/pull/62011#pullrequestreview-5042759412
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

View workflow run

@github-actions github-actions Bot removed the stale label Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs for building with an ICU source directory don't work

4 participants