[GRASS-dev] [GRASS GIS] #595: WinGRASS g.version -c fails

GRASS GRASS developer
1

#595: WinGRASS g.version -c fails
--------------------------+-------------------------------------------------
Reporter: hamish | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: critical | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Keywords: wingrass gpl | Platform: MSWindows XP
      Cpu: x86-32 |
--------------------------+-------------------------------------------------
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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/595&gt;
GRASS GIS <http://grass.osgeo.org>

2

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Changes (by martinl):

  * priority: critical => blocker

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/595#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

3

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/595#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

4

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/595#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

5

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by 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.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/595#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

6

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/595#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

7

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by glynn):

Replying to [comment:5 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?)

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/595#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>

8

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by neteler):

Replying to [comment:7 hamish]:
> Replying to [comment:6 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/595#comment:8&gt;
GRASS GIS <http://grass.osgeo.org>

9

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by glynn):

Replying to [comment:8 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.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/595#comment:9&gt;
GRASS GIS <http://grass.osgeo.org>

10

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by martinl):

Replying to [comment:9 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.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/595#comment:10&gt;
GRASS GIS <http://grass.osgeo.org>

11

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by martinl):

Replying to [comment:10 martinl]:
> Replying to [comment:9 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/595#comment:11&gt;
GRASS GIS <http://grass.osgeo.org>

12

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by glynn):

Replying to [comment:11 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).

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/595#comment:12&gt;
GRASS GIS <http://grass.osgeo.org>

13

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by martinl):

Replying to [comment:12 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/595#comment:13&gt;
GRASS GIS <http://grass.osgeo.org>

14

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Comment (by martinl):

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

Martin

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/595#comment:14&gt;
GRASS GIS <http://grass.osgeo.org>

15

#595: WinGRASS g.version -c fails
---------------------------+------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: closed
  Priority: blocker | Milestone: 6.4.0
Component: License | Version: 6.4.0 RCs
Resolution: fixed | Keywords: wingrass gpl
  Platform: MSWindows XP | Cpu: x86-32
---------------------------+------------------------------------------------
Changes (by hamish):

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

Comment:

done & dusted.

thanks,
Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/595#comment:15&gt;
GRASS GIS <http://grass.osgeo.org>