Skip to content

tzdata: add libintl dependency on FreeBSD#313893

Closed
rhelmot wants to merge 1 commit intoNixOS:masterfrom
rhelmot:freebsd-minimal3/tzdata
Closed

tzdata: add libintl dependency on FreeBSD#313893
rhelmot wants to merge 1 commit intoNixOS:masterfrom
rhelmot:freebsd-minimal3/tzdata

Conversation

@rhelmot
Copy link
Contributor

@rhelmot rhelmot commented May 23, 2024

Description of changes

part of #296581

tzdata doesn't have a configure step. It instead has instructions to edit the make flags until it works. The makefile actually contains the explicit example of FreeBSD as a platform you have to explicitly add -lintl on!

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
    • x86_64-freebsd
  • 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.11 Release Notes (or backporting 23.11 and 24.05 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.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 23, 2024
Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

The makefile actually contains the explicit example of FreeBSD as a platform you have to explicitly add -lintl on!

Not 100%.

The Makefile for tzdata contains:

# Non-default libraries needed to link.
# On some hosts, this should have -lintl unless CFLAGS has -DHAVE_GETTEXT=0.
LDLIBS=

# Add the following to an uncommented "CFLAGS=" line as needed
# to override defaults specified in the source code or by the system.
[...]
#  -DHAVE_GETTEXT if gettext works (e.g., GNU/Linux, FreeBSD, Solaris),
#	where LDLIBS also needs to contain -lintl on some hosts;
#	-DHAVE_GETTEXT=0 to avoid using gettext

I can't judge the "why", i.e. whether FreeBSD is one of "those hosts", but the "how" seems good. But given that it fixed things for you, the "why" seems pretty close.

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

I think it might be good to have a libIntlSeparateFromLibc ? stdenv.hostPlatform.libc and use libIntlSeparateFromLibc in the two conditions?

Also, please add the description to the commit message

@rhelmot
Copy link
Contributor Author

rhelmot commented May 26, 2024

libintl is already defined as

# On non-GNU systems we need GNU Gettext for libintl.
libintl = if stdenv.hostPlatform.libc != "glibc" then gettext else null;

So I guess we can just make the dependency unconditional? Might want to change the default from null to stdenv.cc.libc, if I'm understanding the directions we would like to take for nixpkgs? :)

@Ericson2314
Copy link
Member

Yeah I am surprised it built on darwin, but I guess they include some libintl.

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Hmm, I just cross compiled this without any changes, and it built fine. Are we sure we need this?

@rhelmot
Copy link
Contributor Author

rhelmot commented May 27, 2024

EXCELLENT catch! I did some forensics and found that this is only because of the way the FreeBSD bootstrap tools work, and if I override it for just that one stdenv it works fine.

It turns out that tzdata is doing quite a bit of self-configuration at build time through use of the __has_include macro, which I didn't know existed until today!

@rhelmot rhelmot closed this May 27, 2024
@rhelmot rhelmot deleted the freebsd-minimal3/tzdata branch May 27, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants