newline problem with genVersionHeader.pl

[Dirk Zimoch]

After merging the current EPICS 7.0 branch into my PSI branch, I got a generated epicsVCS.h header file with invalid syntax on RHEL7 (but not on RHEL8):

#ifndef EPICS_VCS_VERSION_DATE
  #define EPICS_VCS_VERSION_DATE "Git: 2023-03-29 13:36:52 +0200
"

Note the newline within the string.

I tracked that down to a strange behavior of git show in git version 1.8.3.1 as installed by default on RHEL7:

Whenever the current commit is a merge, then the output of 'git show -s --format=%ci' (as used by genVersionHeader.pl) shows an extra newline!

Piping it to hexdump show the issue:
$git co 7.0
git show -s --format=%ci | hexdump -C
00000000 32 30 32 33 2d 30 33 2d 31 30 20 31 32 3a 30 32 |2023-03-10 12:02|
00000010 3a 32 39 20 2b 30 30 30 30 0a |:29 +0000.|
0000001a
Just as it should be.

Now check out a merge commit:
$git co e63184e
[...]
HEAD is now at e63184e... Merge PR #63, longout.OOPT
$ git show -s --format=%ci | hexdump -C
00000000 32 30 32 32 2d 31 32 2d 32 39 20 31 36 3a 33 31 |2022-12-29 16:31|
00000010 3a 34 32 20 2d 30 36 30 30 0a 0a |:42 -0600..|
0000001b

Note the extra 0a!

This extra newline gets pasted into epicsVCS.h.

BTW: Git version 2.31.1 as installed by default on RHEL8 does not show this bug.

Of course one could blame the problem on the old git version, but I think just to be on the safe side, it would be better to remove all terminating newlines (and returns, maybe all whitespace) and not only the last one, as perl seems to do.

[Dirk Zimoch (dirk.zimoch)]

PS: The bug in git show can be reproduced even without checking out the commit:
git show -s --format=%ci e63184e
The extra newline does not depend on the format. Even plain 'git show COMMIT' shows one newline too many whenever the COMMIT is a merge.

[Andrew Johnson (anj)]

The genVersionHeader.pl script already has
    chomp $cv;
on line 139 to remove one trailing newline, does adding another copy after line 98 (the "$cv = `git show -s …" line) resolve this?

[mdavidsaver (mdavidsaver)]

> chomp $cv;
> $RevDate=$vcs . ': ' . $cv;
...
> #define ${opt_N}_DATE \"$RevDate\"

I'm not sure I would even describe this as a bug with Git. And I'm puzzled why "chomp" doesn't remove both newlines.

https://perldoc.perl.org/functions/chomp

> ... When in paragraph mode ($/ = ''), it removes all trailing newlines from the string. ...

[Dirk Zimoch (dirk.zimoch)]

> does adding another copy after line 98 (the "$cv = `git show -s …" line) resolve this?
Yes.

[Dirk Zimoch (dirk.zimoch)]

With no 'chomp' in neither line 98 nor 139, I get both newlines in the output.
So it seems that 'chomp' removes only one newline.
My perl version is v5.16.3, in case it may be relevant.

[Dirk Zimoch (dirk.zimoch)]

If I set $/='' as the doc suggests, it removes both newlines.

[Dirk Zimoch (dirk.zimoch)]

$/ is newline by default
https://perldoc.perl.org/perlvar#%24%2F
That makes chomp remove only one newline.

[Andrew Johnson (anj)]

> I'm puzzled why "chomp" doesn't remove both newlines.

Because Perl's $INPUT_RECORD_SEPARATOR = $/ defaults to a newline (line mode). "perldoc perlvar" and search for INPUT_RECORD_SEPARATOR to learn all about it.

Setting $/ to '' should be okay, but I would probably localize it like this (or at least don't do it before the chomp) in case changing it globally has any negative effects on input parsing elsewhere in the code:

{
  local $/; # Temporarily set to ''
  chomp $cv; # Remove all newlines
}

[mdavidsaver (mdavidsaver)]

I had just started to read about "$/". How... complicated.

https://perldoc.perl.org/perlvar#%24%2F

[Andrew Johnson (anj)]

https://en.wikipedia.org/wiki/Waterbed_theory

[Dirk Zimoch (dirk.zimoch)]

I have added $/=''; at the beginning, directly after use strict;
No problem found. The file looks good. All command line options work as before. Can create new files and update existing ones.

[Dirk Zimoch (dirk.zimoch)]

Created PR https://github.com/epics-base/epics-base/pull/371

[Andrew Johnson (anj)]

> No problem found

What about testing the code sections that run for the other version control systems?

This script was designed to work with EPICS applications (not just Base itself) that are stored in Darcs, Mercurial, Git, Subversion or Bazaar, and each of those has a separate section of the script for obtaining version information from the VCS which could be affected by that change. The Darcs code at least appears to handle multi-line results. I'm not saying that there _is_ a problem, but this kind of change could very easily break one of the others unless you localize the $/ variable as I showed above. If you don't do that, they ought to be tested too.

[Dirk Zimoch (dirk.zimoch)]

Someone should add tests for those things. ;-)

[Dirk Zimoch (dirk.zimoch)]

Tested with CVS, Mercurial, Subversion and Bazaar on existing repositories I have access to. There is no difference in behavior with or without $/=''. The generated file looks the same.
Unfortunately, I have no Darcs repository to test with and I am not sure if generating a trivial one is enough for the test.

One interesting observation though: If the tool for the repository type is not installed, the VCSVERSION_DATE string simply looks like this:
  #define VCSVERSION_DATE ": "

[Ralph Lange (ralph-lange)]

I can test with Darcs if I know what to test...

[Dirk Zimoch (dirk.zimoch)]

My test was to generate a version header file with an unmodified version of genVersionHeader.pl and compare it with a header file generated with a version of genVersionHeader.pl which has $/=''; at the beginning and to check that the two files are identical.

I did something like this:
cd <some repo>
$EPICS_BASE/bin/$EPICS_HOST_ARCH/genVersionHeader.pl -t . v1.h

This generates a file 'v1.h'. Then I do the modification and generate a file v2.h and compare the two. I did this for several repositories of each type.