bugGNU roff - Bugs: bug #63824, revert recent change to skip tests...

 
 

bug #63824: revert recent change to skip tests at configuration time

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Tue 21 Feb 2023 08:05:22 AM UTC
   
 
Category:  General Severity:  5 - Blocker
Item Group:  Test Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 23 Feb 2023 04:14:17 PM UTC, comment #4: 


commit 5486b5bc09f394d2697e32e2f2b887eea93f4b14
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Feb 21 16:43:10 2023 -0600

    [grohtml]: Fix Savannah #63824 (1/2).

    [grohtml]: Revise tests to check for requisite programs at test time and
    skip if not found, rather than configuring them away at build time,
    which can cause a distribution archive to be incorrectly structured.

    * m4/groff.m4: Annotate requisite program list since we're violating the
      DRY principle.

    * src/roff/groff/groff.am (groff_TESTS): Populate unconditionally.

    * src/roff/groff/tests/html_works_with_grn_and_eqn.sh:
    * src/roff/groff/tests/smoke-test_html_device.sh: Check for requisite
      programs and skip test if any are not found.

    Fixes <https://savannah.gnu.org/bugs/?63824> (1/2).

commit 2706c735b3c89411d2bcf6e8c31ec8796feab174
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Feb 21 18:57:58 2023 -0600

    [gropdf]: Fix Savannah #63824 (2/2).

    [gropdf]: Revise tests to run unconditionally, rather than configuring
    them away at build time, which can cause a distribution archive to be
    incorrectly structured.  Update configuration notices when optional
    dependencies are absent.

    * font/devpdf/devpdf.am (font_devpdf_TESTS): Remove `USE_GROPDF` and
      `HAVE_URW_FONTS` conditionals.

    * m4/groff.m4 (GROFF_GROPDF_PROGRAM_NOTICE, GROFF_URW_FONTS_NOTICE):
      Warn reader that a gropdf test failure is to be expected.

    Fixes <https://savannah.gnu.org/bugs/?63824> (2/2).


G. Branden Robinson <gbranden>
Group administrator
Tue 21 Feb 2023 11:22:04 PM UTC, comment #3: 

comment #2:

> Have you looked at the --check flag of BuildFoundries?


Somewhat.

> It is intended to report whether BuildFoundries will succeed in populating the download file in devpdf. However, there is a chicken/egg problem, in order to be called from configure it needs to be "made" i.e. populate @PERL@, @GROFF_GHOSTSCRIPT_INTERPRETERS@ and @PATH_SEPARATOR@ with sed or whatever does it and made executable. I don't know if it is possible.


It should be possible, I think: Autoconf tests construct C source files based on Autoconf-determined parameters and compile them to determine the values of other Autoconf parameters all the time.

An Autoconf check that AC_REQUIREs GROFF_AWK_PATH can be sure that $AWK is populated (if missing, it won't run at all).  Similarly, one that AC_REQUIREs GROFF_GHOSTSCRIPT_PATH can be sure that $GHOSTSCRIPT is populated (or the check won't be run).

(I am not sure it is necessary to mess with @GROFF_GHOSTSCRIPT_INTERPRETERS@...we should already know what name is being used for Ghostscript by that time, and not have to search.)

So I think such a check we write can be fully and correctly parameterized at ./configure-time.

However I think I want to leave that adventure for groff 1.23++.  Too many times when I think something will be straightforward, I end up with canid tooth marks in my gluteus maxima.

Here is a diff of my changes in preparation to proceed as discussed in comment #0 for gropdf.  I revised the texts of the notice as discussed and to otherwise bring them up to date with facts.


diff --git a/font/devpdf/devpdf.am b/font/devpdf/devpdf.am
index ddf34e625..4b872182d 100644
--- a/font/devpdf/devpdf.am
+++ b/font/devpdf/devpdf.am
@@ -178,12 +178,9 @@ uninstall_devpdf:
          rmdir $(DESTDIR)$(devpdffontdir); \
        fi

-if USE_GROPDF
-font_devpdf_TESTS = font/devpdf/tests/check-default-foundry.sh
-if HAVE_URW_FONTS
-font_devpdf_TESTS += font/devpdf/tests/check-urw-foundry.sh
-endif
-endif
+font_devpdf_TESTS = \
+  font/devpdf/tests/check-default-foundry.sh \
+  font/devpdf/tests/check-urw-foundry.sh

 TESTS += $(font_devpdf_TESTS)
 EXTRA_DIST += $(font_devpdf_TESTS)
diff --git a/m4/groff.m4 b/m4/groff.m4
index af2ddfdcd..b6c5535be 100644
--- a/m4/groff.m4
+++ b/m4/groff.m4
@@ -325,13 +325,15 @@ AC_DEFUN([GROFF_GROPDF_PROGRAM_NOTICE], [
   then
     AC_MSG_NOTICE(['gropdf' will have reduced function.

-  Because neither Ghostscript nor URW fonts are available, groff
-  documentation will not be available in PDF.
+  Neither Ghostscript nor URW fonts are available; groff documentation
+  thus will not be available in PDF.

   'gropdf' will be able to handle only documents using the standard PDF
   base 14 fonts, plus the 'EURO' font groff supplies, and font embedding
   with its '-e' option (accessed via the 'groff' command with the option
   '-P -e') will not be possible.
+
+  If you run "make check", an automated test for 'gropdf' will fail.
 ])
   fi
 ])
