build: unbreak windows build of ICU source URL

- configure: don't assume ICU source is a path, it's sometimes a URL

Also:
- cleanup comments in configure.py and nodedownload.py
- feat: add a way to set ICU url on windows
- update docs for maintaining ICU per above url

Fixes: #55214

Author: srl295

configure.py

@@ -1798,7 +1798,8 @@ def glob_to_var(dir_base, dir_sub, patch_dir):
   return file_list
 
 def configure_intl(o):
-  def icu_download(path):
+  def icu_download():
+    """Download or verify ICU from the deps file (current_ver.dep)"""
     depFile = tools_path / 'icu' / 'current_ver.dep'
     icus = json.loads(depFile.read_text(encoding='utf-8'))
     # download ICU, if needed
@@ -1848,6 +1849,17 @@ def icu_download(path):
 
   with_intl = options.with_intl
   with_icu_source = options.with_icu_source
+  if not with_icu_source:
+    with_icu_source_path = None
+    # no --with-icu-source
+  elif with_icu_source.startswith('http://') or with_icu_source.startswith('https://'):
+    # --with-icu-source isn't a path.
+    with_icu_source_path = None
+  else:
+    # convert to a resolved path for the dep check (below)
+    with_icu_source = Path(with_icu_source).resolve()
+    # pre-convert to a path for later checks.
+    with_icu_source_path = Path(with_icu_source)
   have_icu_path = bool(options.with_icu_path)
   if have_icu_path and with_intl != 'none':
     error('Cannot specify both --with-icu-path and --with-intl')
@@ -1936,16 +1948,16 @@ def icu_download(path):
     icu_config['variables']['icu_full_canned'] = 1
   # --with-icu-source processing
   # now, check that they didn't pass --with-icu-source=deps/icu
-  elif with_icu_source and Path(icu_full_path).resolve() == Path(with_icu_source).resolve():
-    warn(f'Ignoring redundant --with-icu-source={with_icu_source}')
+  elif with_icu_source and Path(icu_full_path).resolve() == with_icu_source:
+    warn(f'Ignoring redundant --with-icu-source={options.with_icu_source}')
     with_icu_source = None
   # if with_icu_source is still set, try to use it.
   if with_icu_source:
     if Path(icu_full_path).is_dir():
       print(f'Deleting old ICU source: {icu_full_path}')
       shutil.rmtree(icu_full_path)
     # now, what path was given?
-    if Path(with_icu_source).is_dir():
+    if with_icu_source_path and with_icu_source_path.is_dir():
       # it's a path. Copy it.
       print(f'{with_icu_source} -> {icu_full_path}')
       shutil.copytree(with_icu_source, icu_full_path)
@@ -1956,13 +1968,15 @@ def icu_download(path):
         shutil.rmtree(icu_tmp_path)
       icu_tmp_path.mkdir()
       icu_tarball = None
-      if Path(with_icu_source).is_file():
+      if with_icu_source_path and with_icu_source_path.is_file():
         # it's a file. Try to unpack it.
-        icu_tarball = with_icu_source
-      else:
-        # Can we download it?
+        icu_tarball = str(with_icu_source.as_posix()) # resolved path
+      elif not with_icu_source_path:
+        # Can we download it? (not a path)
         local = icu_tmp_path / with_icu_source.split('/')[-1]  # local part
         icu_tarball = nodedownload.retrievefile(with_icu_source, local)
+      else:
+        error(f'Cannot find ICU, not a file, dir, or URL: --with-icu-source={options.with_icu_source}')
       # continue with "icu_tarball"
       nodedownload.unpack(icu_tarball, icu_tmp_path)
       # Did it unpack correctly? Should contain 'icu'
@@ -1972,15 +1986,15 @@ def icu_download(path):
         shutil.rmtree(icu_tmp_path)
       else:
         shutil.rmtree(icu_tmp_path)
-        error(f'--with-icu-source={with_icu_source} did not result in an "icu" dir.')
+        error(f'--with-icu-source={options.with_icu_source} did not result in an "icu" dir.')
 
   # ICU mode. (icu-generic.gyp)
   o['variables']['icu_gyp_path'] = 'tools/icu/icu-generic.gyp'
   # ICU source dir relative to tools/icu (for .gyp file)
   o['variables']['icu_path'] = icu_full_path
   if not Path(icu_full_path).is_dir():
     # can we download (or find) a zipfile?
-    localzip = icu_download(icu_full_path)
+    localzip = icu_download()
     if localzip:
       nodedownload.unpack(localzip, icu_parent_path)
     else:

doc/contributing/maintaining/maintaining-icu.md

