Skip to content

Conversation

@aelkiss
Copy link
Member

@aelkiss aelkiss commented Jan 26, 2026

  • use library-packaged grokj2k
  • use more Debian packaged perl modules

* use library-packaged grokj2k
* use more Debian packaged perl modules
@aelkiss aelkiss requested review from moseshll and rrotter January 26, 2026 20:38
@aelkiss
Copy link
Member Author

aelkiss commented Jan 26, 2026

Playwright tests pass locally for me.

@moseshll n.b. Ryan has built both arm64 and amd64 grokj2k debs.

@aelkiss
Copy link
Member Author

aelkiss commented Jan 26, 2026

The perl tests pass locally for me too. I'll see what's failing here.

@aelkiss
Copy link
Member Author

aelkiss commented Jan 26, 2026

Looks like imgsrv is failing. I'll try a clean checkout & build locally and see if I can reproduce.

@aelkiss
Copy link
Member Author

aelkiss commented Jan 26, 2026

perl tests pass for me locally even with a clean checkout & build w/o using cache. I'm rather confused. I think first I'm just going to re-try the tests here.

@moseshll
Copy link
Contributor

Looks like grok is getting confused by the -num_threads argument. I'm getting the failure locally.

@aelkiss
Copy link
Member Author

aelkiss commented Jan 26, 2026

Interesting; I wonder why I'm not getting it. I'll look into it further.

@moseshll
Copy link
Contributor

moseshll commented Jan 26, 2026

/usr/bin/grk_decompress -h claims the flag is --num-threads and makes no mention of -num_threads, I dare to suspect the developer did a bit of a tidy-up on the option style sometime recently.

And indeed, if I change imgsrv/lib/Process/Image.pm to use the "dash variant" then perl-test passes okay.

There's a case to be made for using the short form -H which appears to be unchanged between our build and the debian testing/unstable ones.

@aelkiss
Copy link
Member Author

aelkiss commented Jan 27, 2026

Thanks for looking into it. I think when I did the clean check-out to see if tests were passing locally it was using the wrong branch. Also there may have been some caching issues because even on the right branch when I built & then ran the perl-test container it was debian 12. 🤦

Option style has changed in more recent releases to be more consistent.
# RUN sed -i 's/main.*/main contrib non-free/' /etc/apt/sources.list
FROM debian:trixie AS babel-base

RUN apt-get update && apt-get install -y \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RUN apt-get update && apt-get install -y \
RUN apt-get update && apt-get install -y --no-install-recommends \

This mirrors APT::Install-Recommends "false", which is in the global apt settings on all of our servers. This will occasionally leave a package in a broken state, but if it does, we want to know about it and add necessary dependencies explicitly.


RUN echo "deb [signed-by=/etc/apt/keyrings/mlibrary-archive-keyring.gpg] https://apt.lib.umich.edu trixie main" > /etc/apt/sources.list.d/mlibrary.list

RUN apt-get update && apt-get install grokj2k
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RUN apt-get update && apt-get install grokj2k
RUN apt-get update && apt-get install -y --no-install-recommends grokj2k

I would be surprised if this works without -y, unless you happen to be installing all of grok's deps above.

Copy link
Contributor

@moseshll moseshll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a total of five uses of the old grok -num_threads option. I would recommend changing the remaining four to use -H (I have no experience with recompress so I don't know if it is needed, but...):

grep -rni num_threads imgsrv
imgsrv/bin/recompress:18:IPC::Run::run([ "/l/local/bin/kdu_expand", "-num_threads", "0", "-quiet", "-i", $image_filename, "-o", $tmp_filename, "-reduce", "$reduce" ]);
imgsrv/bin/recompress:19:IPC::Run::run([ "/l/local/bin/kdu_compress", "-num_threads", "0", "-quiet", "-i", $tmp_filename, "-o", "$final_filename", "-slope", "42988" ]);
imgsrv/lib/Process/Image.pm:197:                "-num_threads", "0", 
imgsrv/lib/Process/Image.pm:459:                "-num_threads", "0",

This is the only outstanding issue I wanted to note.

@aelkiss
Copy link
Member Author

aelkiss commented Jan 27, 2026

There are a total of five uses of the old grok -num_threads option. I would recommend changing the remaining four to use -H (I have no experience with recompress so I don't know if it is needed, but...):

grep -rni num_threads imgsrv
imgsrv/bin/recompress:18:IPC::Run::run([ "/l/local/bin/kdu_expand", "-num_threads", "0", "-quiet", "-i", $image_filename, "-o", $tmp_filename, "-reduce", "$reduce" ]);
imgsrv/bin/recompress:19:IPC::Run::run([ "/l/local/bin/kdu_compress", "-num_threads", "0", "-quiet", "-i", $tmp_filename, "-o", "$final_filename", "-slope", "42988" ]);
imgsrv/lib/Process/Image.pm:197:                "-num_threads", "0", 
imgsrv/lib/Process/Image.pm:459:                "-num_threads", "0",

This is the only outstanding issue I wanted to note.

These are all for Kakadu (kdu_expand or kdu_compress) rather than grokj2k. We may still be using a (very old) version of it in production -- it's at least still installed -- and we're not licensed for newer versions. Worth testing more thoroughly with a test machine with Debian 13.

@moseshll
Copy link
Contributor

Oh silly me, I didn't even stop to think that we still had kakadu in the mix. Likely false alarm then, sorry.

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

Successfully merging this pull request may close these issues.

4 participants