[Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

List overview All Threads
Download

newer

older

[Einstein Toolkit] #379: Python...

[Einstein Toolkit] #595: unused...

Einstein Toolkit

27 Sep 2011 27 Sep '11

11:30 a.m.

#586: Syntax errors in make/make.config.rules.in ----------------------+----------------------------------------------------- Reporter: eschnett | Owner: Type: defect | Status: new Priority: critical | Milestone: Component: Cactus | Version: Keywords: | ----------------------+----------------------------------------------------- The file make.config.rules.in contains shell syntax errors, e.g. in this construct:

define NOTIFY_PREPROCESSING { if test "$(VERBOSE)" == "yes"; then echo Preprocessing $<; fi }

1. "test" doesn't know the "==" operator, it's "=" instead 2. Commands within { } need to be terminated by a semicolon

Also:

3. In other places, we allow both upper and lower case settings

And:

4. The { } seem superfluous here; the if ... fi already brackets the statement 5. The VERBOSE variable could be checked by make, not by the shell

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Show replies by date

Einstein Toolkit

27 Sep 27 Sep

2:17 p.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

#586: Syntax errors in make/make.config.rules.in -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: accepted Priority: critical | Milestone: Component: Cactus | Version: Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by knarf):

* owner: => knarf * status: new => accepted

Comment:

Right, I should have thought of that. It runs on machines which have bash as default shell, since this is valid bash syntax, but it's not valid sh syntax. I will fix this.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:1 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

2:41 p.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

Comment (by eschnett):

The missing semicolon is required even for bash.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:2 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

3:28 p.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

Comment (by hinder):

Is there a good reason not to require bash? If you set SHELL = /bin/bash, make will use bash. Is there really a system that we need to target these days that does not have bash installed? If we can assume bash, we don't have to worry about conforming to sh features.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:3 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

5:12 p.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

Comment (by eschnett):

In this case it's only a question of writing = instead of ==.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:4 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

10:15 p.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

#586: Syntax errors in make/make.config.rules.in -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: closed Priority: critical | Milestone: Component: Cactus | Version: Resolution: fixed | Keywords: -----------------------+---------------------------------------------------- Changes (by knarf):

* status: accepted => closed * resolution: => fixed

Comment:

This should be fixed now. The == bashism is gone, the {} as well (although the semicolon was not necessary, as the statement inside ended with fi). upper case is implemented, and checking VERBOSE by make itself would require a larger patch, so I don't do this right now.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:5 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

10:18 p.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

#586: Syntax errors in make/make.config.rules.in -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: closed Priority: critical | Milestone: Component: Cactus | Version: Resolution: fixed | Keywords: -----------------------+----------------------------------------------------

Comment (by knarf):

As to why we should not assume bash: we should ask the other way around: unless we really have to use bash we should stick to sh, because it is more portable. I agree that bash is probably around on almost every machine you can think of, but there might be some where it is not, is in a non-standard place, not in the path, or such. Also, bash can be a tiny bit slower than a light replacement, which is why some linux distributions change their standard shell away from bash (but still install it).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:6 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

28 Sep 28 Sep

9:03 a.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

#586: Syntax errors in make/make.config.rules.in -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: reopened Priority: critical | Milestone: Component: Cactus | Version: Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by eschnett):

* status: closed => reopened * resolution: fixed =>

Comment:

This statement

VERBOSE=`echo $VERBOSE | tr '[:upper:]' '[:lower:]'`; \ if test "$(VERBOSE)" = "yes"; then echo Preprocessing $<; fi

checks an empty shell variable $VERBOSE first, and the make variable $(VERBOSE) later. That is, the conversion to lower case doesn't happen.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:7 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

11:04 p.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

#586: Syntax errors in make/make.config.rules.in -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: reopened Priority: critical | Milestone: Component: Cactus | Version: Resolution: | Keywords: -----------------------+----------------------------------------------------

Comment (by knarf):

Please explain further.

1. 'make sim VERBOSE=YES' does the same as 'make sim VERBOSE=yes' for me: producing verbose output. What does it do for you? 2. grep for 'upper' in lib/make and you will find this kind of conversion for other variables as well. What is special about VERBOSE? Or do you imply that this is wrong for all of these variables?

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:8 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

29 Sep 29 Sep

8:24 a.m.

New subject: [Einstein Toolkit] #586: Syntax errors in make/make.config.rules.in

#586: Syntax errors in make/make.config.rules.in -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: reopened Priority: critical | Milestone: Component: Cactus | Version: Resolution: | Keywords: -----------------------+----------------------------------------------------

Comment (by eschnett):

The first line sets a shell variable VERBOSE, the second line checks a make variable VERBOSE. These are different variables. The first line is therefore inconsequential. These line should instead read

{{{ VERBOSE=`echo $(VERBOSE) | ... if test "$$VERBOSE" ... }}}

If you say VERBOSE=YES, then the lines "Preprocessing" are never output.

Another way of implementing this would be

{{{ ifneq ($(shell echo $(VERBOSE) | tr '[:upper:]' '[:lower:]'),yes) define NOTIFY_PREPROCESSING endef ... other definitions else define NOTIFY_PREPROCESSING echo "Preprocessing $<..." endef ... other definitions endif }}}

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:9 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

4:01 p.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files (was: Syntax errors in make/make.config.rules.in)

#586: wrong usage of VERBOSE in Cactus make files -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: accepted Priority: major | Milestone: ET_2011_10 Component: Cactus | Version: Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by knarf):

* priority: critical => major * status: reopened => accepted * milestone: => ET_2011_10

Comment:

Thanks Erik. I attached a patch which implements your suggestion.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:10 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

5:31 p.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files

Comment (by eschnett):

I see lines containing "-n" being output. This is probably due to the line

{{{ echo -n "" }}}

Just saying "echo" instead should also work. On the other hand, there seem to be too many such empty lines, and outputting nothing instead may be better.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:11 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

10:14 p.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files

Comment (by knarf):

{{{ echo "" }}} without the -n option produces an empty line (a newline is output). The -n option prevents this newline, at least the echo command I know does have that option. The "-n" isn't output on my system. Can you check if your echo command lacks it? That would explain why lines containing "-n" are output there. If there are systems with the echo command lacking this option we would have to do something else instead. This line is supposed to simply do nothing; we should be able to do come up with another command for that, maybe a single ":" might work just as well, at least when I test it in sh, tcsh and bash.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:12 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

30 Sep 30 Sep

9:08 a.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files

Comment (by eschnett):

Yes, this seems to be the case on my system (Mac OSX). There are several echos available; there is /bin/echo, the bash builtin echo, and likely also a dash builtin echo, with dash being used as /bin/sh. My echo man page states that -n is available, but is not standard, and recommends printf instead.

I would use : for a no-op command.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:13 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

3 Oct 3 Oct

9:19 a.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files

Comment (by knarf):

I added a new patch which implements the : as no-op command. It also addresses the VERBOSE-issue in the Makefile itself, and sets VERBOSE=yes if SILENT is set to no for backwards-compatibility. I believe all issues raised so far are now addressed. Please review.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:14 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

10:48 a.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files

Comment (by eschnett):

NOTIFY_DIVIDER in lib/make/make.configuration has a colon preceded by a space; that space should instead be a tab.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:15 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

10:51 a.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files

#586: wrong usage of VERBOSE in Cactus make files -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: review Priority: major | Milestone: ET_2011_10 Component: Cactus | Version: Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by knarf):

* status: accepted => review

Comment:

This is fixed in a new patch.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:16 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

11:23 a.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files

#586: wrong usage of VERBOSE in Cactus make files -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: review Priority: major | Milestone: ET_2011_10 Component: Cactus | Version: Resolution: | Keywords: -----------------------+----------------------------------------------------

Comment (by eschnett):

Please apply.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:17 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

4 Oct 4 Oct

8:32 a.m.

New subject: [Einstein Toolkit] #586: wrong usage of VERBOSE in Cactus make files

#586: wrong usage of VERBOSE in Cactus make files -----------------------+---------------------------------------------------- Reporter: eschnett | Owner: knarf Type: defect | Status: closed Priority: major | Milestone: ET_2011_10 Component: Cactus | Version: Resolution: fixed | Keywords: -----------------------+---------------------------------------------------- Changes (by knarf):

* status: review => closed * resolution: => fixed

Comment:

Applied, plus an updated documentation in this regard.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/586#comment:18 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

5484

Age (days ago)

5491

Last active (days ago)

trac@lists.einsteintoolkit.org

18 comments

1 participants

tags (0)

participants (1)

Einstein Toolkit