Re: [PATCH] maint.mk: detect_empty_lines_at_EOF_: avoid FP for an empty file

[Jim Meyering]

Jim Meyering wrote:
> FYI,
> I discovered (not in coreutils) that an empty version-controlled
> file would provoke a false-positive match from this new rule.
> But only when it is not the last file in the list.
>
>   $ :>empty; echo > f; echo g > g; tail -n1 empty f g
>   ==> empty <==
>
>   ==> f <==
>
>
>   ==> g <==
>   g
>
> This fixes it:
>
> Subject: [PATCH] maint.mk: detect_empty_lines_at_EOF_: avoid FP for an empty 
> file
...
> -  /^==> ([^\n]+) <==\n\n/m and (print "$$1\n"), $$fail = 1;          \
> +  /^==> ([^\n]+) <==\n\n\n/m and (print "$$1\n"), $$fail = 1;                
> \

Humph.  That change actually disabled the test.

Avoiding an FP for an empty file may be easy, but handling
that case would add enough complexity that I now prefer the more
direct -- and more efficient perl-only solution, in spite of the
fact that it is longer.

I've pushed this:


>From 01995a99d9e6f934f057fd37a55292c1dac05bdf Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 11 Apr 2010 13:35:33 +0200
Subject: [PATCH] maint.mk: improve empty-line-at-EOF check

* top/maint.mk (sc_prohibit_empty_lines_at_EOF): Use Perl-based
solution, rather than tail+Perl-based one.  The latter would read
a few kilobytes from the end of each file, and did not handle empty
files properly.
---
 ChangeLog    |    6 ++++++
 top/maint.mk |   33 +++++++++++++++++++++++----------
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 688f8c0..89f557e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2010-04-11  Jim Meyering  <address@hidden>

+       maint.mk: improve empty-line-at-EOF check
+       * top/maint.mk (sc_prohibit_empty_lines_at_EOF): Use Perl-based
+       solution, rather than tail+Perl-based one.  The latter would read
+       a few kilobytes from the end of each file, and did not handle empty
+       files properly.
+
        maint.mk: print the elapsed time for each syntax-check rule
        * top/maint.mk (sc_m_rules_): Save start time in a file.
        (sc_z_rules_): New rules: remove temp file and print elapsed time.
diff --git a/top/maint.mk b/top/maint.mk
index bc62fa3..bc8a581 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -674,27 +674,40 @@ sc_prohibit_cvs_keyword:
        halt='do not use CVS keyword expansion'                         \
          $(_sc_search_regexp)

-# The following tail+perl pipeline would be more concise, and would
-# produce slightly better output (including counts) if written as
+ include $(srcdir)/dist-check.mk
+
+# This Perl code is slightly obfuscated.  Not only is each "$" doubled
+# because it's in a Makefile, but the $$c's are comments;  we cannot
+# use "#" due to the way the script ends up concatenated onto one line.
+# It would be much more concise, and would produce better output (including
+# counts) if written as:
 #   perl -ln -0777 -e '/\n(\n+)$/ and print "$ARGV: ".length $1' ...
 # but that would be far less efficient, reading the entire contents
-# of each file, rather than just the last few bytes of each.
+# of each file, rather than just the last two bytes of each.
 #
-# This is a perl script that operates on the output of
-# tail -n1 TWO_OR_MORE_FILES
+# This is a perl script that is expected to be the single-quoted argument
+# to a command-line "-le".  The remaining arguments are file names.
 # Print the name of each file that ends in two or more newline bytes.
 # Exit nonzero if at least one such file is found, otherwise, exit 0.
+# Warn about, but otherwise ignore open failure.  Ignore seek/read failure.
 #
 # Use this if you want to remove trailing empty lines from selected files:
 #   perl -pi -0777 -e 's/\n\n+$/\n/' files...
 #
 detect_empty_lines_at_EOF_ =                                           \
-  /^==> ([^\n]+) <==\n\n\n/m and (print "$$1\n"), $$fail = 1;          \
-  END { exit defined $$fail }
+  foreach my $$f (@ARGV) {                                             \
+    open F, "<", $$f or (warn "failed to open $$f: $$!\n"), next;      \
+    my $$p = sysseek (F, -2, 2);                                       \
+    my $$c = "seek failure probably means file has < 2 bytes; ignore"; \
+    my $$two;                                                          \
+    defined $$p and $$p = sysread F, $$two, 2;                         \
+    close F;                                                           \
+    $$c = "ignore read failure";                                       \
+    $$p && $$two eq "\n\n" and (print $$f), $$fail=1;                  \
+    } END { exit defined $$fail }
 sc_prohibit_empty_lines_at_EOF:
-       @tail -n1 $$($(VC_LIST_EXCEPT)) /dev/null                       \
-           | perl -00 -ne '$(detect_empty_lines_at_EOF_)'              \
-         || { echo '$(ME): the above files end with empty line(s)'     \
+       @perl -le '$(detect_empty_lines_at_EOF_)' $$($(VC_LIST_EXCEPT)) \
+          || { echo '$(ME): the above files end with empty line(s)'     \
                1>&2; exit 1; } || :;                                   \

 # Make sure we don't use st_blocks.  Use ST_NBLOCKS instead.
--
1.7.1.rc0.264.g94f6e