New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buildconf.bat: Fix echo for paths with special chars #379

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@tiwoc

tiwoc commented Aug 13, 2015

If there was a closing parenthesis ")" in the path leading to the checkout, buildconf.bat exited with an error:

c:\(example)\curl\>cmd /C buildconf.bat
Generating prerequisite files
\curl\Makefile was unexpected at this time.
buildconf.bat: Fix echo for paths with special chars
If there was a closing parenthesis ")" in the path leading to the
checkout, buildconf.bat exited with an error:

c:\(example)\curl\>cmd /C buildconf.bat
Generating prerequisite files
\curl\Makefile was unexpected at this time.
@tiwoc

This comment has been minimized.

Show comment
Hide comment
@tiwoc

tiwoc Aug 13, 2015

Just noticed that the quotes do not get stripped before printing, so we get output that looks like:

Generating prerequisite files
"* C:\(example)\curl\Makefile"
"* C:\(example)\curl\src\tool_hugehelp.c"
"* C:\(example)\curl\include\curl\curlbuild.h"

If anyone knows how to solve this, please let me know. The hacks that I found on StackOverflow made my brain hurt. Otherwise, I think it's better to have some quotes in the output than to have a broken build ;)

tiwoc commented Aug 13, 2015

Just noticed that the quotes do not get stripped before printing, so we get output that looks like:

Generating prerequisite files
"* C:\(example)\curl\Makefile"
"* C:\(example)\curl\src\tool_hugehelp.c"
"* C:\(example)\curl\include\curl\curlbuild.h"

If anyone knows how to solve this, please let me know. The hacks that I found on StackOverflow made my brain hurt. Otherwise, I think it's better to have some quotes in the output than to have a broken build ;)

@captain-caveman2k

This comment has been minimized.

Show comment
Hide comment
@captain-caveman2k

captain-caveman2k Aug 13, 2015

Member

Hi,

Thanks for the report and attempted fix.

The problem isn't the echo as such - it will work under certain circumstances such as in a brand new batch file with nothing else in it.

The problem is the if statement as it uses brackets - although I'm not too sure why.

Moving the echo outside the if or into a separate function call fixes the problem.

Member

captain-caveman2k commented Aug 13, 2015

Hi,

Thanks for the report and attempted fix.

The problem isn't the echo as such - it will work under certain circumstances such as in a brand new batch file with nothing else in it.

The problem is the if statement as it uses brackets - although I'm not too sure why.

Moving the echo outside the if or into a separate function call fixes the problem.

@jay

This comment has been minimized.

Show comment
Hide comment
@jay

jay Aug 14, 2015

Member

+1 on the patch. It's the right parenthesis. The command interpreter expands the variables before it evaluates the if block. It's easier to understand if you boil the other stuff away

  if exist Makefile.dist (
  echo * %CD%\Makefile
  )

The interpreter expands all variables first (think preprocessor):

if exist Makefile.dist (echo * C:\(example)\Makefile )

It starts to interpret it as an if statement:

if exist Makefile.dist (echo * C:\(example)

but then chokes because the leftover isn't expected: \Makefile was unexpected at this time.

I think the best way is to wrap it in quotes, that is what I do in many of my scripts because it is the most resilient. You may see some people use delayed variable expansion, ie !CD! instead of %CD%. Though that works, if you enable delayed expansion globally you're essentially trading one problem for another, because what happens is when other variables contain exclamation points they can be corrupted because they are interpreted as delayed variables to be expanded. For example echo Foo! bar! will output Foo.

If you really don't want quotes enable delayed expansion temporarily since nested setlocal is allowed:

  setlocal ENABLEDELAYEDEXPANSION
  if exist Makefile.dist (
  echo * !CD!\Makefile
  )
  endlocal

Of course if that block were more complicated and included variables expanded normally that contained exclamation points, their data would be corrupted (assuming you didn't do it purposely).

Member

jay commented Aug 14, 2015

+1 on the patch. It's the right parenthesis. The command interpreter expands the variables before it evaluates the if block. It's easier to understand if you boil the other stuff away

  if exist Makefile.dist (
  echo * %CD%\Makefile
  )

The interpreter expands all variables first (think preprocessor):

if exist Makefile.dist (echo * C:\(example)\Makefile )

It starts to interpret it as an if statement:

if exist Makefile.dist (echo * C:\(example)

but then chokes because the leftover isn't expected: \Makefile was unexpected at this time.

I think the best way is to wrap it in quotes, that is what I do in many of my scripts because it is the most resilient. You may see some people use delayed variable expansion, ie !CD! instead of %CD%. Though that works, if you enable delayed expansion globally you're essentially trading one problem for another, because what happens is when other variables contain exclamation points they can be corrupted because they are interpreted as delayed variables to be expanded. For example echo Foo! bar! will output Foo.

If you really don't want quotes enable delayed expansion temporarily since nested setlocal is allowed:

  setlocal ENABLEDELAYEDEXPANSION
  if exist Makefile.dist (
  echo * !CD!\Makefile
  )
  endlocal

Of course if that block were more complicated and included variables expanded normally that contained exclamation points, their data would be corrupted (assuming you didn't do it purposely).

captain-caveman2k added a commit that referenced this pull request Aug 14, 2015

@captain-caveman2k

This comment has been minimized.

Show comment
Hide comment
@captain-caveman2k

captain-caveman2k Aug 14, 2015

Member

I've pushed a fix that simply moves the echo outside of the if statement.

I don't want to enable delayed variable expansion as that relies on Windows NT and would break on legacy systems and like the author I don't like the use of quotes as they get output rather than stripped off.

An alternative fix is to move the echo and copy into it's own function and have the if call that function.

Member

captain-caveman2k commented Aug 14, 2015

I've pushed a fix that simply moves the echo outside of the if statement.

I don't want to enable delayed variable expansion as that relies on Windows NT and would break on legacy systems and like the author I don't like the use of quotes as they get output rather than stripped off.

An alternative fix is to move the echo and copy into it's own function and have the if call that function.

jgsogo added a commit to jgsogo/curl that referenced this pull request Oct 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment