Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

detect errors from make regen #73

Open
atoomic opened this issue May 16, 2019 · 11 comments
Open

detect errors from make regen #73

atoomic opened this issue May 16, 2019 · 11 comments

Comments

@atoomic
Copy link
Member

atoomic commented May 16, 2019

we want to detect these errors from travis
idea: redirect stdout and stderr to different files
display them
and have a unit test checking that the file does not exist or is empty

> make regen >/dev/null
mysterious name [ENTER_with_name(name)] in parts/apidoc.fnc, line 161
mysterious name [LEAVE_with_name(name)] in parts/apidoc.fnc, line 164
parts/inc/misc provides av_tindex, which is still marked todo for 5.17.9
parts/inc/misc provides av_top_index, which is still marked todo for 5.17.9
parts/inc/misc provides isALPHANUMERIC, which is still marked todo for 5.17.8
parts/inc/misc provides isIDCONT, which is still marked todo for 5.17.8
parts/inc/misc provides isOCTAL, which is still marked todo for 5.13.5
parts/inc/misc provides isWORDCHAR, which is still marked todo for 5.13.6
parts/inc/uv provides utf8_to_uvchr_buf, which is still marked todo for 5.15.9
parts/inc/mess provides vmess, which is still marked todo for 5.6.0

#61 is for example fixing the vmess warning

copy: @pali

@pali
Copy link
Contributor

pali commented May 16, 2019

Not so clean, but oneline solutiion:

( make regen || echo 'make regen failed' >&2 ) 3>&2 2>&1 1>&3- | ( grep --color '.' && exit 1 || exit 0 )

Returns zero iff make regen returns zero && make regen have not written anything to stderr.

@pali
Copy link
Contributor

pali commented May 16, 2019

Note that both stdout and stderr is written to terminal, so no output is lost.

@pali
Copy link
Contributor

pali commented May 17, 2019

Also you should run make regen as:

DPPP_CHECK_LEVEL=2 make regen

It will do more checks...
(PS: You have typo in issue subject regex vs regen)

@pali
Copy link
Contributor

pali commented May 17, 2019

Maybe easier would be to change all warn and print STDERR to die when some env variable like DPPP_CHECK_FOR_ERRORS is set. So in travis you could just call:
DPPP_CHECK_LEVEL=2 DPPP_CHECK_FOR_ERRORS=1 make regen

@pali
Copy link
Contributor

pali commented May 17, 2019

And here is simple implementation for DPPP_CHECK_LEVEL=2 DPPP_CHECK_FOR_ERRORS=1 make regen:

diff --git a/PPPort_pm.PL b/PPPort_pm.PL
index 19d5985..d3b16f9 100644
--- a/PPPort_pm.PL
+++ b/PPPort_pm.PL
@@ -65,8 +65,8 @@ for (keys %raw_todo) {
 for (@api) {
   if (exists $raw_todo{$_} and exists $raw_base{$_}) {
     if ($raw_base{$_} eq $raw_todo{$_}) {
-      warn "$INCLUDE/$provides{$_} provides $_, which is still marked "
-           . "todo for " . format_version($raw_todo{$_}) . "\n";
+      check(0, "$INCLUDE/$provides{$_} provides $_, which is still marked "
+           . "todo for " . format_version($raw_todo{$_}));
     }
     else {
       check(2, "$_ was ported back to " . format_version($raw_todo{$_}) .
@@ -140,7 +140,7 @@ sub include
   for (@{$data->{provides}}) {
     if (exists $provides{$_}) {
       if ($provides{$_} ne $file) {
-        warn "$file: $_ already provided by $provides{$_}\n";
+        check(0, "$file: $_ already provided by $provides{$_}");
       }
     }
     else {
@@ -279,7 +279,7 @@ sub expand_pp_expr
       }
     }
     else {
-      warn "found no prototype for $func\n";;
+      check(0, "found no prototype for $func");
     }
 
     $explicit{$func} = 'func';
@@ -347,8 +347,12 @@ sub check
 {
   my $level = shift;
 
-  if (exists $ENV{DPPP_CHECK_LEVEL} and $ENV{DPPP_CHECK_LEVEL} >= $level) {
-    print STDERR @_, "\n";
+  if ($level == 0 or (exists $ENV{DPPP_CHECK_LEVEL} and $ENV{DPPP_CHECK_LEVEL} >= $level)) {
+    if ($ENV{DPPP_CHECK_FOR_ERRORS}) {
+      die @_, "\n";
+    } else {
+      warn @_, "\n";
+    }
   }
 }
 

@pali pali mentioned this issue Jul 13, 2019
@pali
Copy link
Contributor

pali commented Oct 4, 2019

@atoomic see ↑↑↑

@atoomic
Copy link
Member Author

atoomic commented Oct 8, 2019

Thanks @pali

@atoomic
Copy link
Member Author

atoomic commented Oct 8, 2019

after applying @pali patch via 183e448

> DPPP_CHECK_LEVEL=2 DPPP_CHECK_FOR_ERRORS=1 make regen

differing prototypes for newCONSTSUB:
  API: CV * newCONSTSUB(pTHX_ HV * stash, const char * name, SV * sv)
  PPP: void newCONSTSUB(HV * stash, const char * name, SV * sv)
make: *** [regen_pm] Error 255

@atoomic atoomic changed the title detect errors from make regex detect errors from make regen Oct 8, 2019
atoomic added a commit to atoomic/Devel-PPPort that referenced this issue Oct 8, 2019
@pali
Copy link
Contributor

pali commented Oct 8, 2019

Maybe this patch could help?

diff --git a/parts/inc/newCONSTSUB b/parts/inc/newCONSTSUB
index f5b744d..7624ccd 100644
--- a/parts/inc/newCONSTSUB
+++ b/parts/inc/newCONSTSUB
@@ -19,11 +19,6 @@ NEED_newCONSTSUB    /* Because we define this weirdly */
 
 =implementation
 
-/* Hint: newCONSTSUB
- * Returns a CV* as of perl-5.7.1. This return value is not supported
- * by Devel::PPPort.
- */
-
 /* newCONSTSUB from IO.xs is in the core starting with 5.004_63 */
 #if { VERSION < 5.004_63 } && { VERSION != 5.004_05 }
 
@@ -37,9 +32,10 @@ NEED_newCONSTSUB    /* Because we define this weirdly */
 /* (There's no PL_parser in perl < 5.005, so this is completely safe)     */
 #define D_PPP_PL_copline PL_copline
 
-void
+CV *
 newCONSTSUB(HV *stash, const char *name, SV *sv)
 {
+        CV *cv;
         U32 oldhints = PL_hints;
         HV *old_cop_stash = PL_curcop->cop_stash;
         HV *old_curstash = PL_curstash;
@@ -50,7 +46,7 @@ newCONSTSUB(HV *stash, const char *name, SV *sv)
         if (stash)
                 PL_curstash = PL_curcop->cop_stash = stash;
 
-        newSUB(
+        cv = newSUB(
 
                 start_subparse(FALSE, 0),
 
@@ -63,6 +59,8 @@ newCONSTSUB(HV *stash, const char *name, SV *sv)
         PL_curcop->cop_stash = old_cop_stash;
         PL_curstash = old_curstash;
         PL_curcop->cop_line = oldline;
+
+        return cv;
 }
 #endif
 #endif

But it is needed to test it for old Perl versions...

@atoomic
Copy link
Member Author

atoomic commented Oct 8, 2019

@pali applies as part of d7e7ca4
next error is now
No API definition for provided element CopFILE found.

@pali
Copy link
Contributor

pali commented Oct 8, 2019

So all errors needs to be fixed before DPPP_CHECK_FOR_ERRORS=1 is enabled. You can run check with DPPP_CHECK_FOR_ERRORS=0 to just throw warnings instead of die.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants