Batch script for loop won't set variable correctly

Question

I have following code but I see variables inside for loop are not set correctly. I tried using ! mark too still that doesn't work.

set dev_root=.\
set prod_root=..\prod\
setlocal disableDelayedExpansion

for /f "tokens=1* delims=\" %%A in ('forfiles /s /m *.txt /c "cmd /c echo @relpath"') do for %%F in (^"%%B) do (
    set "file=%%~F"
    set dev_filepath="%dev_root%%file%"
    set prod_filepath="%prod_root%%file%"

    if exist %prod_filepath% (
            fc /b %dev_filepath% %prod_filepath% > nul
        if errorlevel 1 (
            sd edit %prod_filepath%
            echo f | xcopy /f /y %dev_filepath% %prod_filepath%
        ) 
    ) else (
        echo f | xcopy /f /y %dev_filepath% %prod_filepath%
            sd add %prod_filepath%
    )
)

endlocal

You need [delayed expansion](https://ss64.com/nt/delayedexpansion.html) and to use `copy` instead of `xcopy` as the pipe deactivates the delayed expansion. — npocmaka, Mar 30 '17 at 07:07
Possible duplicate of [Batch file variables initialized in a for loop](http://stackoverflow.com/questions/691047/batch-file-variables-initialized-in-a-for-loop) — geisterfurz007, Mar 30 '17 at 07:08
@npocmaka, the pipe does not deactivate delayed expansion, it is simply applied in the parent `cmd` instance before the pipe is actually executed; though I do agree `copy` is better as `echo f | xcopy` to copy a single file... — aschipfl, Mar 30 '17 at 07:35
As far as I understand your code, you are using `forfiles` to get relative paths; you could also use `xcopy /L /S /I "*.txt" "%TEMP%"` instead, which does not copy anything (due to `/L`) and is much faster than `forfiles`. And with the outer `for /F` loop, you are trying to remove the `.\ `prefix, is that correct? do you know that this does not disturb? for instance, a path like `.\root\dir\..\sub\.\folder\.` is the same as `root\sub\folder`... — aschipfl, Mar 30 '17 at 08:56
@aschipfl - I used xcopy instead of copy because I wanted parent directories also to get created in destination path if they are not already present — shravya, Mar 31 '17 at 03:42
This linkis helpful http://stackoverflow.com/questions/13805187/how-to-set-a-variable-inside-a-loop-for-f/13809834#13809834 — shravya, Mar 31 '17 at 06:43

score 0 · Answer 1 · answered Mar 30 '17 at 07:23

0

Try like this (though I have no idea what is sd):

set dev_root=.\
set prod_root=..\prod\
setlocal enableDelayedExpansion

for /f "tokens=1* delims=\" %%A in ('forfiles /s /m *.txt /c "cmd /c echo @relpath"') do for %%F in (^"%%B) do (
    set "file=%%~F"
    set "dev_filepath=%dev_root%%%~F"
    set "prod_filepath=%prod_root%%%~F"

    if exist "!prod_filepath!" (
            fc /b "!dev_filepath!" "!prod_filepath!" > nul 2>nul || (
                sd edit "!prod_filepath!"
                copy /Y "!dev_filepath!" "!prod_filepath!"

            )

    ) else (
        copy /Y "!dev_filepath!" "!prod_filepath!"
        sd add "!prod_filepath!"
    )
)

answered Mar 30 '17 at 07:23

npocmaka

55,367
18
148
187

for first iteration in the loop variable file is set as follows set "file=%~F" it takes the last set value of variable F instead of current set value Also !prod_filepath!" is not being replaced with its value. its coming as below EX: ..\prod\!prod_filepath! – shravya Mar 30 '17 at 16:42
@shravya - have you changed the third line with `setlocal enableDelayedExpansion` – npocmaka Mar 30 '17 at 16:58
Yes I have changed it – shravya Mar 31 '17 at 03:40
Found this link helpful http://stackoverflow.com/questions/13805187/how-to-set-a-variable-inside-a-loop-for-f/13809834#13809834 – shravya Mar 31 '17 at 06:43

aschipfl · Answer 2 · 2017-03-31T08:23:34.470

There are several things that I would change. The main thing is to replace forfiles by xcopy /L to retrieve relative paths, because the latter is much faster. Here is the reworked code containing a lot of explanatory rem remarks:

@echo off
setlocal DisableDelayedExpansion

rem /* Use the quoted `set` syntax (the `""` do not become part of the value);
rem    do not include the trailing `\` here, do that later when building paths: */
set "dev_root=."
set "prod_root=..\prod"

rem /* You should change to a dedicated working directory, because you are only
rem    using relative paths later: */
cd /D "%~dp0." & rem // (this is the script's parent directory, just as an example)

rem /* Use `xcopy /L` rather than `forfiles` to retrieve list of relative paths;
rem    `/L` avoids to copy anything; the output is preceded by the drive letter
rem    (e. g., `C:`), which is split off by the `for /F` options; the summary text
rem    `?? File(s)` is not enumerated by `for /F` as it does not contain a `:`;
rem    the returned items are not preceded by `.\` and are not in between `""`: */
for /F "tokens=2 delims=:" %%F in ('xcopy /L /S /I "*.txt" "%TEMP%"') do (
    rem /* Build source and target paths; there is no interim variable `file`
    rem    necessary, use `%%F` immediately; include path separators `\` here;
    rem    at this point, delayed expansion is still disabled: */
    set "dev_filepath=%dev_root%\%%F"
    set "prod_filepath=%prod_root%\%%F"
    rem /* Now let us enable delayed expansion at this point in order to be able
    rem    to read the previously built file paths; avoid immediate `%` expansion
    rem    and `for` variables beyond this point in order not to lose `!`: */
    setlocal EnableDelayedExpansion
    rem /* Always use quoted file paths in order to avoid trouble with white-spaces
    rem    and other special characters (`^`, `&`, `(`, `)`,...): */
    if exist "!prod_filepath!" (
        rem /* You can use the `||` operator to execute commands only if the
        rem    previous one failed, hence its exit is not zero: */
        fc /B "!dev_filepath!" "!prod_filepath!" > nul || (
            rem // I do not know the `sd` command, so I keep it, with quoted path:
            sd edit "!prod_filepath!"
            rem /* For copying a single file, you should use `copy` rather than
            rem    `xcopy`, because the former does not prompt for file/directory
            rem    (`F`/`D`); keep in mind the prompt is language-dependent;
            rem    before copying, create the destination directory: */
            md "!prod_filepath!\.." 2> nul 
            copy /Y "!dev_filepath!" "!prod_filepath!"
        )
    ) else (
        rem /* Again use `copy` rather than `xcopy` to copy a single file: */
        md "!prod_filepath!\.." 2> nul 
        copy /Y "!dev_filepath!" "!prod_filepath!"
        rem // I do not know the `sd` command, so I keep it, but with quoted path:
        sd add "!prod_filepath!"
    )
    rem /* End environment localisation at the end of the loop iteration in order
    rem    to have delayed expansion disabled at the beginning of the next loop
    rem    iteration and to avoid execeeding the nesting limit of `setlocal`: */
    endlocal

endlocal
exit /B

Batch script for loop won't set variable correctly

2 Answers2