[PATCH] D127938: Update Windows packaging script.

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/utils/release/build_llvm_release.bat:6
+REM
+REM Run the script with administrator permissions.
+if not exist %SYSTEMROOT%\SYSTEM32\WDI\LOGFILES (
----------------
thieta wrote:
> CarlosAlbertoEnciso wrote:
> > hans wrote:
> > > Wait, what? This seems undesirable.
> > Running the script from a standard DOS prompt, I get the following errors:
> > 
> > 
> > ```
> > ...
> > 7-Zip 21.06 (x64) : Copyright (c) 1999-2021 Igor Pavlov : 2021-11-24
> > 
> > Scanning the drive for archives:
> > 1 file, 223803667 bytes (214 MiB)
> > 
> > Extracting archive: src.zip
> > --
> > Path = src.zip
> > Type = zip
> > ...
> > ERROR: Cannot create symbolic link : A required privilege is not held by the client. : .\llvm-project-main\libclc\clspv64
> > ERROR: Cannot create symbolic link : A required privilege is not held by the client. : .\llvm-project-main\libcxx\test\std\pstl
> > ERROR: Cannot create symbolic link : A required privilege is not held by the client. : .\llvm-project-main\openmp\tools\analyzer\llvm-openmp-analyzer++
> > 
> > Sub items Errors: 17
> > 
> > Archives with Errors: 1
> > 
> > Sub items Errors: 17
> > ```
> > May be the configuration we use is incorrect.
> symlinks requires admin rights on windows unfortunately. But I have never seen this - so it has to be some place where it conditions the symlinking?
Removed the administrator check. I am assumming it is my local configuration.


================
Comment at: llvm/utils/release/build_llvm_release.bat:125
+REM Preserve original path
+set original_path=%PATH%
+
----------------
hans wrote:
> Maybe we should move this closer to the top, and do PATH=%original_path% at the start of build_32 and build_64 instead? That seems less brittle.
Done.


================
Comment at: llvm/utils/release/build_llvm_release.bat:184
+
+  7z x -y LLVM-%package_version%-win32.exe -orepack
+  rmdir /s /q repack\$PLUGINSDIR
----------------
thieta wrote:
> CarlosAlbertoEnciso wrote:
> > hans wrote:
> > > Since it takes quite a while to run, can this repackaging step be put behind a flag?
> > Any specific name for the flag?
> CompressInstaller maybe?
I am proposing to use long option names to accomodate the `CompressInstaller`, `build_dir_base` and `LogPath or TestLogPath` cases:


```
--build-32    --build-64     # build step
--test-32     --test-64      # test step
--pack-32     --pack-64      # package step
--repack-32   --repack-64    # repackagin step

--trace                      # debugging trace
--base-dir                   # build directory base
--log-file                   # log filename (in the same build directory) 
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127938/new/

https://reviews.llvm.org/D127938