What's wrong with my SVN hook script-----pre-commit.bat?

Question

I Wrote a pre-commit hook script for SVN which running on Windows (.bat).

The main purposes are:

1. checking the reversion log length;
2. prevent normal users deleting [repo]/trunk/ folder;
3. prevent normal users deleting [repo]/trunk/xxx/ folders;

But SVN always says:

command syntax not correct (exitcode 255)

The code is here:

@echo off
:: Stops commits that have empty log messages.
@echo off

setlocal

set REPOS=%1
set TXN=%2

Rem Check log length.
svnlook log -t "%TXN%" "%REPOS%" | findstr ".........." > nul
if %errorlevel% gtr 0 goto NoLog

@echo off
set Drop=No

Rem Check Delete operation on trunk
svnlook changed -t "%TXN%" "%Repos%" | findstr "^D[ ]*trunk//$"
if %ERRORLEVEL% == 0 (set Drop=Yes)

Rem Check Delete operation on subdirectory of Trunk
svnlook changed -t "%TXN%" "%Repos%" | findstr "^D[ ]*trunk/[.]*//$"
if %ERRORLEVEL% == 0 (set Drop=Yes)

if Drop == Yes
(goto DropTrunk)
else
(exit 0)

:NoLog
echo You must inpu reversion log, and not less than 10 characters! 1>&2
exit 1

:DropTrunk
echo Only admin can delete the trunk directory and its subdirectory! 1>&2
exit 1

Answer 1

Perhaps you simply need to tell your hook script where to find the svnlook command. From the SVN book:

By default, Subversion executes hook scripts with an empty environment—that is, no environment variables are set at all, not even $PATH (or %PATH%, under Windows). Because of this, many administrators are baffled when their hook program runs fine by hand, but doesn't work when invoked by Subversion. Administrators have historically worked around this problem by manually setting all the environment variables their hook scripts need in the scripts themselves.

From: http://svnbook.red-bean.com/en/1.8/svn.reposadmin.create.html#svn.reposadmin.hooks.configuration

Answer 2

The core problem of the error is the syntax of the if Drop == Yes query. The command interpreter does not know that the line if Drop == Yes is continued in the next line unless an open parenthesis appears.

So the statement must read:

if Drop == Yes (
    goto DropTrunk
) else (
    exit 0
)

or as a single line:

if Drop == Yes (goto DropTrunk) else (exit 0)

Type if /? in a command prompt window to see the correct if/else syntax.

But there is still another issue: if Drop == Yes compares the literal string Drop with Yes, which will never be true of course. To compare the value of the variable Drop you need to change it to this:

if %Drop% == Yes (
    goto DropTrunk
) else (
    exit 0
)

or:

if %Drop% == Yes (goto DropTrunk) else (exit 0)

---

For comparing strings like this, I would put quotation marks around the strings to avoid trouble with some special characters or with empty strings, and I would also use the /I switch to do a case-insensitive comparison:

if /I "%Drop%"=="Yes" (
    goto DropTrunk
) else (
    exit 0
)

or:

if /I "%Drop%"=="Yes" (goto DropTrunk) else (exit 0)

---

Anyway, I would do the whole approach differently, like the following:

@echo off
set "Drop="

Rem Check Delete operation on trunk
svnlook changed -t "%TXN%" "%Repos%" | findstr "^D[ ]*trunk//$" > nul
if %ErrorLevel% EQU 0 (set "Drop=Yes")

Rem Check Delete operation on subdirectory of Trunk
svnlook changed -t "%TXN%" "%Repos%" | findstr "^D[ ]*trunk/[.]*//$" > nul
if %ErrorLevel% EQU 0 (set "Drop=Yes")

if defined Drop (
    goto DropTrunk
) else (
    exit 0
)

As you can see, I avoided the string comparison by merely checking whether or not the variable Drop is defined, and I cleared it initially rather than setting it to No.

In addition, I changed the comparison operator for the ErrorLevel check from == (string comparison) to EQU (numeric comparison).