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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace libtiff_4_5 with libtiff_and_tools, update hylafaxplus #298561

Merged
merged 6 commits into from Apr 21, 2024

Conversation

Yarny0
Copy link
Contributor

@Yarny0 Yarny0 commented Mar 24, 2024

Description of changes

libtiff 4.6.0 (#261791) dropped a bunch of helper tools, thereby breaking packages that depend on these tools. To fix those packages, nixpkgs started packaging libtiff_4_5 separately, for hylafaxplus (#264226). then for gscan2pdf (#268836).

Lee Howard (core developer of hylafaxplus) later forked libtiff 4.6.0 (calling it libtiff version 4.6.0t) to provide a current version that restores those dropped helper tools (https://sourceforge.net/p/hylafax/mailman/message/58751878/ , http://www.libtiff.org/releases/v4.6.0t.html ). At the same time, he published the newest release of hylafaxplus which is compatible with his fork of libtiff.

The pull request at hand essentially replaces the old libtiff_4_5 with a new pakcage libtiff_and_tools libtiff_t containing the forked libtiff variant. This seems the right thing to do, as we no longer rely on an old and unmaintained libtiff version. The pull request also updates hylafaxplus to the newest version 7.0.8.

Alternatively, we could also replace libtiff completely with the forked version. This might be even better as we would have only one libtiff library that provides all features needed by all packages in nixpkgs. However, the future of the fork is unclear (Lee Howard advocates for the helper tools to be restored in the original libtiff library, https://www.asmail.be/msg0055042152.html), so I guess we should wait some time and also watch what other distributions do.

The forked library of LibTIFF is -- unfortunatelly -- also called LibTIFF. The attribute name libtiff_and_tools is inspired by the name "libtiff+tools" of the corresponding Gitlab repository. The attribute name libtiff_t is inspired by the version suffix t.

Notifying gscan2pdf maintainer @pacien and contributor of the gscan2pdf workaround pull request @gador: I verified that gscan2pdf still builds with libtiff_t 4.6.0t, but you might want to test the resulting package. By the way: The gscan2pdf project seems to be already preparing for libtiff 4.6.0 compatibility: https://sourceforge.net/p/gscan2pdf/bugs/427/ .

Also Notifying @trofi and @risicle , as you helped me shape the libtiff_4_5 pull request and there is not libtiff maintainer.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Result of nixpkgs-review run on x86_64-linux 1

9 packages built:
  • gscan2pdf
  • gscan2pdf.man
  • hylafaxplus
  • libtiff_and_tools
  • libtiff_and_tools.bin
  • libtiff_and_tools.dev
  • libtiff_and_tools.dev_private
  • libtiff_and_tools.doc
  • libtiff_and_tools.man

Add a 馃憤 reaction to pull requests you find important.

@Yarny0
Copy link
Contributor Author

Yarny0 commented Mar 24, 2024

The original branch failed the "pkgs/by-name" check, so I updated the branch. The failed check reported:

Attribute pkgs.libtiff_and_tools is a new top-level package using pkgs.callPackage ./pkgs/development/libraries/libtiff/libtiff_and_tools.nix { /* ... */ }.
Please define it in pkgs/by-name/li/libtiff_and_tools/package.nix instead.
See pkgs/by-name/README.md for more details.
Since the second callPackage argument is { }, no manual callPackage in pkgs/top-level/all-packages.nix is needed anymore.

This is not so easy: libtiff and libtiff_and_tools share some patch files, so moving them to pkgs/by-name would require duplicating those files. Instead, I followed the Recommendation for new packages with multiple versions and added an indirection via inherit to the corresponding packages in all-packages.nix. This should satisfy the check.

@gador
Copy link
Contributor

gador commented Mar 31, 2024

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).

Result of nixpkgs-review pr 298561 run on x86_64-linux 1

9 packages built:
  • gscan2pdf
  • gscan2pdf.man
  • hylafaxplus
  • libtiff_and_tools
  • libtiff_and_tools.bin
  • libtiff_and_tools.dev
  • libtiff_and_tools.dev_private
  • libtiff_and_tools.doc
  • libtiff_and_tools.man

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3745

@pacien
Copy link
Contributor

pacien commented Apr 6, 2024 via email

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3805

@Yarny0
Copy link
Contributor Author

Yarny0 commented Apr 17, 2024

Notifying some members of the new libtiff maintainer team: @imincik @l0b0 @nh2

Copy link
Contributor

@imincik imincik left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR.
I left one comment suggesting description change in libtiff_and_tools.

Assuming that 1) means that libtiff_and_tools doesn't contain any known security issues, I am OK with this PR. All changes look good to me.

pkgs/development/libraries/libtiff/libtiff_and_tools.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libtiff/libtiff_and_tools.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@Yarny0 Yarny0 force-pushed the hylafax-update-libtiff-and-tools branch from 03a04ce to 9ac909d Compare April 19, 2024 16:31
Note that this update makes hylafaxplus compatible
with libtiff 4.6.0t (a fork of libtiff at version 4.6.0).
A follow-up commit will introduce that library
fork and make hylafaxplus build with it.
libtiff 4.6.0 dropped a bunch of helper tools,
thereby breaking packages that depend on these tools.
To fix those packages, nixpkgs started packaging libtiff_4_5
separately, see commit f57a4b0.

Currently, two packages use libtiff_4_5:

* hylafaplus (cd3771c)
* gscan2pdf (9a579e1)

Lee Howard (core developer of hylafaxplus)
forked libtiff 4.6.0 to provide a current version
that restores those dropped helper tools.
The library is also called "libtiff",
with current version "4.6.0t".
It is based on libtiff 4.6.0 and incorporates several fixes,
particularly for the dropped helper tools,
see https://sourceforge.net/p/hylafax/mailman/message/58751878/
and http://www.libtiff.org/releases/v4.6.0t.html .

The commit at hand packages that fork for nixpkgs.
Follow-up commits will replace libtiff_4_5 with
libtiff_t, so affected packages can
again use a current libtiff library.

The build recipe of libtiff_t is based on the libtiff recipe.
Besides adapted URLs, the only change is dropping `passthru`, as
it referred to many packages depending on the original libtiff.
The unorthodox code introduced in all-packages.nix
is needed to satisfy the automated "by-name" check;
see "Recommendation for new packages with multiple versions"
in the file `pkgs/by-name/README.md`.

Depending on how things develop in the future,
we might want to switch completely
to the forked libtiff library one day.
Or the original libtiff restores the missing tools,
making libtiff_t superfluous.
hylafaxplus is not compatible with libtiff 4.6.0
as published on https://libtiff.gitlab.io/libtiff/
and http://www.simplesystems.org/libtiff/ .
To fix the build, we used an older libtiff version
(see cd3771c).

In the meantime, hylafaxplus developer Lee Howard
published a forked version "4.6.0t" of libtiff,
providing the missing tools for hylafaxplus, see
https://sourceforge.net/p/hylafax/mailman/message/58751879/ .
The commit at hand changes the libtiff
version for hylafaxplus to use that fork.
gscan2pdf is not compatible with libtiff 4.6.0
as published on https://libtiff.gitlab.io/libtiff/
and http://www.simplesystems.org/libtiff/ .
To fix the build, we used an older libtiff version
(see 9a579e1).

In the meantime, hylafaxplus developer Lee Howard
published a forked version "4.6.0t" of libtiff,
providing the missing tools, see
https://sourceforge.net/p/hylafax/mailman/message/58751879/ .
That fork is also suitable for gscan2pdf,
so the commit at hand changes the libtiff
version for gscan2pdf to use that fork.
This reverts commit f57a4b0.

The old version libtiff_4_5 is no longer needed.
Both dependents (gscan2pdf and hylafaxplus)
have switched to the forked libtiff version 4.6.0t
which is based on the current libtiff version 4.6.0
but also contains required command line tools
missing in the original libtiff library.
@Yarny0 Yarny0 force-pushed the hylafax-update-libtiff-and-tools branch from 9ac909d to 87eabf9 Compare April 20, 2024 09:10
Copy link
Contributor

@imincik imincik left a comment

Choose a reason for hiding this comment

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

All comments are resolved. I am happy with this PR. Thank you !

@SuperSandro2000 SuperSandro2000 merged commit 241d162 into NixOS:master Apr 21, 2024
26 checks passed
@Yarny0 Yarny0 deleted the hylafax-update-libtiff-and-tools branch April 24, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants