installation issue when building with NO_CURL=YesPlease

As I could not find any bug reporting information on the wiki I thought I would mention this here, please let me know if there is a better forum for bug reports. I believe the latest git release (1.7.1.) has an installation bug when building with "NO_CURL=YesPlease". Looking at the Makefile line 1999 it reads

for p in $(REMOTE_CURL_ALIASES); do

which results in "/bin/sh: syntax error at line 1 : `;' unexpected" as REMOTE_CURL_ALIASES is empty.

Paul Walker venit, vidit, dixit 26.05.2010 15:35:

That seems to depend on the shell. My bash 4.1.2 doesn't care. What does /bin/sh --version say for you?

Michael

On 26 May 2010, at 14:58, Michael J Gruber wrote:

I am struggling to work this out as my version of sh does not support the --version flag. I tried a bunch of other options provided by google with not success. If it helps I believe that /bin/sh is a version of ksh that comes with AIX 6.1

Hi,

Bugs are usually reported on this mailing list.

I can't reproduce this. How exactly are you building? I'm using $ NO_CURL=YesPlease make ... and it builds fine for me.

-- Ram

On 26 May 2010, at 14:58, Ramkumar Ramachandra wrote:

I used:

gmake prefix=<path> NO_CURL=YesPlease NO_PYTHON=YesPlease all

which works fine, followed by

gmake prefix=<path> NO_CURL=YesPlease NO_PYTHON=YesPlease install

which reported the above error

Setting NO_CURL leaves some variables like REMOTE_CURL_ALIASES empty, which creates no fun when for-looping over $(REMOTE_CURL_ALIASES) unconditionally. Make it conditional.

Reported-by: Paul Walker <PWalker752@aol.com>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Against master but suggested for maint.
Note that the problem occurs during "make install", not "make".

 Makefile |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Am 26.05.2010 16:15 schrieb Paul Walker:

I had a similar problem when "make install"ing under AIX. Not with NO_CURL but with some other NO_* option. I forgot which. This yealded to an empty $(REMOTE_***_ALIASES) macro (REMOTE_CURL_ALIASES in your case) which my AIX shell cannot handle.

It reads "for p in; do" which makes it unhappy. I solved my problem with 
  the SHELL_PATH environment variable (look at the first few lines in 
the Makefile).

$ SHELL_PATH=/bin/bash NO_SOMETHING=YesPlease make install

then worked fine for me.

HTH,
     Dirk

Dirk Süsserott <newsletter@dirk.my1.cc> writes:

[...]

It wouldn't be too horrible to fix the Makefiles, though. Doing stuff like this works portably (judging by what some OpenSSL Makefiles do):

	foo="$(REMOTE_CURL_ALIASES)"; for i in $$foo; do \

[...]

Am 26.05.2010 20:45 schrieb Bruce Stephens:

Bruce,

I just saw that Michael posted a patch which tests for the emptyness of REMOTE_CURL_ALIASES. Probably that's a more convenient solution.

Dirk

From: Brandon Casey <drafnel@gmail.com>

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 Makefile |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

From: Brandon Casey <drafnel@gmail.com>

ksh does not like it when the list argument is missing in a for loop. This can happen when NO_CURL is set which causes REMOTE_CURL_ALIASES to be unset. In this case, the for loop in the Makefile expands to look like this:

   for p in ; do

and ksh complains like this:

   /bin/ksh: syntax error at line 15 : `;' unexpected

The existing attempt to work around this issue, introduced by 70b89f87, tried to protect the for loop by first testing whether REMOTE_CURL_ALIASES was empty, but it does not seem to work. So adopt Bruce Stephens's suggestion (which comes from OpenSSL) for working around this issue.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 Makefile |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Hi,

Brandon Casey <casey@nrlssc.navy.mil> writes:

[snip]

I believe the idiom

  test x'$(foo)' = x || for p in ''$(foo); do

works equally well in this case, and is less invasive.

- Hari

Brandon Casey venit, vidit, dixit 02.07.2010 20:50:

What does that mean? Either it works or it doesn't. I did work back then. Does it (i.e.: the test for emtyness) fail to work for certain shells?

Michael

Am 7/4/2010 20:37, schrieb Michael J Gruber:

Before the test for emptyness can happen, the complete statement must be parsed, but ksh finds a syntax error in the statement and, therefore, cannot even begin to execute the statement. (ksh doesn't follow POSIX in this regard, where this would not be a syntax error.)

-- Hannes

Johannes Sixt venit, vidit, dixit 05.07.2010 08:19:

OK, thanks for clarifying. I suggest this to go into the commit message so that the "does not seem to work" is qualified.

The OP back then (before 70b89f87) used ksh on AIX 6.1, but maybe he left the thread without testing. I assume Hari's suggestion works on ksh, as well?

If we go for Brandon's version: Is there a reason for small-casing the var name? It looks as if we had two different variables with different case (which we don't).

BTW: Is the $$var gmake specific? Has anyone tested the new version on,
say, AIX, not just on Linux with ksh?

Michael

Am 7/5/2010 10:14, schrieb Michael J Gruber:

No. $ is a (special?) make variable that contains only a dollar sign. To expand the variable in the Makefile, you have to write $$ (like for any other Makefile variable whose name has only a single character, like $@, $<, etc). As a result, you get a single dollar sign in the shell command text. :-)

-- Hannes

Johannes Sixt venit, vidit, dixit 05.07.2010 11:15:

I guess I need this in my personal tree:

Johannes Sixt <j.sixt@viscovery.net> writes:

I had to stare at Brandon's patch that was essentially:

-    for p in $(FOO); do echo $$p; done
+    foo=$(FOO); for p in $$foo; do echo $$p; done

and the above two doesn't look like there should be any difference; your explanation makes quite a lot of sense but that's arcane. I doubt I will be able to justify and explain the latter construction without consulting your message I am responsing to, if somebody asks "why do we assign $(FOO) to a shell variable and then iterate over it?" 6 months from now.

It might make sense to use $(foreach) instead of rolling our own loop in the shell to avoid glitches like this.

 Makefile |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

Junio C Hamano <gitster@pobox.com> writes:

                       ^^

Missing && at the end of the preceding line (otherwise the last line is effectively commented out).

Andreas.

It looks like Johannes has already supplied the explanation that was missing from my commit message. Thanks.

On Mon, Jul 5, 2010 at 1:18 PM, Junio C Hamano <gitster@pobox.com> wrote:

$(foreach) works too. I only avoided it because it has already caused a problem once before by creating a command line that exceeded the maximum argument list length on IRIX.

REMOTE_CURL_ALIASES only has 3 items in it right now, and probably won't grow much larger, if it grows at all, so there is little chance of exceeding the maximum argument list length on IRIX. So $(foreach) is fine with me if you think that reads better.

-Brandon

Brandon Casey <drafnel@gmail.com> writes:

Ok, fair enough.

Well your patch fixes the issue, and I would actually prefer it as long as it is explained well ;-).

Thanks.

From: Brandon Casey <drafnel@gmail.com>

ksh does not like it when the list argument is missing in a 'for' loop. This can happen when NO_CURL is set which causes REMOTE_CURL_ALIASES to be unset. In this case, the 'for' loop in the Makefile is expanded to look like this:

   for p in ; do

and ksh complains like this:

   /bin/ksh: syntax error at line 15 : `;' unexpected

The existing attempt to work around this issue, introduced by 70b89f87, tried to protect the 'for' loop by first testing whether REMOTE_CURL_ALIASES was empty, but this does not work since, as Johannes Sixt explains, "Before the test for emptyness can happen, the complete statement must be parsed, but ksh finds a syntax error in the statement and, therefore, cannot even begin to execute the statement. (ksh doesn't follow POSIX in this regard, where this would not be a syntax error.)".

Make's $(foreach) function could be used to avoid this shell glitch, but since it has already caused a problem once before by generating a command line that exceeded the maximum argument list length on IRIX, let's adopt Bruce Stephens's suggestion for working around this issue in the same way the OpenSSL folks have done it. This solution first assigns the contents of the REMOTE_CURL_ALIASES make variable to a shell variable and then supplies the shell variable as the list argument in the 'for' loop. This satisfies ksh and has the expected behavior even if $(REMOTE_CURL_ALIASES) is empty.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---

On 07/05/2010 09:36 PM, Junio C Hamano wrote:

Heh, ok, hopefully this commit message does a better job.

-Brandon

 Makefile |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)