Skip to content

tvheadend: update to 2026-08-17 and require GNU libiconv - #30346

Open
BKPepe wants to merge 3 commits into
openwrt:masterfrom
BKPepe:tvheadend-require-libiconv-full
Open

tvheadend: update to 2026-08-17 and require GNU libiconv#30346
BKPepe wants to merge 3 commits into
openwrt:masterfrom
BKPepe:tvheadend-require-libiconv-full

Conversation

@BKPepe

@BKPepe BKPepe commented Aug 21, 2026

Copy link
Copy Markdown
Member

📦 Package Details

Maintainer: none (the address in the Makefile belongs to a deleted
GitHub account)

Description:
Updates tvheadend to 2026-08-17 and makes it depend on libiconv-full
unconditionally instead of $(ICONV_DEPENDS), which only pulls it in when
BUILD_NLS is set.

tvheadend has no transliteration of its own. It asks iconv for
<charset>//TRANSLIT//IGNORE, a GNU extension musl does not implement, so
iconv_open() fails with EINVAL.

050-iconv-test-continue.patch removed the abort() from the startup
self-test that reports this. That silenced the message without fixing
anything, because cleanup_filename() in src/dvr/dvr_rec.c still cannot
build a recording file name. With GNU libiconv the self-test passes and the
patch is no longer needed.

An upstream fix, so that tvheadend degrades gracefully instead of aborting on
such iconv implementations, is proposed in tvheadend/tvheadend#2226. It is not
a substitute for this change. Without GNU libiconv there is still no
transliteration, only the original UTF-8 text.

One user-visible side effect: the version the binary reports changes from
<git hash>~openwrt<PKG_RELEASE> to the package version,
2026.08.17~ddb984f4. The OpenWrt release number is no longer part of it.
See the third commit for why the override was removed.


🧪 Run Testing Details

  • OpenWrt Version: SNAPSHOT r0+35801-d7a1df26be
  • OpenWrt Target/Subtarget: mpc85xx/p2020
  • OpenWrt Device: CZ.NIC Turris 1.x

✅ Formalities

  • I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

If your PR contains a patch:

This PR removes a patch and adds none, so the items below do not apply.

Copilot AI lite review requested due to automatic review settings August 21, 2026 07:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Update to git master ddb984f4, 124 commits on from the previously
packaged 395afb85. Most of that is web UI work and translation updates,
with smaller fixes in the build system, mpegts and dvr code.

Signed-off-by: Josef Schlehofer <[email protected]>
@BKPepe
BKPepe force-pushed the tvheadend-require-libiconv-full branch from 24fea08 to f64fad3 Compare August 21, 2026 08:16
@BKPepe
BKPepe force-pushed the tvheadend-require-libiconv-full branch 2 times, most recently from 7c8348a to 04dd2b7 Compare August 21, 2026 12:26

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 3 new commits. Nothing here blocks a merge — the three inline remarks are one optional cleanup and two nits.

The substance looks right. libiconv-full in openwrt core is not gated on CONFIG_BUILD_NLS, so depending on it unconditionally instead of $(ICONV_DEPENDS) is safe, and all ten arch jobs on 04dd2b7 are green with libiconv-full2 (1.18-r1) pulled in and "All linked libraries for /usr/bin/tvheadend are present". Dropping 050-iconv-test-continue.patch leaves the package with no patches at all, which is the right outcome given the self-test now passes rather than being silenced.

All three commit messages match what their diffs actually do, so no commit checks.


Generated by Claude Code

