[PATCH] D129559: Windows packaging script. Include lld-link, llvm-lib and llvm-windres from stage0

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/utils/release/build_llvm_release.bat:71
 
+if exist %build_dir% rd /s /q %build_dir%
 mkdir %build_dir%
----------------
hans wrote:
> I don't think we should do this. I think we should exit and let the user decide what to do.
Changed to `Displays a message and exit`.


================
Comment at: llvm/utils/release/build_llvm_release.bat:77
 curl -L https://github.com/llvm/llvm-project/archive/%revision%.zip -o src.zip || exit /b
-7z x src.zip || exit /b
+7z x -y src.zip || exit /b
 mv llvm-project-* llvm-project || exit /b
----------------
hans wrote:
> What are the 7z queries that need "yes"? I don't remember seeing any.
Removed the `-y` flag.
It was left there before the change to remove any existing build directory to overwrite the existing files being extracted.


================
Comment at: llvm/utils/release/build_llvm_release.bat:113
+REM Set Python environment
+set PYTHONHOME=%python32_dir%
+set PYTHONPATH=%PYTHONHOME%\lib
----------------
hans wrote:
> Is there a reason for these new variables, instead of just using `%python32_dir%' "inline" as in the old code?
Just to add clarity about the location of Pyhton. The only variable that I think is good to keep is `PYTHONHOME`.



================
Comment at: llvm/utils/release/build_llvm_release.bat:144
+  -DCMAKE_C_FLAGS="-Wno-nonportable-include-path" ^
+  -DCMAKE_CXX_FLAGS="-Wno-nonportable-include-path" ^
+  -DCMAKE_C_COMPILER=%stage0_bin_dir%/clang-cl.exe ^
----------------
hans wrote:
> These flags seem new?
Sorry. These are from another unrelated change. Removed.


================
Comment at: llvm/utils/release/build_llvm_release.bat:149
+  -DCMAKE_AR=%stage0_bin_dir%/llvm-lib ^
+  -DCMAKE_RC=%stage0_bin_dir%/llvm-windres
+set cmake_flags=%all_cmake_flags:\=/%
----------------
hans wrote:
> Could the LINKER, AR, and RC changes be done separately from this patch?
Removed LINKER, AR and RC options. To be done separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129559