WinGRASS g.version -c fails

[hamish]

Hi,

if you try 'g.version -c' to print the COPYING file it doesn't because the $GISBASE/etc/VERSION file is empty.

Apparently the g.version/Makefile sed magic is failing:

COPYING:
        cat ./../../COPYING | sed -f sed.script | tr -d '\012' > $(GRASS_VERSION_FILE)

sed.script contains:

s/.*$/&\\n/g

Hamish

[hellik]

in the grass64 delivered by the osgeo4w-stack "g.version -c" works

in the self compiled grass65-devel (rev39115) the$GISBASE/etc/VERSION is empty

[cnielsen]

cat ./../../COPYING | sed -f sed.script | tr -d '\012' > /src/dev_6/dist.i686-pc-mingw32/etc/VERSION
sed: file sed.script line 1: Unknown option to 's'

I might be missing something here but when I took the contents of sed.script and stuck them into the cmd itself with -e it works fine:

cat ./../../COPYING | sed -e 's/.*$/&\\n/g' | tr -d '\012' > /src/dev_6/dist
.i686-pc-mingw32/etc/VERSION

Is there a benefit to having it in a *.script instead?

-Colin

[cnielsen]

Spoke too soon, it fails if that change is made in the Makefile but not if run in the shell. Note the difference: When Makefile runs:

cat ./../../COPYING | sed -e 's/.*&\\n/g' | tr -d '\012' > /src/dev_6/dist.i686-pc-mingw32/etc/VERSION
sed: -e expression #2, char 10: Unterminated `s' command

Although in Makefile I wrote:

cat ./../../COPYING | sed -e 's/.*$/&\\n/g' | tr -d '\012' > $(GRASS_VERSION_FILE)

$/ is going missing from the middle of it. I'm not sure how to fix this though.

[hamish]

When Makefile runs:

cat ./../../COPYING | sed -f sed.script | tr -d '\012' > /src/dev_6/dist.i686-pc-mingw32/etc/VERSION
sed: file sed.script line 1: Unknown option to 's'

(sed.script contains s/.*$/&\\n/g )

so what is this Makefile line doing exactly?

• It's copying the COPYING file into the $GISBASE/etc/VERSION file
• it's terminating each line with a literal '\n'
• it's removing all actual newline chars from the file.

So this is getting the file ready to be a string embedded in the C code.

The g.version Makefile has it with comments:

# cat the COPYING file, add a c line-break \n at each line end
# and remove the unix newline.
COPYING=`cat ./../../COPYING | sed -f sed.script | tr -d '\012'`

...

EXTRA_CFLAGS=-DVERSION_NUMBER=\"'$(VERSION_NUMBER)'\"
  -DVERSION_DATE=\"'$(VERSION_DATE)'\"
  -DVERSION_UPDATE_PKG=\"'$(VERSION_UPDATE_PKG)'\"
  -DCOPYING="\"$(COPYING)\""

...

COPYING:
        cat ./../../COPYING | sed -f sed.script | tr -d '\012' > $(GRASS_VERSION_FILE)

GRASS_CONFIGURE_PARAMS:
        head -n 7 ./../../config.status | tail -n 1 | sed 's+#++1' | tr -d '\012' > $(GRASS_BUILD_FILE)

On linux 'strings' shows the COPYING text within the binary, from the native WinGrass Msys prompt it doesn't.

strings `which g.version`

so the COPYING: ... > VERSION make rule could be simplified into a straight cp?

interestingly the build info string does make it into the WinGrass binary and the $GISBASE/etc/BUILD file also has the correct content in it.

??, Hamish

[glynn]

Replying to hamish:

so what is this Makefile line doing exactly?

• It's copying the COPYING file into the $GISBASE/etc/VERSION file
• it's terminating each line with a literal '\n'
• it's removing all actual newline chars from the file.

So this is getting the file ready to be a string embedded in the C code.

Yep.

The g.version Makefile has it with comments:

> EXTRA_CFLAGS=...

>   -DCOPYING="\"$(COPYING)\""

On linux 'strings' shows the COPYING text within the binary, from the native WinGrass Msys prompt it doesn't.

I suspect that the -DCOPYING=... may run into problems with the maximum length of a command line on Windows.

It would be better to convert the COPYING file to a complete C source file (including the quotes) which can then be #include'd. E.g.:

EXTRA_CFLAGS=-DGRASS_VERSION_FILE="\"$(GRASS_VERSION_FILE)\"" ...

$(GRASS_VERSION_FILE): ../../COPYING
	sed -e 's/^.*$/"&\\n"/' $< > $@

and:

static const char COPYING[] =
#include GRASS_VERSION_FILE
;

so the COPYING: ... > VERSION make rule could be simplified into a straight cp?

No. The VERSION file has "\n" instead of newline.

Maybe it's provided as a convenience for add-on modules (nothing in the GRASS source tree uses it)?

interestingly the build info string does make it into the WinGrass binary and the $GISBASE/etc/BUILD file also has the correct content in it.

I note that -DGRASS_CONFIGURE_PARAMS=... comes first; maybe the command gets truncated at that point (does g.version have the complete build command?)

[hamish]

Getting back to this one..

Replying to glynn:

I note that -DGRASS_CONFIGURE_PARAMS=... comes first; maybe the command gets truncated at that point (does g.version have the complete build command?)

Yes. (ends --with-odbc, same as mswindows/osgeo4w/package.sh). g.version -b | wc -c # 624 bytes.

I did a test making a 5000 byte long string:

for i in `seq 1 500` ; do
  echo $i | awk '{printf("%0.10d", $1)}' >> count.txt
done

