PhilterPaper / Perl-PDF-Builder

Extended version of the popular PDF::API2 Perl-based PDF library for creating, reading, and modifying PDF documents
https://www.catskilltech.com/FreeSW/product/PDF%2DBuilder/title/PDF%3A%3ABuilder/freeSW_full
Other
6 stars 7 forks source link

Corrupt PDF from G4-encoded TIFF #167

Open carygravel opened 3 years ago

carygravel commented 3 years ago

Here is another TIFF that PDF::Builder doesn't embed correctly.

I'll work on this now. I'm guessing the problem is that the TIFF is compressed in strips, and PDF has no concept of strips. There was the same problem with LZW.

PhilterPaper commented 3 years ago

I can confirm that the PDF from this TIFF fails to display in all my readers: Adobe Acrobat, Firefox, Chrome, XpdfReader. Thanks for taking a look at this -- I appreciate your work on TIFF. Don't forget to check if the PlanarConfig might be different in this case.

carygravel commented 3 years ago

I've more or less finished the fix for this. I'll have a go at #172 first, then complete this.

carygravel commented 3 years ago

The tests runs fine everywhere except on Windows:

https://github.com/carygravel/Perl-PDF-Builder/runs/3923782560?check_suite_focus=true

I'm guessing that this means that the newly wrapped TIFFReverseBits() is segfaulting for some reason, although the windows tests for Graphics::TIFF itself pass fine:

https://github.com/carygravel/graphics-tiff/actions/runs/1328737417

Would you mind installing Graphics::TIFF v17, run tiff.t from PDF::Builder, and give me the error message?

PhilterPaper commented 3 years ago

With v17, all 19 t/tiff.t tests return "ok". Could you check that you're up-to-date on PDF::Builder? Do I need to pull #174 first?

carygravel commented 3 years ago

With v17, all 19 t/tiff.t tests return "ok". Could you check that you're up-to-date on PDF::Builder? Do I need to pull #174 first?

Yes. I wouldn't merge it just yet, just pull to a local branch, please and test again.

PhilterPaper commented 3 years ago

As far as I can tell, my working copy is the same as what's on GitHub, and with #174 pulled in (I kept a backup copy) it still gives all 19 tests "ok". A few weeks ago I did make some minor changes to, among other things, Builder.pm, TIFF.pm and TIFF_GT.pm, so please be sure that you're up to date there. Could the Graphics::TIFF it's trying to run (for you) be corrupted somehow?

test.yml ran OK too (takes a long time on Windoze).

carygravel commented 3 years ago

If you only had 19 tests, then you'd not pulled my branch with the changes.

I've split up the commit. #175 doesn't include the new method from Graphics::TIFF, and therefore all test pass, also in Github, so I think you can merge this with no risk.

Once you've done so, I'll create a pull request for the rest so that we can debug the smaller problem.

PhilterPaper commented 3 years ago

I have not been pulling or copying anything directly from your branch, just pulling your PRs against PDF::Builder. Your #175 looks quite large, so it will take me a day or two to comb over it, satisfy myself that I understand what you're doing and that the code looks good and you got my latest changes into it. I'll let you know when it passes all the tests (test.yml, t/tiff.t, my local TIFF test suite, and other general Builder tests) and it's a good place to drive a stake into the ground.

PhilterPaper commented 3 years ago

I took a look-over #175 and posted some questions and issues there, in case you didn't see it.

PhilterPaper commented 3 years ago

I pulled your PR 175 and ran t/tiff.t. test 20 is the first new one. The following is with the OLD .pm files:

ok 19 - bilevel and alpha when width not a whole number of bytes with GT

   **** Error: File encountered 'rangecheck' error while processing an image.
               Output may be incorrect.
not ok 20 - bilevel and alpha when width not a whole number of bytes without GT # TODO No support for alpha layer without GT yet
#   Failed (TODO) test 'bilevel and alpha when width not a whole number of bytes without GT'
#   at t\tiff.t line 466.
#          got: '# ImageMagick pixel enumeration: 6,1,255,gray
# 0,0: (255)  #FFFFFF  gray(255)
# 1,0: (255)  #FFFFFF  gray(255)
# 2,0: (255)  #FFFFFF  gray(255)
# 3,0: (255)  #FFFFFF  gray(255)
# '
#     expected: '# ImageMagick pixel enumeration: 6,1,255,gray
# 0,0: (0)  #000000  gray(0)
# 1,0: (255)  #FFFFFF  gray(255)
# 2,0: (0)  #000000  gray(0)
# 3,0: (255)  #FFFFFF  gray(255)
# '
ok 21 - multi-strip group 3 (not converted to flate) with GT
ok 22 - multi-strip g4 (not converted to flate) with GT
ok 23 - multi-strip g3 min-is-black (not converted to flate) with GT
ok 24 - multi-strip g4 min-is-black (not converted to flate) with GT
ok 25 - LSB fillorder with GT

With the NEW (updated) .pm files, things get worse:

1..25
ok 1 - '$pdf->image_tiff(filename)' isa 'PDF::Builder::Resource::XObject::Image::TIFF_GT'
ok 2 - Image from filename has a width
ok 3 - Add TIFF to PDF
ok 4 - '$pdf->image_tiff(filehandle)' isa 'PDF::Builder::Resource::XObject::Image::TIFF'
ok 5 - Image from filehandle has a width
ok 6 - '$pdf->image_tiff(), LZW compression' isa 'PDF::Builder::Resource::XObject::Image::TIFF_GT'
ok 7 - Add TIFF to PDF
ok 8 - Fail fast if the requested file doesn't exist
ok 9 - alpha + flate
ok 10 - G4 (not converted to flate)
ok 11 - single-strip lzw (not converted to flate) with GT
ok 12 - multi-strip lzw (not converted to flate) with GT
ok 13 - lzw+horizontal predictor (not converted to flate) with GT
ok 14 - alpha + lzw

   **** Error: Unable to determine object number for -nouseGT so ignoring it               Output may be incorrect.
   **** Error: File did not complete the page properly and may be damaged.
               Output may be incorrect.
not ok 15 - single-strip lzw (not converted to flate) without GT
#   Failed test 'single-strip lzw (not converted to flate) without GT'
#   at t\tiff.t line 325.
#          got: '# ImageMagick pixel enumeration: 1000,100,255,gray
# 0,0: (255)  #FFFFFF  gray(255)
# 1,0: (255)  #FFFFFF  gray(255)
... lots of lines cut out ...
# 998,99: (255)  #FFFFFF  gray(255)
# 999,99: (255)  #FFFFFF  gray(255)
# '
#     expected: '# ImageMagick pixel enumeration: 1000,100,255,gray
# 0,0: (255)  #FFFFFF  gray(255)
# 1,0: (255)  #FFFFFF  gray(255)
... lots of lines cut out ...
# 998,99: (255)  #FFFFFF  gray(255)
# 999,99: (255)  #FFFFFF  gray(255)
# '

   **** Error: Unable to determine object number for -nouseGT so ignoring it               Output may be incorrect.
   **** Error: File did not complete the page properly and may be damaged.
               Output may be incorrect.
not ok 16 - lzw+horizontal predictor (not converted to flate) without GT
#   Failed test 'lzw+horizontal predictor (not converted to flate) without GT'
#   at t\tiff.t line 352.
#          got: '# ImageMagick pixel enumeration: 20,20,255,gray
# 0,0: (255)  #FFFFFF  gray(255)
# 1,0: (255)  #FFFFFF  gray(255)
... lots of lines cut out ...
# 18,19: (255)  #FFFFFF  gray(255)
# 19,19: (255)  #FFFFFF  gray(255)
# '
#     expected: '# ImageMagick pixel enumeration: 20,20,255,gray
# 0,0: (0)  #000000  gray(0)
# 1,0: (0)  #000000  gray(0)
... lots of lines cut out ...
# 18,19: (0)  #000000  gray(0)
# 19,19: (0)  #000000  gray(0)
# '
ok 17 # skip multi-strip lzw without GT is not currently supported
ok 18 - successfully read TIFF with colormap
ok 19 - bilevel and alpha when width not a whole number of bytes with GT

   **** Error: Unable to determine object number for -nouseGT so ignoring it               Output may be incorrect.
   **** Error: File did not complete the page properly and may be damaged.
               Output may be incorrect.
not ok 20 - bilevel and alpha when width not a whole number of bytes without GT # TODO No support for alpha layer without GT yet
#   Failed (TODO) test 'bilevel and alpha when width not a whole number of bytes without GT'
#   at t\tiff.t line 466.
#          got: '# ImageMagick pixel enumeration: 6,1,255,gray
# 0,0: (255)  #FFFFFF  gray(255)
# 1,0: (255)  #FFFFFF  gray(255)
# 2,0: (255)  #FFFFFF  gray(255)
# 3,0: (255)  #FFFFFF  gray(255)
# '
#     expected: '# ImageMagick pixel enumeration: 6,1,255,gray
# 0,0: (0)  #000000  gray(0)
# 1,0: (255)  #FFFFFF  gray(255)
# 2,0: (0)  #000000  gray(0)
# 3,0: (255)  #FFFFFF  gray(255)
# '
ok 21 - multi-strip group 3 (not converted to flate) with GT
ok 22 - multi-strip g4 (not converted to flate) with GT
ok 23 - multi-strip g3 min-is-black (not converted to flate) with GT
ok 24 - multi-strip g4 min-is-black (not converted to flate) with GT
ok 25 - LSB fillorder with GT
# Version: ImageMagick 7.0.10-49 Q16 x64 2020-12-14 http://www.imagemagick.org
# Copyright: Copyright (C) 1999-2018 ImageMagick Studio LLC
# License: http://www.imagemagick.org/script/license.php
# Visual C++: 192829334
# Features: Cipher DPC HDRI Modules OpenCL OpenMP(2.0) 
# Delegates (built-in): bzlib cairo flif freetype gslib heic jng jp2 jpeg lcms lqr lzma openexr pangocairo png ps raw rsvg tiff webp xml zlib
# GPL Ghostscript 9.53.3 (2020-10-01)
# Copyright (C) 2020 Artifex Software, Inc.  All rights reserved.
# Looks like you failed 2 tests of 25.

I have NOT updated Ghostscript, ImageMagick, or any other utilities you use in tiff.t. Do I need to? Graphics::TIFF is v17.

PhilterPaper commented 3 years ago

Reopening, as discussion taking place here for PRs such as #175.

PhilterPaper commented 3 years ago

test.yml reports Full Windows failed on Run choco install...

Ghostscript.app v9.55.0 [Approved]
ghostscript.app package files install completed. Performing other installation steps.
Installing 64 bit version
Installing Ghostscript.app...
Ghostscript.app has been installed.
  ghostscript.app may be able to be automatically uninstalled.
ghostscript not installed. An error occurred during installation:
 Access to the path 'C:\ProgramData\chocolatey\lib\Ghostscript.app\tools\gs9550w64.exe' is denied.
ghostscript package files install failed with exit code 1. Performing other installation steps.
The install of ghostscript was NOT successful.
ghostscript not installed. An error occurred during installation:
 Access to the path 'C:\ProgramData\chocolatey\lib\Ghostscript.app\tools\gs9550w64.exe' is denied.

Chocolatey installed 5/6 packages. 1 packages failed.
 See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).

Installed:
 - imagemagick.app v7.1.0.10
 - dejavufonts v2.37
 - imagemagick v7.1.0.10
 - vcredist2010 v10.0.40219.32503
 - ghostscript.app v9.55.0

Failures
 - ghostscript (exited 1) - ghostscript not installed. An error occurred during installation:
 Access to the path 'C:\ProgramData\chocolatey\lib\Ghostscript.app\tools\gs9550w64.exe' is denied.
Error: Process completed with exit code 1.
carygravel commented 3 years ago

I pulled your PR 175 and ran t/tiff.t. test 20 is the first new one. The following is with the OLD .pm files:

Note that test 20 is not marked as a test failure, but as TODO and thus doesn't count. I just wanted to make sure it wasn't forgotten.

But if tests 22 and 24 pass, then you are not using the old .pms, as I made sure that tests failed before adding the new filter to fix things.

The other problems show me that something must have gone wrong with your local changes, as what you merged obviously works fine in the full Ubuntu runner on github, and the windows failures have nothing to do with the Perl code.

I have NOT updated Ghostscript, ImageMagick, or any other utilities you use in tiff.t. Do I need to? Graphics::TIFF is v17.

No you don't need to update Ghostscript or ImageMagick.

Your repo on Github already has my changes merged. My suggestion would be to make a local fresh clone of your repo and before you change anything, run the tests.

carygravel commented 3 years ago

test.yml reports Full Windows failed on Run choco install...

The install of ghostscript was NOT successful. ghostscript not installed. An error occurred during installation: Access to the path 'C:\ProgramData\chocolatey\lib\Ghostscript.app\tools\gs9550w64.exe' is denied.

This worked in the past, and therefore must be an infrastructure problem with Github. To confirm this, I hit the rerun button on one of my old runs, and it fails, too.

I've just reported this to Github. Let's see what they say.

PhilterPaper commented 3 years ago

Better news (I hope):

  1. I re-ran the test.yml suite this evening, and it ran without error. Whatever glitch was on GitHub seems to have been resolved.
  2. I double-checked that you had based your Builder.pm and TIFF_GT.pm on my latest updates (it appears you had), and what I was running against matched GitHub (after I pulled 175). I'm not sure what (if anything) changed, but now t/tiff.t reports only test 20 gets a 'rangecheck' (output at the top of my penultimate post, for "old" .pm), and 24 other tests pass. You mentioned this is a "TODO" item -- do you expect it to fail?

If test 20's failure is acceptable, I ran my other TIFF tests. t/filter-ccittfaxdecode.t runs all 26 tests successfully. The TIFF test suite (with and without GT) has interesting results. Everything appears to be fine for the GT tests, and now G4.TIF properly displays on Adobe! Before, I got a message about insufficient data (it displayed OK on Firefox and XpdfReader, and still does). However, G31D.TIF doesn't display anymore on Firefox, so there are still some items yet to be resolved (I recall that it worked before). After a quick look, everything looks OK for the non-GT cases.

carygravel commented 3 years ago
2. I double-checked that you had based your Builder.pm and TIFF_GT.pm on my latest updates (it appears you had), and what I was running against matched GitHub (after I pulled 175). I'm not sure what (if anything) changed, but now t/tiff.t reports only test 20 gets a 'rangecheck' (output at the top of my penultimate post, for "old" .pm), and 24 other tests pass. You mentioned this is a "TODO" item -- do you expect it to fail?

If you look at the test, it is marked skip & todo, which means that the test is run and thus you see the failure, but that the failure does not count. This is expected. When one of us has time, the filters are now in place that we could the patch the non-GT code to work with alpha layers, etc.

If test 20's failure is acceptable, I ran my other TIFF tests. t/filter-ccittfaxdecode.t runs all 26 tests successfully. The TIFF test suite (with and without GT) has interesting results. Everything appears to be fine for the GT tests, and now G4.TIF properly displays on Adobe! Before, I got a message about insufficient data (it displayed OK on Firefox and XpdfReader, and still does). However, G31D.TIF doesn't display anymore on Firefox, so there are still some items yet to be resolved (I recall that it worked before). After a quick look, everything looks OK for the non-GT cases.

I did check the test images as well, but only with Evince, not Firefox. I'll see if I can reproduce your problem with G31D.TIF & Firefox. Having said that, I didn't touch the way Builder handles group 3 images - I implemented a group 3 filter, but I had to do that on the way to the group 4 filter.

PhilterPaper commented 3 years ago

Last night I reran the Action and it appeared to pass. This morning I had an email that it had failed, so I ran it again. I see test 20 in tiff.t "failed", and it said there was another failure (apparently in tiff.t) but I don't see it.

I don't think test 20 is skipping properly, as it's being treated as a true failure. Is the intent that it just skip unconditionally, or just in some cases?

carygravel commented 3 years ago

I don't think test 20 is skipping properly, as it's being treated as a true failure. Is the intent that it just skip unconditionally, or just in some cases?

https://github.com/PhilterPaper/Perl-PDF-Builder/runs/3984667376?check_suite_focus=true:

t/tiff.t (Wstat: 29696 Tests: 22 Failed: 0) Non-zero exit status: 116 Parse errors: Bad plan. You planned 25 tests but ran 22.

If you are talking about this failure, the problem is not test 20, but that it ran and passed tests 1-22 fine, but died in the middle of running test 23 (but only on Windows - Linux ran all ok). This is going to be hard to debug unless we can reproduce it regularly. I'll take a look and see if I can narrow it down, but the weird thing is that is passed before on Windows.

PhilterPaper commented 3 years ago

Here is what the Actions log shows for yesterday morning's run:

t/tiff.t .................... 
1..25
ok 1 - '$pdf->image_tiff(filename)' isa 'PDF::Builder::Resource::XObject::Image::TIFF_GT'
ok 2 - Image from filename has a width
ok 3 - Add TIFF to PDF
ok 4 - '$pdf->image_tiff(filehandle)' isa 'PDF::Builder::Resource::XObject::Image::TIFF'
ok 5 - Image from filehandle has a width
ok 6 - '$pdf->image_tiff(), LZW compression' isa 'PDF::Builder::Resource::XObject::Image::TIFF_GT'
ok 7 - Add TIFF to PDF
ok 8 - Fail fast if the requested file doesn't exist
ok 9 - alpha + flate
ok 10 - G4 (not converted to flate)
ok 11 - single-strip lzw (not converted to flate) with GT
ok 12 - multi-strip lzw (not converted to flate) with GT
ok 13 - lzw+horizontal predictor (not converted to flate) with GT
ok 14 - alpha + lzw
ok 15 - single-strip lzw (not converted to flate) without GT
ok 16 - lzw+horizontal predictor (not converted to flate) without GT
ok 17 # skip multi-strip lzw without GT is not currently supported
ok 18 - successfully read TIFF with colormap
ok 19 - bilevel and alpha when width not a whole number of bytes with GT

   **** Error: File encountered 'rangecheck' error while processing an image.
               Output may be incorrect.
not ok 20 - bilevel and alpha when width not a whole number of bytes without GT # TODO No support for alpha layer without GT yet
#   Failed (TODO) test 'bilevel and alpha when width not a whole number of bytes without GT'
#   at t/tiff.t line 466.
#          got: '# ImageMagick pixel enumeration: 6,1,255,gray
# 0,0: (255)  #FFFFFF  gray(255)
# 1,0: (255)  #FFFFFF  gray(255)
# 2,0: (255)  #FFFFFF  gray(255)
# 3,0: (255)  #FFFFFF  gray(255)
# '
#     expected: '# ImageMagick pixel enumeration: 6,1,255,gray
# 0,0: (0)  #000000  gray(0)
# 1,0: (255)  #FFFFFF  gray(255)
# 2,0: (0)  #000000  gray(0)
# 3,0: (255)  #FFFFFF  gray(255)
# '
ok 21 - multi-strip group 3 (not converted to flate) with GT
ok 22 - multi-strip g4 (not converted to flate) with GT
Dubious, test returned 116 (wstat 29696, 0x7400)
Failed 3/25 subtests 
    (less 1 skipped subtest: 21 okay)
t/viewer-preferences.t ...... 
1..3
ok 1 - Duplex => Simplex
ok 2 - Duplex => DuplexFlipLongEdge
ok 3 - Duplex => DuplexFlipShortEdge
ok

Test Summary Report
-------------------
t/00-all-usable.t         (Wstat: 0 Tests: 111 Failed: 0)
Failed 1/43 test programs. 0/712 subtests failed.
  TODO passed:   111
t/tiff.t                  (Wstat: 29696 Tests: 22 Failed: 0)
  Non-zero exit status: 116
  Parse errors: Bad plan.  You planned 25 tests but ran 22.
Files=43, Tests=712, 32 wallclock secs ( 0.19 usr +  0.11 sys =  0.30 CPU)
Result: FAIL
mingw32-make: *** [Makefile:999: test_dynamic] Error 255
Error: Process completed with exit code 1.

OK, the Fine Print seems to suggest that test 20 was skipped, but then why all the errors reported during it? That's very confusing. Maybe it would be cleaner to unconditionally skip test 20 until it's ready. Then it looks like tests 23 through 25 vanished into a Black Hole, like maybe this was a catastrophic failure in test 23 that just knocked tiff.t off the tracks?

carygravel commented 3 years ago

OK, the Fine Print seems to suggest that test 20 was skipped, but then why all the errors reported during it? That's very confusing. Maybe it would be cleaner to unconditionally skip test 20 until it's ready. Then it looks like tests 23 through 25 vanished into a Black Hole, like maybe this was a catastrophic failure in test 23 that just knocked tiff.t off the tracks?

The errors were reported because it was skipped as TODO, which means that it is run anyway and the failure ignored, as opposed with without the TODO, when the test is completely skipped. If you think it is confusing, simply take away the TODO.

As I said before: it ran and passed tests 1-22 fine, but died in the middle of running test 23 (but only on Windows - Linux ran all ok). This is going to be hard to debug unless we can reproduce it regularly. I'll take a look and see if I can narrow it down, but the weird thing is that is passed before on Windows.

PhilterPaper commented 3 years ago

What is your intent with test 20? Is it "ready to run" with GT + GS + IM, in which case there is an error to be dealt with, or is it intended to be skipped for the time being (incomplete)? The TODO message seems to be such that it should run (only) with GT but not without it, and so the SKIP should include and $has_GT? Even then, if it's currently not ready for use, the SKIP should just unconditionally skip it?

Even after removing the TODO clause, test 20 still fails with the same 'rangecheck' error, and all other tests (including 23-25) run OK (running locally "perl t\tiff.t" on Win10). Is this failure only on Windows? I seem to encounter it regularly, so if there's any debugging you want to ask me to do...

By the way, being somewhat red-green colorblind, I think the "local" keyword in test 20 is colored red. Are all Perl keywords colored red in the Github listing? It's hard for me to tell. If it's just "local" that's red, perhaps it's being flagged as a problem.

carygravel commented 3 years ago

What is your intent with test 20? Is it "ready to run" with GT + GS + IM, in which case there is an error to be dealt with, or is it intended to be skipped for the time being (incomplete)? The TODO message seems to be such that it should run (only) with GT but not without it, and so the SKIP should include and $has_GT? Even then, if it's currently not ready for use, the SKIP should just unconditionally skip it?

My intent with test 20 is to have the test ready for when someone has the time to fix the code. The pieces (i.e. filters) are all there now. It is just a matter of porting the code from TIFF_GT.pm to TIFF.pm. Hence the code runs without GT.

Even after removing the TODO clause, test 20 still fails with the same 'rangecheck' error, and all other tests (including 23-25) run OK (running locally "perl t\tiff.t" on Win10). Is this failure only on Windows? I seem to encounter it regularly, so if there's any debugging you want to ask me to do...

Test 20 fails on Linux too (see line 960). The failure doesn't count at the moment, because it is skipped/marked TODO.

By the way, being somewhat red-green colorblind, I think the "local" keyword in test 20 is colored red. Are all Perl keywords colored red in the Github listing? It's hard for me to tell. If it's just "local" that's red, perhaps it's being flagged as a problem.

Github colours many keywords red, e.g. local, unless, and, etc. This is not the problem. We could rewrite it as a todo_skip.

PhilterPaper commented 3 years ago

This is getting stranger and stranger. First of all, regarding test 20, from the TODO notes and the use of -nouseGT => 1 it sounds like this test is ONLY for non-GT with alpha. Since alpha is not supported without GT, this test should always fail if run. Therefore I changed the SKIP to just bypass it unconditionally until such time as someone gets around to putting alpha support in the pure Perl TIFF support. If you or someone else wants something to do and is eager to implement this, be my guest; I don't see any point in extending the pure Perl support when we have Graphics::TIFF to do the job.

While test 20 was "failing", test 23 seemed to fail catastrophically (24 and 25 didn't even try to run). I put "I am here" debug statements in 23, 24, and 25. Now 23 and 24 ran OK, and 25 failed somewhere between creating the PDF and running GS to create a PNG file. That's odd, so I instrumented all the other code in 25. Now it fails catastrophically in test 22! Tests 23-25 never get a chance to run. Same message as always: Dubious, test returned 116 (wstat 29696, 0x7400). This has got to be a bug in Github or even Perl -- I can't see how a flaw in tiff.t is going to make a failure jump around like that. I'll have to stare at tiff.t some more. I vaguely recall encountering an inexplicable jumping bug a year or two ago, but I don't remember right now how I tracked it down and swatted it.

carygravel commented 3 years ago

While test 20 was "failing", test 23 seemed to fail catastrophically (24 and 25 didn't even try to run). I put "I am here" debug statements in 23, 24, and 25. Now 23 and 24 ran OK, and 25 failed somewhere between creating the PDF and running GS to create a PNG file. That's odd, so I instrumented all the other code in 25. Now it fails catastrophically in test 22! Tests 23-25 never get a chance to run. Same message as always: Dubious, test returned 116 (wstat 29696, 0x7400). This has got to be a bug in Github or even Perl -- I can't see how a flaw in tiff.t is going to make a failure jump around like that. I'll have to stare at tiff.t some more. I vaguely recall encountering an inexplicable jumping bug a year or two ago, but I don't remember right now how I tracked it down and swatted it.

I tried rerunning the action from PR and am seeing something similar - it fails in a difference place from the previous failure.

If it didn't take 25mins to run, it would be easier to debug.

carygravel commented 3 years ago

BTW. You seem to be debugging in master. Personally, I would always debug/develop in a new branch, then you can squash the commits when you have finished before rebasing/merging with master, thereby not polluting master with lots of tiny debugging commits.

carygravel commented 3 years ago

Now it ran cleanly.

carygravel commented 3 years ago

Interestingly, rerunning it, it now fails adding the TIFF

PhilterPaper commented 3 years ago

Is Github using any particular Perl release for its Action tests? I wonder if forcing an older version (such as 5.26) would show that the problem is the latest release of Perl, on Windows ("runs-on: windows-latest"). But basic-windows uses the same version?

If it didn't take 25mins to run, it would be easier to debug.

Any idea what it's doing for Full Windows that might take so long? In basic-windows, you have Readonly, but Graphics::TIFF in full-windows. All the difference seems to be there in that one step. Could choco be taking so long? There's some extra business with GS and IM on full-windows.

carygravel commented 3 years ago

Adding perl -v to test.yml reveals that at the moment, Github on Windows uses 5.32.1.

The action output shows the timing for the various steps. There we can see that choco runs in 70s, and that most of the 25mins are in cpan. Installing Graphics::TIFF is not the problem, but compiling libtiff first.

PhilterPaper commented 3 years ago

Just an aside... we may both be running on Github, but is it possible that these are physically different servers that might have slightly different configurations (for whatever reason)? I can't believe that they run all of Github on one server! They ought to be exactly the same configuration, but who knows.

20+ minutes to run the build and test under full-windows is ridiculous. Could there be multiple substeps (within 'cpan'?) that are timing out, or something? How many packages are being built here but not in basic-windows? Just GT? That shouldn't take 20 minutes to brew up!

You added Readonly to basic-windows, but not full-windows. Could that make any difference? full-windows was slow before you added the code needing Readonly, but was wondering if it was an oversight.

carygravel commented 3 years ago

Just an aside... we may both be running on Github, but is it possible that these are physically different servers that might have slightly different configurations (for whatever reason)? I can't believe that they run all of Github on one server! They ought to be exactly the same configuration, but who knows.

No. Each run starts with exactly the same docker (virtual) container, which is then thrown away afterwards.

20+ minutes to run the build and test under full-windows is ridiculous. Could there be multiple substeps (within 'cpan'?) that are timing out, or something? How many packages are being built here but not in basic-windows? Just GT? That shouldn't take 20 minutes to brew up!

The 20 minutes is to compile libtiff first.

You added Readonly to basic-windows, but not full-windows. Could that make any difference? full-windows was slow before you added the code needing Readonly, but was wondering if it was an oversight.

I didn't need to for full-windows, because it is a dependency of Graphics::TIFF, and therefore pulled in anyway.

PhilterPaper commented 3 years ago

Nearly 20 minutes to compile/link the libtiff library? How long does it take when you're testing GT alone? Something must be wrong if it takes that long!

carygravel commented 3 years ago

When I test GT on its own depending on how libtiff is built, it takes up to 30 mins on Windows. Because I can fetch pre-built binaries for Linux, it takes seconds.

PhilterPaper commented 3 years ago

When committing to Github, is there any way to avoid building all the other stuff (especially Graphics::TIFF) from scratch and use prebuilt binaries/libraries built ahead of time? It just seems ridiculous that if it doesn't come with Windows, it has to be built from source every time. When installing from CPAN, most prerequisite binaries are available already (from time to time one is out-of-date and has to be built on-the-fly); why can't Github behave this way?

carygravel commented 3 years ago

When committing to Github, is there any way to avoid building all the other stuff (especially Graphics::TIFF) from

Compiling Graphics::TIFF is not the problem - that takes seconds. The problem is libtiff.

scratch and use prebuilt binaries/libraries built ahead of time? It just seems ridiculous that if it doesn't come with Windows, it has to be built from source every time. When installing from CPAN, most prerequisite binaries are available already (from time to time one is out-of-date and has to be built on-the-fly); why can't Github behave this way?

It is not about coming with Windows - Perl doesn't ship with Windows, it is about being available. Unfortunately, choco does not package libtiff currently. The github windows image also ship with vcpkg, which is like choco, but does have a libtiff package.

I did play with that for a while when I was setting up the windows tests, but I was unable to get Perl to link against it (which is when I finally got the Alien version working). Of course, if you could remove your local libtiff installation, install via vcpkg, and convince Perl to compile Graphics::TIFF against the vcpkg package, it would speed up the builds by an order of magnitude.

PhilterPaper commented 3 years ago

If everything gets tested (tiff.t) under Ubuntu build, I propose that for the time being we just comment out (disable) the full-windows build. Try it again some time in the future, to see if whatever glitch causing the error has gone away, and maybe do something to shorten the build time. I'm sure the folks at Github aren't happy that a single test build and run takes over 20 minutes -- that ties up quite a few of their server resources.

There's no point in trying to reduce the build/test time for full-windows until the runtime bug is fixed, and at this point it sounds like neither of us has any idea what that bug is. What do you think?

carygravel commented 3 years ago

I'm sure the folks at Github aren't happy that a single test build and run takes over 20 minutes

I don't suppose they care - as long as the monthly limit is not exceeded.

There's no point in trying to reduce the build/test time for full-windows until the runtime bug is fixed, and at this point it sounds like neither of us has any idea what that bug is. What do you think?

I'm working on it. I'll let you know when I have something to report.

PhilterPaper commented 3 years ago

OK, I'll leave test.yml alone for now, so long as I don't have frequent commits to make. Just to repeat, tiff.t (and other t-tests) work fine on Windows (my local Strawberry installation); it's just on Github that it's failing for Windows.

PhilterPaper commented 2 years ago

I had to go ahead and disable the full-windows section of test.yml. Thanks for the effort, and if you can come up with a definite fix for it, that's great, but I don't think it's any great loss to not fully test on Windows. The code (.pm files) is checked on the Ubuntu test, so only certain parts of the Windows build process are really being skipped, and I should be catching anything there when I build and run on my local Windows system.

PhilterPaper commented 2 years ago

The GD owner (see https://rt.cpan.org/Public/Bug/Display.html?id=141125) suggests using cpm rather than cpan to install prereqs. By any chance have you looked into this?

carygravel commented 2 years ago

No. And I only use cpan when testing with the Windows runners on Github, as for the Linux runners, the Perl dependencies are more quickly resolved using apt.

PhilterPaper commented 2 years ago

Attached is the GD test.yml as a possible model for PDF::Builder (I haven't tried anything). Just FYI. GD-test.yml.txt rename to test.yml.

PhilterPaper commented 2 years ago

I was just looking at something in test.yml (GitHub build/test on commit) and noticed that you require a ppa: jeffreyratcliffe/ppa for the full Ubuntu build. I'm a bit concerned that it is something that is not "stock" code, and thus PDF::Builder could end up passing on GitHub, but fail on someone's install. What is in this ppa? Something to do with gscan2pdf? I don't want tests depending on private repositories in the long term -- they should use public tools (private tools are OK over a short-term project). I tried commenting out the ppa line and full-ubuntu failed, so it's depending on something.

By the way, lint, full-, and basic-windows all started failing at one point or another, so I have disabled them for now. I don't think Windows testing is critical, as I build and run on Windows all the time, although it would be nice to have that working again. I run perlcritic against my code, so a lint isn't all that critical.

carygravel commented 2 years ago

ppa: jeffreyratcliffe/ppa is used so that it doesn't have to build Graphics::TIFF from source. BTW - I've also got PDF::Builder builds there for users with older Ubuntu versions. I uploaded it to Debian as soon as I released the first version in September 2020. Ubuntu automatically uploaded it from Debian, so the first Ubuntu release with Graphics::TIFF was Impish (AKA 21.04), released in April 2021. The github runners are still using Ubuntu 20.04, as that is the current long term support release (LTS).

The next Ubuntu LTS release is 22.04, which I assume github will switch to soon afterwards. i.e. the ppa will probably no longer be necessary sometime in May or June.

PhilterPaper commented 2 years ago

Sounds good, but doesn't Ubuntu et al. have libtiff already built? Is this just the "glue" XS code you're referring to, or is libtiff just too old a version (at the moment)? Anyway, I'm looking forward to being able to get rid of private repository prereqs. Please let me know when I can update test.yml to get rid of the ppa.

Over on the Windows side, since libtiff doesn't ship with Windows itself, can anything similar be done so that it doesn't have to build TIFF each time? Is the ppa style of things Ubuntu only? Strawberry Perl 5.22 (the current minimum for PDF::Builder) ships with libtiff 4.0.3, and 5.24 ships with 4.0.6. Are those too back-level? I vaguely recall needing 4.0.10 but I might be mistaken. Some time after May of 2022 I plan to up the minimum Perl to 5.24. Is it reasonable to have users manually obtain the libtiff from a later version of Strawberry Perl and overwrite their back-level version? I wonder if supplying a libtiff library for the GitHub Windows build (copied from Strawberry) would be permitted (by license), and even included with the PDF::Builder package? As for ActiveState and other old Perl distributions, I don't have anything to recommend if they want to stay with that. I have no experience with supplying canned library (e.g., libtiff.a) files that any build process can easily use. Do you have any experience with this?

I haven't played around in the TIFF-related build material for a while, so I may be forgetting or misremembering something that's already been discussed.

carygravel commented 2 years ago

Graphics::TIFF is just the XS wrapper, not libtiff itself. The ppa is only Ubuntu - not just only Linux.

Graphics::TIFF needs a minimum of libtiff 4.0.3, so what Strawberry Perl ships is fine.

If you wanted to provide something newer, given that the full Windows build starts by building libtiff using alien, you could take that build and turn it into an msi. If libtiff is available, then it is reasonable to expect Windows users to build Graphics::TIFF against it. Indeed, if you wanted to provide Graphics::TIFF binaries, you'd have to ship a set of binaries for every combination of libtiff version and Perl version.

PhilterPaper commented 2 years ago

Graphics::TIFF is just the XS wrapper, not libtiff itself.

Yeah, I was commenting on the Windows part of the GitHub build trying to build libtiff itself, which seems to use up a huge amount of time and is failing anyway. Do you suppose it would be possible to include libtiff.a from some recent Strawberry Perl version in the PDF::Builder GitHub repository, and have test.yml use it for the Windows full build? If that works, perhaps libtiff.a could be shipped with the PDF::Builder distribution, although I don't know how to untangle the various distributions (e.g., Strawberry Perl would not need it, but ActiveState and others might need it, and rather than using Alien to try to build it...).

Making sure the license allows this is one thing; making it work technically (needed only on non-Strawberry Windows) is quite another. Have you seen any releases which successfully do this?

carygravel commented 2 years ago

Do you suppose it would be possible to include libtiff.a from some recent Strawberry Perl version in the PDF::Builder GitHub repository, and have test.yml use it for the Windows full build?

Possibly. You would have to ship not just the .a file, but also the matching headers (.h files).

Making sure the license allows this is one thing; making it work technically (needed only on non-Strawberry Windows) is quite another. Have you seen any releases which successfully do this?

No. It is only needed for Windows, and I've never done any development on Windows before. Presumably the problem is a matter of working out which directory the files must land in.

PhilterPaper commented 1 year ago

See also #199 concerning the need for a good debugger.

PhilterPaper commented 1 year ago

Some of the TIFF images that PDF::Builder fails with appear to be only with Adobe Acrobat Reader (the free download). Although it is so widely used that it may be considered the Gold Standard, AAR seems to have some limitations, especially in support of various compression formats, that other Readers have no problem with. In such cases, all I can do is document the sorts of things AAR is found to have trouble with, and suggest that users try a better Reader if they plan to use such formats.