[PATCH] D129263: Windows packaging script. Check administrator permissions and/or 7-Zip version.

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/utils/release/build_llvm_release.bat:4
 
+REM Script for building the LLVM installer on Windows.
+REM
----------------
hans wrote:
> This line is saying the same as line 26. If we move the 7zip version check down before :begin, there would be no need to repeat it, and I think it would make sense to do that anyway.
Good point.


================
Comment at: llvm/utils/release/build_llvm_release.bat:6
+REM
+REM Note:
+REM   To enable the creation of symbolic links:
----------------
hans wrote:
> I'd suggest baking these notes into the one right before the actual check. Also a little more context may be needed -- the reader might not know why it's desirable to create symlinks.
> 
> How about something like : "7zip versions 21.x and higher will try to extract the symlinks in llvm's git archive, which requires running as administrator."
All notes replaced with
```
REM Note:
REM   7zip versions 21.x and higher will try to extract the symlinks in
REM   llvm's git archive, which requires running as administrator.

REM Check for correct 7-zip version and/or administrator permissions.
....
```



================
Comment at: llvm/utils/release/build_llvm_release.bat:12
+REM Check for correct 7-zip version and/or administrator permissions.
+for /f "delims=" %%i in ('7z.exe ^| findstr /r "2[1-3]"') do set version=%%i
+if not "%version%"=="" (
----------------
hans wrote:
> Any reason not to do 1-9 instead of 1-3?
Changing to 1-9.


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

https://reviews.llvm.org/D129263