@@ -363,6 +365,7 @@ AC_DEFUN([GROFF_URW_FONTS_CHECK], [
     _list_paths=`$GHOSTSCRIPT -h | $AWK 'BEGIN { found = 0 } /Search path:/ { found = 1 } /^[ ]*\// { print $'0' }'| tr : ' '`
   fi

+dnl Keep this list in sync with font/devpdf/Foundry.in.
   _list_paths="$_list_paths \
     /usr/share/fonts/type1/gsfonts/ \
     /usr/share/fonts/default/Type1/ \
@@ -409,12 +412,16 @@ AC_DEFUN([GROFF_URW_FONTS_NOTICE], [
   then
     AC_MSG_NOTICE([URW fonts in Type 1/PFB format were not found.

-  URW font generation for groff's 'gropdf' output driver will not work
-  properly.  You can obtain the URW base 35 fonts from their GitHub
-  project.
+  groff font description files for the URW fonts, used by the 'gropdf'
+  output driver, will not be available.  Use and embedding of fonts from
+  the 'U' foundry in PDF documents generated by groff will not be
+  possible.
+
+  If you run "make check", an automated test for 'gropdf' will fail.

-  As of this writing (2023-02-15), you can find them in the 'fonts'
-  directory of the following archives (choose one).
+  You can obtain the URW base 35 fonts from their GitHub project.  As of
+  this writing (2023-02-15), you can find them in the 'fonts' directory
+  of the following archives (choose one).

     https://github.com/ArtifexSoftware/urw-base35-fonts/archive/refs/
       tags/20200910.zip
@@ -426,12 +433,10 @@ AC_DEFUN([GROFF_URW_FONTS_NOTICE], [
     https://github.com/ArtifexSoftware/urw-base35-fonts/releases

   By default, groff will look for these fonts in the search path shown
-  by the 'gs -h' command (if available) and in the two directories
-    /usr/share/fonts/type1/gsfonts/
-  and
-    /opt/local/share/fonts/urw-fonts/
-  (these locations are specified in font/devpdf/Foundry.in).  You will
-  need to re-run the 'configure' script after installing these fonts.
+  by the 'gs -h' command (if available) and in several directories
+  specified in font/devpdf/Foundry.in).  You will need to 'make
+  distclean' and re-run the 'configure' script after installing these
+  fonts.

   Alternatively, you can pass the option '--with-urw-fonts-dir=DIR'
   to 'configure' to look for them in the directory DIR you specify.


Please comment if you have time before I push.  (No idea when that will be.)

G. Branden Robinson <gbranden>
Group administrator
Tue 21 Feb 2023 06:03:13 PM UTC, comment #2: 

original submission:

> The gropdf situation is harder.  The thing that can scotch its tests is not missing commands but missing fonts.  There's no easy way to tell from within a test script whether gropdf is going to find Type 1 fonts it can embed in the documents it generates.  (Locating them is a complex process.)  And Deri are still wrangling with testing issues anyway (see bug #63808).
>
> In the long run (groff 1.23.1?), maybe what we want to do is something Deri wanted to do a long time ago, which is bust out part of BuildFoundries.pl as a script that is runnable at configuration time (we depend on Perl anyway).  That script can be the snout that sniffs out the presence of embeddable Type 1 fonts on the system.  Not only can we set shell variables that will inform the user whether gropdf's functionality will be attenuated (as we already do), but we can create artifacts in the build directory (at build time) that the gropdf scripts can look for.  (We already have an Automake variable, `HAVE_URW_FONTS`, that is produced by `AM_CONDITIONAL` in "configure.ac".  Perhaps what we want is more like `HAVE_EMBEDDABLE_DEFAULT_FOUNDRY` and `HAVE_EMBEDDABLE_URW_FOUNDRY`.)

Have you looked at the --check flag of BuildFoundries? It is intended to report whether BuildFoundries will succeed in populating the download file in devpdf. However, there is a chicken/egg problem, in order to be called from configure it needs to be "made" i.e. populate @PERL@, @GROFF_GHOSTSCRIPT_INTERPRETERS@ and @PATH_SEPARATOR@ with sed or whatever does it and made executable. I don't know if it is possible.

Currently it returns non-zero if a problem is found, but it would be more useful to change it to return an integer 0..3 corresponding to the scenario discovered. It could remove some duplicated tests in configure.

> The gropdf test scripts could then simply look for these artifacts in the build tree, and continue or skip themselves as appropriate.
>
> But for groff 1.23.0, it might make more sense to just run the tests every time and let them fail as we warned in ./configure output if the host environment isn't ship-shape in the embeddable fonts department.
>
> None of this would prevent a build from being deployed elsewhere, or Ghostscript and/or URW packages from being removed after a groff build, so gropdf will still need to re-verify the availability of fonts to be embedded every time it runs anyway.  And it does.  (Another thing Deri and I have wrangled over is the wording of the diagnostics that happen in this scenario.  :) )
>
> Maybe it doesn't even make too much sense to worry about users being alarmed by test failures here.  Everybody's heard of "./configure && make && make install".  Few seem to bother with having "make check" in there... :-/

Deri James <deri>
Group Member
Tue 21 Feb 2023 08:06:38 AM UTC, comment #1: 

Soliciting comment from Deri; setting planned release.

G. Branden Robinson <gbranden>
Group administrator
Tue 21 Feb 2023 08:05:22 AM UTC, original submission:  

A couple of recent changes were ill-conceived.


commit 668a1b183b748473e2fec17bdde4db286129f659
Author:     G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Thu Feb 16 12:14:58 2023 -0600
Commit:     G. Branden Robinson <g.branden.robinson@gmail.com>
CommitDate: Fri Feb 17 06:12:11 2023 -0600

    [tests]: Don't test grohtml if unsupported.

    * src/roff/groff/groff.am (groff_TESTS) [!USE_GROHTML]: Don't test the
      'grohtml' driver if we know it won't work.

commit e3954e5b15ae956c2ee7f98c8f323bfa9172e5a3
Author:     G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Sat Feb 18 02:04:00 2023 -0600
Commit:     G. Branden Robinson <g.branden.robinson@gmail.com>
CommitDate: Sat Feb 18 02:05:00 2023 -0600

    [gropdf]: Don't test if hobbled.

    [gropdf]: Don't run automated tests if 'gropdf' will be operating with
    reduced function.  If neither Ghostscript nor the URW fonts are
    available at configuration time, there is no point testing for
    successful build-time population of the font descriptions for the
    default and URW foundries.

    * font/devpdf/devpdf.am (font_devpdf_TESTS) [USE_GROPDF]: Run
      "check-default-foundry" only if gropdf is fully functional.

      (font_devpdf_TESTS) [USE_GROPDF && HAVE_URW_FONTS]: Run
      "check-urw-foundry" only if the URW fonts were found.

    Continues <https://savannah.gnu.org/bugs/?63808>.  Thanks to Deri James
    for the continued discussion.


The problem with these is some other lines in the *.am files:


EXTRA_DIST += $(font_devpdf_TESTS)

EXTRA_DIST += $(groff_TESTS)


These mean that the distribution archive's contents will depend on the configuration of the system from which "make dist" was run.

That is bad, bad, bad.

These `USE_GROHTML` and `USE_GROPDF` bracketings should be undone.

However, the tests are being skipped for a good reason.  Instead of not executing them, it would probably be better to just have internal logic to exit 77 ("SKIP" to GNU Automake-driven tests).

It's a bit duplicative (and will therefore warrant comments trying to keep things in sync), but the grohtml tests can just test for the same commands that our Autoconf test does in m4/groff.m4, with command -v >/dev/null, and skip the test if any aren't present.

The gropdf situation is harder.  The thing that can scotch its tests is not missing commands but missing fonts.  There's no easy way to tell from within a test script whether gropdf is going to find Type 1 fonts it can embed in the documents it generates.  (Locating them is a complex process.)  And Deri are still wrangling with testing issues anyway (see bug #63808).

In the long run (groff 1.23.1?), maybe what we want to do is something Deri wanted to do a long time ago, which is bust out part of BuildFoundries.pl as a script that is runnable at configuration time (we depend on Perl anyway).  That script can be the snout that sniffs out the presence of embeddable Type 1 fonts on the system.  Not only can we set shell variables that will inform the user whether gropdf's functionality will be attenuated (as we already do), but we can create artifacts in the build directory (at build time) that the gropdf scripts can look for.  (We already have an Automake variable, `HAVE_URW_FONTS`, that is produced by `AM_CONDITIONAL` in "configure.ac".  Perhaps what we want is more like `HAVE_EMBEDDABLE_DEFAULT_FOUNDRY` and `HAVE_EMBEDDABLE_URW_FOUNDRY`.)

The gropdf test scripts could then simply look for these artifacts in the build tree, and continue or skip themselves as appropriate.

But for groff 1.23.0, it might make more sense to just run the tests every time and let them fail as we warned in ./configure output if the host environment isn't ship-shape in the embeddable fonts department.

None of this would prevent a build from being deployed elsewhere, or Ghostscript and/or URW packages from being removed after a groff build, so gropdf will still need to re-verify the availability of fonts to be embedded every time it runs anyway.  And it does.  (Another thing Deri and I have wrangled over is the wording of the diagnostics that happen in this scenario.  :) )

Maybe it doesn't even make too much sense to worry about users being alarmed by test failures here.  Everybody's heard of "./configure && make && make install".  Few seem to bother with having "make check" in there... :-/

G. Branden Robinson <gbranden>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by deri (Posted a comment)
  • -email is unavailable- added by gbranden (gropdf author/maintainer)
  • -email is unavailable- added by gbranden (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-02-23 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2023-02-21 gbranden Planned ReleaseNone 1.23.0
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code