Comment on lines +25 to +27
ICONV_FULL_PREFIX:=$(STAGING_DIR)/usr/lib/libiconv-full
TARGET_CFLAGS += -I$(ICONV_FULL_PREFIX)/include
TARGET_LDFLAGS += -L$(ICONV_FULL_PREFIX)/lib -Wl,-rpath-link=$(ICONV_FULL_PREFIX)/lib

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional, non-blocking: nls.mk is still included above, and with CONFIG_BUILD_NLS=y it already appends the identical flags for the identical prefix — ICONV_CFLAGS/ICONV_LDFLAGS in include/nls.mk#L26-L28 resolve to -I$(STAGING_DIR)/usr/lib/libiconv-full/include and -L.../lib -Wl,-rpath-link=.../lib. So NLS builds get every flag twice. Harmless duplication, but wrapping these three lines in ifneq ($(CONFIG_BUILD_NLS),y) would keep the flag set identical in both configurations.

Related question while you are here: nls.mk also sets TARGET_CPPFLAGS (ICONV_CPPFLAGS), which this block does not. Does any tvheadend configure probe run the preprocessor without CFLAGS? If one does, it would still see musl's iconv.h rather than GNU's on a BUILD_NLS=n build. From the CI log the compile flags do carry the include path, so this looks fine — just confirming there is no preprocessor-only check.


Generated by Claude Code

+libffi \
+libopenssl \
$(ICONV_DEPENDS) \
+libiconv-full \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: the PR body says Maintainer: none, but this package does declare one a few lines above — MAINTAINER:=Marius Dinu <[email protected]> at multimedia/tvheadend/Makefile#L36. Since this change alters the dependency set and drops the only patch, the listed maintainer is probably worth a CC.


Generated by Claude Code

Comment on lines -255 to -260
define Build/Prepare
$(call Build/Prepare/Default)
echo 'Tvheadend $(shell echo $(PKG_SOURCE_VERSION) | sed "s/^v//")~openwrt$(PKG_RELEASE)' \
> $(PKG_BUILD_DIR)/debian/changelog
endef

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: this removal is a user-visible change — the binary's reported version goes from <git-hash>~openwrt<PKG_RELEASE> to 2026.08.17~ddb984f4, and the OpenWrt release number is no longer encoded in it — but neither the PR title ("update to 2026-08-17 and require GNU libiconv") nor the PR body mentions it at all. The commit message on 04dd2b7 explains it well; a sentence in the PR body would keep the three descriptions in agreement.

For what it's worth, the reasoning checks out in CI: the x86_64 job logs /usr/bin/tvheadend: version 2026.08.17~ddb984f4 and passes the generic version check, so support/version does fall back to the source directory name as the commit message claims.


Generated by Claude Code

BKPepe added 2 commits August 22, 2026 08:28
tvheadend has no transliteration of its own.  It asks iconv for
"<charset>//TRANSLIT//IGNORE", a GNU extension that musl does not
implement.  Converting "ŽluťoučkýKůň" to ASCII on mpc85xx/p2020
(Turris 1.x), musl 1.2.6:

    charset                   musl             GNU libiconv
    ASCII//TRANSLIT//IGNORE   EINVAL           "Zlutouck'yKun"
    ASCII//TRANSLIT           EINVAL           "Zlutouck'yKun"
    ASCII                     "*lu*ou*k*K**"   EILSEQ

050-iconv-test-continue.patch removed the abort() from the startup
self-test that reports this, so the package could be built against
musl's iconv.  That silenced the check without fixing anything:
intlconv_utf8() still returns -EIO, so cleanup_filename() in
src/dvr/dvr_rec.c cannot build a recording file name and
pvr_generate_filename() fails.  The charset cannot be avoided either,
because intlconv_filesystem_charset() returns "ASCII" unless $LANG
names a UTF-8 locale, which OpenWrt does not set by default.

Depend on libiconv-full instead of $(ICONV_DEPENDS), which pulls it in
only when BUILD_NLS is set, and point the compiler at its staging prefix
when nls.mk has not already done so.  The binary then links against
libiconv.so.2, the self-test passes and the patch can be dropped.

Signed-off-by: Josef Schlehofer <[email protected]>
The handwritten debian/changelog is only used to override the version
reported by support/version, but it is unnecessary and causes a version
mismatch:

- Build/Prepare feeds the raw git hash into debian/changelog, so the
  binary reports ddb984f4c17f4a26fd34f3e1b24e3323f9641b75~openwrt1
  instead of the package version 2026.08.17~ddb984f4.
- The CI package test then fails because it cannot find the package
  version in the output of any installed executable.
- Without .git, debian/changelog or rpm/version, support/version falls
  back to the source directory name.
- OpenWrt already names the source directory tvheadend-$(PKG_VERSION),
  so the binary correctly reports 2026.08.17~ddb984f4 without any
  override.

Alpine uses rpm/version in the same way and does not encode its own
package release into the version either.

Signed-off-by: Josef Schlehofer <[email protected]>
@BKPepe
BKPepe force-pushed the tvheadend-require-libiconv-full branch from 04dd2b7 to d0b7340 Compare August 22, 2026 06:32

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the 2 rewritten commits since 04dd2b7. The only content change is the new ifneq ($(CONFIG_BUILD_NLS),y) guard around the iconv flags; nothing here blocks a merge, the one inline remark is an optional indentation nit.

The guard is correct and mirrors include/nls.mk exactly — ICONV_PREFIX there resolves to the same $(STAGING_DIR)/usr/lib/libiconv-full, so the BUILD_NLS=y path now gets one copy of the flags instead of two, and the BUILD_NLS=n path is unchanged. It sits after the nls.mk include, so CONFIG_BUILD_NLS is defined, and nls.mk already appends CONFIG_BUILD_NLS to PKG_CONFIG_DEPENDS, so toggling NLS still forces a rebuild. All ten arch jobs plus the three formality checks are green on d0b7340.

The commit message on 005f355 was reworded in step with the code ("point the compiler at its staging prefix when nls.mk has not already done so"), and d0b7340 is unchanged from 04dd2b7. Both messages match their diffs, so no commit checks. The PR body also picked up the version-change note the earlier nit asked for.

Still open from the previous round, not re-posted inline: the TARGET_CPPFLAGS half of that threadnls.mk sets ICONV_CPPFLAGS on the BUILD_NLS=y side, this block sets none, so the two configurations are close but not identical. The green BUILD_NLS=n builds suggest nothing in tvheadend's configure runs a preprocessor-only probe, so this is likely a non-issue; flagging only so the thread is not lost.


Generated by Claude Code

Comment on lines +27 to +29
ICONV_FULL_PREFIX:=$(STAGING_DIR)/usr/lib/libiconv-full
TARGET_CFLAGS += -I$(ICONV_FULL_PREFIX)/include
TARGET_LDFLAGS += -L$(ICONV_FULL_PREFIX)/lib -Wl,-rpath-link=$(ICONV_FULL_PREFIX)/lib

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit (optional, does not affect the build): every other conditional body in this Makefile is indented with a tab — e.g. TARGET_CFLAGS := ... inside the TVHEADEND_OPTIMIZE_SPEED block at multimedia/tvheadend/Makefile#L77-L80 and --disable-pie at L72-L74. These three lines are flush left. Make does not care here (none of this is a recipe), so purely for consistency with the rest of the file:

Suggested change
ICONV_FULL_PREFIX:=$(STAGING_DIR)/usr/lib/libiconv-full
TARGET_CFLAGS += -I$(ICONV_FULL_PREFIX)/include
TARGET_LDFLAGS += -L$(ICONV_FULL_PREFIX)/lib -Wl,-rpath-link=$(ICONV_FULL_PREFIX)/lib
ICONV_FULL_PREFIX:=$(STAGING_DIR)/usr/lib/libiconv-full
TARGET_CFLAGS += -I$(ICONV_FULL_PREFIX)/include
TARGET_LDFLAGS += -L$(ICONV_FULL_PREFIX)/lib -Wl,-rpath-link=$(ICONV_FULL_PREFIX)/lib

Generated by Claude Code

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.

3 participants