Re: epub generation depends on perl Archive::ZIP module

[Gavin Smith]

On Sun, Jul 31, 2022 at 09:22:01AM +0200, Patrice Dumas wrote:
> Leaving things as they are now, there is an error mesage like that,
> which is not very user friendly:
> 
> ./texi2any.pl --epub3 test_config.texi 
> Can't locate Archive/Zip.pm in @INC (you may need to install the Archive::Zip 
> module) (@INC contains: ../tp/maintain/lib/Text-Unidecode/lib 
> ../tp/maintain/lib/Unicode-EastAsianWidth/lib 
> ../tp/maintain/lib/libintl-perl/lib ../tp/Texinfo/XS/parsetexi 
> ../tp/Texinfo/XS ../tp . 
> /home/dumas/perl5/lib/perl5//5.34.0/x86_64-linux-gnu-thread-multi 
> /home/dumas/perl5/lib/perl5//5.34.0 
> /home/dumas/perl5/lib/perl5//x86_64-linux-gnu-thread-multi 
> /home/dumas/perl5/lib/perl5/ 
> /home/dumas/src/mini_ker_install/share/perl/5.32.1/ /etc/perl 
> /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 
> /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 
> /usr/lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 
> /usr/share/perl/5.34 /usr/local/lib/site_perl) at ../tp/ext/epub3.pm line 714.
> 
> Any idea?  There is probably a way to use an eval block or something
> that tests that the require Archive::Zip works and another message can
> be used instead, but I am not sure that it is really so much better.

I have been trying to find a good way to return an error when
loading a module.  I tried calling "die" from inside the module
with an error message, and catching this error message inside the
loading code as $@.  The problem was I couldn't suppress error
messages completely, leading to extraneous error messages being
output from perl.

It seems that perl modules are not supposed to fail at the "require"
stage.  The epub3.pm module should be loaded "successfully", but then
we have to get the information that it shouldn't used.  I tried
two approaches:

* Call a function in the module after it is loaded.  This is difficult
to do because it is loaded in the Texinfo::Config namespace.

* Call functions in the module as it is being loaded to register
errors.

A patch for the second approach is at the end of this email.  I'm
sure you'll have ideas for how this could be improved.

It would be better if we could avoid loading the rest of the module
once the problem is detected, although I don't know if this is
possible without calling "die".

Unlike the current code, this makes Archive::Zip a hard requirement
for EPUB output - it does not allow creating the unzipped folder.  This
could easily be changed, I expect.

A separate issue, but is it worth outputting the unzipped folder at
all?  Wouldn't it be cleaner to delete the unzipped folder when
done and just leave the zipped EPUB file?

Test it by changing "require Archive::Zip" to "require Archive::Zipx" or
some other module that doesn't exist.

diff --git a/tp/Texinfo/Config.pm b/tp/Texinfo/Config.pm
index afe4dd81f2..b068166c77 100644
--- a/tp/Texinfo/Config.pm
+++ b/tp/Texinfo/Config.pm
@@ -119,6 +119,12 @@ sub _GNUT_document_warn($) {
                    "%s: warning: %s\n"), $real_command_name, $text)));
 }
 
+my @module_errors;
+sub GNUT_config_failed($) {
+  my $error = shift;
+  push @module_errors, $error;
+}
+
 # called from texi2any.pl main program.
 # eval $FILE in the Texinfo::Config namespace. $FILE should be a binary string.
 sub GNUT_load_init_file($) {
@@ -133,6 +139,13 @@ sub GNUT_load_init_file($) {
     _GNUT_document_warn(sprintf(__("error loading %s: %s"),
                 _GNUT_decode_input($file), $e));
   }
+  for my $error (@module_errors) {
+    warn sprintf(__("error loading %s: %s"),
+                _GNUT_decode_input($file), $error)."\n";
+  }
+  if (@module_errors) {
+    exit 1;
+  }
 }
 
 # L2H removed in 2021
diff --git a/tp/ext/epub3.pm b/tp/ext/epub3.pm
index 0767c7daea..0e61fce15d 100644
--- a/tp/ext/epub3.pm
+++ b/tp/ext/epub3.pm
@@ -78,6 +78,12 @@ use Texinfo::Common;
 use Texinfo::Convert::Utils;
 use Texinfo::Convert::Text;
 
+eval { require Archive::Zip; };
+
+if ($@) {
+  GNUT_config_failed("Archive::Zip is required for EPUB output");
+}
+
 # the 3.2 spec was used for the implementation.  However, it seems to be
 # designed to be backward compatible with 3.0 and mandates to use 3.0 as
 # version.
@@ -830,8 +836,6 @@ EOT
   }
 
   if ($self->get_conf('EPUB_CREATE_CONTAINER_FILE')) {
-    require Archive::Zip;
-
     # this is needed if there are non ascii file names, otherwise, for instance
     # with calibre the files cannot be read, one get
     # "There is no item named 'EPUB/osé.opf' in the archive"