[PATCH] D135138: Detect errors in Windows packaging script

Tue Oct 4 04:29:13 PDT 2022

mstorsjo added inline comments.

================
Comment at: llvm/utils/release/build_llvm_release.bat:69
 echo.
-pause

----------------
hans wrote:
> pbo-linaro wrote:
> > hans wrote:
> > > I liked this, but I can see how it doesn't make sense for everyone.
> > I'm open to discuss this.
> > We are using a pre-script that downloads dependencies, and having this one to stop after some time, is not so handy.
> I think dropping it is fine.
FWIW, if there's an easy way to check if the current terminal is interactive or not (it's fairly easy to do this for unix shell scripts at least), it could be kept conditionally for such cases I think.

================
Comment at: llvm/utils/release/build_llvm_release.bat:128
+call "%vsdevcmd%" -arch=x86 || exit /b 1
+ at echo on
 mkdir build32_stage0
----------------
pbo-linaro wrote:
> mstorsjo wrote:
> > Why this added `@echo on` here? Does `call "%vsdevcmd%"` affect the echo mode of this surrounding script?
> Alas yes. Any script called with call "pollutes" current environment, including for echoing commands.
> So, an @echo off will disable echo in parent script.
> It's equivalent to a "source" in bash, which can be counter intuitive.
Ah, right - it's been a couple decades since I spent more time thinking about the intricacies of bat scripts... Then this sounds reasonable to me.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135138