0

My colleague and I have been pulling our hair out all day over this.

We have a simple Windows batch file. We want it to read from a text file whose file path we are generating programmatically, take the single numeric value in this file, and compare it to a local variable. But we're getting completely inexplicable behavior.

The file contains a single scalar number, such as the number 2. Here's the code:

ThisAppFlagFileName=foo.txt
if not exist "%HOMEPATH%\ourcompanyname\%ThisAppFlagFileName%" (
   ECHO do something here
) else (
    SET /P InstalledVersion=<"%HOMEPATH%\ourcompanyname\%ThisAppFlagFileName%"
    ECHO We think the file contains: %InstalledVersion%

    IF %InstalledVersion% GEQ %ThisVersionInstallDataNum% ( 
         ECHO Version %ThisVersion% of the %ThisAppVisibleName% has already been installed for this user; exiting.
         GOTO TheEnd
)
)

:TheEnd
Echo END

We keep getting an error reading 2 was unexpected at this time. So we inserted some trace message and, just in case the else was problematic, stuck to two different if statements:

ThisAppFlagFileName=foo.txt
if not exist "%HOMEPATH%\ourcompanyname\%ThisAppFlagFileName%" (
   ECHO do something here
)
ECHO Trace Message 1 before IF
if exist "%HOMEPATH%\ourcompanyname\%ThisAppFlagFileName%" (
    ECHO Trace Message 2 after IF before CD
    SET /P InstalledVersion=<%HOMEPATH%\ourcompanyname\%ThisAppFlagFileName%"
    ECHO We think the file contains: %InstalledVersion%

    IF %InstalledVersion2% GEQ %ThisVersionInstallDataNum% ( 
         ECHO Version %ThisVersion% of the %ThisAppVisibleName% has already been installed for this user; exiting.
         GOTO TheEnd
)
)

:TheEnd
Echo END

And we see only the first trace message (before the if statement), and not the second trace message. So our conclusion is that somehow the content of the file is being interpolated into the line if exist "%HOMEPATH%\ourcompanyname\%ThisAppFlagFileName%", but of course we don't understand why the first if not exist works but the second doesn't.

Can anyone spot the mistake, please? Environment is Windows 7 cmd.exe window, but we are hoping to deploy to both Windows 7 and Windows XP.

jbeldock
  • 2,755
  • 3
  • 18
  • 31
  • You're not reading the content of the file at all. You're setting `InstalledVersion` to `<` plus the filename (`<%ThisAppFlagFileName%`). `<` does not redirect in a `SET` assignment. You need to use a `for /f` loop to actually read the content of the file. (You can confirm this from a command prompt - type `set something= – Ken White Aug 03 '13 at 01:16
  • Apologies. I forgot the /p switch. I have edited the code. Online references had indicated SET /p does accept <. Will look up and try your approach. – jbeldock Aug 03 '13 at 01:29
  • `SET /P something= – Ken White Aug 03 '13 at 01:34
  • @KenWhite - SET /P works with redirection, as does any command that reads stdin. But it only reads the first line of the file. The OP's file likely has many lines. – dbenham Aug 03 '13 at 04:08
  • @dbenham: Can you cite a reference for that? It doesn't work here (Win7 64) in the quick tests I did before commenting. – Ken White Aug 03 '13 at 04:24
  • @KenWhite - It works fine on my Win7 64 machine, as well as the many other Win platforms I have used it on **many** times. I'm not aware of any ref material that explicitly states what commands work with redirected stdin, nor would I expect to see it since it is the expected behavior based on the definition of redirected input. Go to any batch/"DOS" site and you will find many SET /P usage examples. However, SET /P does not give the desired result with piped input because both sides of the pipe are executed in a new CMD thread, so the assignment is lost as soon as the pipe thread terminates. – dbenham Aug 03 '13 at 04:48
  • @dbenham: Ok. Must be something I'm doing here. :-) Not important enough to me to investigate now (although I probably will at some point to figure out what I'm doing wrong), which is why I posted a comment and not an answer. :-) I'm quite familiar with `SET /P`, BTW, but thanks for the suggestion that I investigate it. :-) I thought that what I said was that `SET /P` does not give the desired result with piped input, but perhaps I was mistaken. – Ken White Aug 03 '13 at 04:56
  • FWIW, I see lots of references to `SET /P` accepting redirection. See, e.g. (this thread)[http://stackoverflow.com/questions/3068929/how-to-read-file-contents-into-a-variable-in-a-batch-file] . But I also can't make it work, so I am now using the `FOR /F` approach, which is, at least, WORKING! :-) – jbeldock Aug 03 '13 at 15:38

2 Answers2

2

The issue here is that the entire IF expression is evaluated before the SET /P statement within it can be executed. InstalledVersion is not set yet, and so this invalid expression is evaluated:

IF GEQ 2 (

Nothing inside of the IF expression executes because it cannot be completely evaluated.

A solution is to enable delayed expansion and replace %InstalledVersion% with !InstalledVersion!, as described in this post.

You can also restructure the code so the GEQ comparison happens after the IF expression.

Community
  • 1
  • 1
Marc Litchfield
  • 1,144
  • 7
  • 9
2

Your code have several errors. The first line:

ThisAppFlagFileName=foo.txt

missed a set command, so it is tryed to be executed as ThisAppFlagFileName command. This mean that ThisAppFlagFileName variable is NOT defined in your program, so InstalledVersion variable is never read from the file.

All references to InstalledVersion variable must use Delayed Expansion, that is, enclose they between exclamation marks instead percents and include setlocal EnableDelayedExpansion command at beginning of your program.

setlocal EnableDelayedExpansion
set ThisAppFlagFileName=foo.txt
if not exist "%HOMEPATH%\ourcompanyname\%ThisAppFlagFileName%" (
    ECHO do something here
) else (
    SET /P InstalledVersion=<"%HOMEPATH%\ourcompanyname\%ThisAppFlagFileName%"
    ECHO We think the file contains: !InstalledVersion!

    IF !InstalledVersion! GEQ %ThisVersionInstallDataNum% ( 
         ECHO Version %ThisVersion% of the %ThisAppVisibleName% has already been installed for this user; exiting.
         GOTO TheEnd
    )
)

:TheEnd
Echo END

You must be aware that all variables that are modified inside parentheses must also be enclosed in exclamation marks instead percent signs. Search for "delayed expansion" for details.

Aacini
  • 65,180
  • 12
  • 72
  • 108
  • Thanks, AAcini! The missing `set` was just a typo when transcribing to StackOverflow, but the other stuff makes perfect sense. Thanks! – jbeldock Aug 03 '13 at 15:35