and pasted that after an echo command in both the cmd.exe and Msys unix prompts. That worked fine in both, so command line length probably isn't the culprit. (wc -c VERSION is 1509 chars)

The VERSION file has "\n" instead of newline.

Maybe it's provided as a convenience for add-on modules (nothing in the GRASS source tree uses it)?

It is there in the Gmakefile since GRASS 4.3, I suspect it is just a dead leaf which is still on the tree. If anyone can justify keeping it in $(ETC), perhaps it should be renamed to "ABOUT" with all the \n reinstated.

It would be better to convert the COPYING file to a complete C source file (including the quotes) which can then be #include'd.

still your diagnosis?

Hamish

[neteler]

Replying to hamish:

Replying to glynn:

...

It is there in the Gmakefile since GRASS 4.3, I suspect it is just a dead leaf which is still on the tree. If anyone can justify keeping it in $(ETC), perhaps it should be renamed to "ABOUT" with all the \n reinstated.

It would be better to convert the COPYING file to a complete C source file (including the quotes) which can then be #include'd.

... is that hard to do? Do it & proceed?

still your diagnosis?

Any suggestions here to get rid of this blocker?

thanks, Markus

[glynn]

Replying to neteler:

It would be better to convert the COPYING file to a complete C source file (including the quotes) which can then be #include'd.

... is that hard to do? Do it & proceed?

Done in r39842 for 7.0 (also for configure command).

The necessary changes for 6.x should be similar, but this probably can't be back-ported directly to 6.x due to Makefile differences.

[martinl]

Replying to glynn:

The necessary changes for 6.x should be similar, but this probably can't be back-ported directly to 6.x due to Makefile differences.

Backported to devbr6 in r39844. After some testing could be backported to relbr64.

[martinl]

Replying to martinl:

Replying to glynn:

The necessary changes for 6.x should be similar, but this probably can't be back-ported directly to 6.x due to Makefile differences.

Backported to devbr6 in r39844. After some testing could be backported to relbr64.

Done r39921, anyway compilation fails on my computer. The following patch helped me

Index: general/g.version/Makefile
===================================================================
--- general/g.version/Makefile  (revision 40022)
+++ general/g.version/Makefile  (working copy)
@@ -22,8 +22,8 @@
 
 default: cmd
 
-$(OBJDIR)/copying.h: $(MODULE_TOPDIR)/COPYING | $(OBJDIR)
+$(OBJDIR)/copying.h: $(MODULE_TOPDIR)/COPYING $(OBJDIR)
        sed -e 's/^\(.*\)$$/"\1\\n"/' $< > $@
 
-$(OBJDIR)/confparms.h: $(MODULE_TOPDIR)/config.status | $(OBJDIR)
+$(OBJDIR)/confparms.h: $(MODULE_TOPDIR)/config.status $(OBJDIR)
        sed -n '7s/^#\(.*\)$$/"\1"/p' $< > $@

Could someone review this patch? Thanks. Martin

[glynn]

Replying to martinl:

The necessary changes for 6.x should be similar, but this probably can't be back-ported directly to 6.x due to Makefile differences.

Done r39921, anyway compilation fails on my computer. The following patch helped me

-$(OBJDIR)/copying.h: $(MODULE_TOPDIR)/COPYING | $(OBJDIR)
+$(OBJDIR)/copying.h: $(MODULE_TOPDIR)/COPYING $(OBJDIR)
        sed -e 's/^\(.*\)$$/"\1\\n"/' $< > $@

Could someone review this patch? Thanks. Martin

You should probably omit $(OBJDIR) from the prerequisites list altogether, and forcibly create it before making $(OBJDIR)/copying.h and $(OBJDIR)/confpams.h, e.g.:

default:
	$(MAKE) $(OBJDIR)
	$(MAKE) cmd

Anything to the right of a "|" in the prerequisites line indicates an order-only prerequisite. These are ignored when checking if a target is older than any of its prerequisites, but if make decides that the target does need to be re-built, these will be made before the commands are executed if they don't already exist.

The 7.0 Makefiles generally use order-only prerequisites for the directory into which the target will be placed. They have to exist before the target can be made, but the target doesn't need to be re-made just because the directory has been updated.

The main issue is that they only work correctly with make 3.81. Although that's been out since April 2006, some people are still using older versions, so they aren't generally used in 6.x (they are used in a few places, e.g. Rules.make, conditional upon $(BROKEN_MAKE) being defined, with the alternative being to unconditionally create the directory within the rule's commands).

[martinl]

Replying to glynn:

You should probably omit $(OBJDIR) from the prerequisites list altogether, and forcibly create it before making $(OBJDIR)/copying.h and $(OBJDIR)/confpams.h, e.g.:

> default:
> 	$(MAKE) $(OBJDIR)
> 	$(MAKE) cmd

Done in r40060.

Anything to the right of a "|" in the prerequisites line indicates an order-only prerequisite. These are ignored when checking if a target is older than any of its prerequisites, but if make decides that the target does need to be re-built, these will be made before the commands are executed if they don't already exist.

The 7.0 Makefiles generally use order-only prerequisites for the directory into which the target will be placed. They have to exist before the target can be made, but the target doesn't need to be re-made just because the directory has been updated.

The main issue is that they only work correctly with make 3.81. Although that's been out since April 2006, some people are still using older versions, so they aren't generally used in 6.x (they are used in a few places, e.g. Rules.make, conditional upon $(BROKEN_MAKE) being defined, with the alternative being to unconditionally create the directory within the rule's commands).

Thanks for the explanation.

Martin

[martinl]

Tested on Windows, seems to work. I would suggest to close this ticket.

Martin

[hamish]

done & dusted.

thanks, Hamish