@@ -109,15 +109,17 @@ Node.js is built.
 * Configure Node.js with the specific [ICU version](http://site.icu-project.org/download)
   you want to upgrade to, for example:
 
+(posix)
 ```bash
 ./configure \
     --with-intl=full-icu \
     --with-icu-source=https://github.com/unicode-org/icu/releases/download/release-67-1/icu4c-67_1-src.tgz
 make
 ```
-
-> _Note_ in theory, the equivalent `vcbuild.bat` commands should work also,
-> but the commands below are makefile-centric.
+(vcbuild)
+```shell
+vcbuild.bat full-icu with-icu-source https://github.com/unicode-org/icu/releases/download/release-67-1/icu4c-67_1-src.tgz test
+```
 
 * If there are ICU version-specific changes needed, you may need to make changes
   in `tools/icu/icu-generic.gyp` or add patch files to `tools/icu/patches`.

tools/configure.d/nodedownload.py

@@ -70,18 +70,19 @@ def checkHash(targetfile, hashAlgo):
 
 def unpack(packedfile, parent_path):
     """Unpacks packedfile into parent_path. Assumes .zip. Returns parent_path"""
+    packedsuffix = packedfile.lower().split('.')[-1]  # .zip, .tgz etc
     if zipfile.is_zipfile(packedfile):
-        with contextlib.closing(zipfile.ZipFile(packedfile, 'r')) as icuzip:
-            print(' Extracting zipfile: %s' % packedfile)
-            icuzip.extractall(parent_path)
-            return parent_path
+      print(' Extracting zip file: %s' % packedfile)
+      with contextlib.closing(zipfile.ZipFile(packedfile, 'r')) as icuzip:
+          icuzip.extractall(parent_path)
+          return parent_path
     elif tarfile.is_tarfile(packedfile):
-        with contextlib.closing(tarfile.TarFile.open(packedfile, 'r')) as icuzip:
-            print(' Extracting tarfile: %s' % packedfile)
-            icuzip.extractall(parent_path)
-            return parent_path
+      # this will support gz, bz2, xz, etc. See tarfile.open()
+      print(' Extracting tar file: %s' % packedfile)
+      with contextlib.closing(tarfile.TarFile.open(packedfile, 'r')) as icuzip:
+          icuzip.extractall(parent_path)
+          return parent_path
     else:
-        packedsuffix = packedfile.lower().split('.')[-1]  # .zip, .tgz etc
         raise Exception('Error: Don\'t know how to unpack %s with extension %s' % (packedfile, packedsuffix))
 
 # List of possible "--download=" types.

vcbuild.bat

@@ -135,6 +135,7 @@ if /i "%1"=="full-icu"      set i18n_arg=%1&goto arg-ok
 if /i "%1"=="intl-none"     set i18n_arg=none&goto arg-ok
 if /i "%1"=="without-intl"  set i18n_arg=none&goto arg-ok
 if /i "%1"=="download-all"  set download_arg="--download=all"&goto arg-ok
+if /i "%1"=="with-icu-source" set with_icu_source=%2&goto arg-ok-2
 if /i "%1"=="ignore-flaky"  set test_args=%test_args% --flaky-tests=dontcare&goto arg-ok
 if /i "%1"=="dll"           set dll=1&goto arg-ok
 if /i "%1"=="enable-vtune" set enable_vtune_arg=1&goto arg-ok
@@ -201,6 +202,7 @@ if defined enable_static    set configure_flags=%configure_flags% --enable-stati
 if defined no_NODE_OPTIONS  set configure_flags=%configure_flags% --without-node-options
 if defined link_module      set configure_flags=%configure_flags% %link_module%
 if defined i18n_arg         set configure_flags=%configure_flags% --with-intl=%i18n_arg%
+if defined with_icu_source  set configure_flags=%configure_flags% --with-icu-source=%with_icu_source%
 if defined config_flags     set configure_flags=%configure_flags% %config_flags%
 if defined target_arch      set configure_flags=%configure_flags% --dest-cpu=%target_arch%
 if defined debug_nghttp2    set configure_flags=%configure_flags% --debug-nghttp2
@@ -801,7 +803,7 @@ set exit_code=1
 goto exit
 
 :help
-echo vcbuild.bat [debug/release] [msi] [doc] [test/test-all/test-addons/test-doc/test-js-native-api/test-node-api/test-internet/test-tick-processor/test-known-issues/test-node-inspect/test-check-deopts/test-npm/test-v8/test-v8-intl/test-v8-benchmarks/test-v8-all] [ignore-flaky] [static/dll] [noprojgen] [projgen] [clang-cl] [small-icu/full-icu/without-intl] [nobuild] [nosnapshot] [nonpm] [nocorepack] [ltcg] [licensetf] [sign] [x64/arm64] [vs2022] [download-all] [enable-vtune] [lint/lint-ci/lint-js/lint-md] [lint-md-build] [format-md] [package] [build-release] [upload] [no-NODE-OPTIONS] [link-module path-to-module] [debug-http2] [debug-nghttp2] [clean] [cctest] [no-cctest] [openssl-no-asm]
+echo vcbuild.bat [debug/release] [msi] [doc] [test/test-all/test-addons/test-doc/test-js-native-api/test-node-api/test-internet/test-tick-processor/test-known-issues/test-node-inspect/test-check-deopts/test-npm/test-v8/test-v8-intl/test-v8-benchmarks/test-v8-all] [ignore-flaky] [static/dll] [noprojgen] [projgen] [clang-cl] [small-icu/full-icu/without-intl] [with-icu-source path-or-URL] [nobuild] [nosnapshot] [nonpm] [nocorepack] [ltcg] [licensetf] [sign] [x64/arm64] [vs2022] [download-all] [enable-vtune] [lint/lint-ci/lint-js/lint-md] [lint-md-build] [format-md] [package] [build-release] [upload] [no-NODE-OPTIONS] [link-module path-to-module] [debug-http2] [debug-nghttp2] [clean] [cctest] [no-cctest] [openssl-no-asm]
 echo Examples:
 echo   vcbuild.bat                          : builds release build
 echo   vcbuild.bat debug                    : builds debug build