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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gettext: remove cross suffix from envHook #50881

Closed
wants to merge 1 commit into from

Conversation

hedning
Copy link
Contributor

@hedning hedning commented Nov 21, 2018

Motivation for this change

role_post is meant for cross compilation, eg. CC_FOR_BUILD and
CC_FOR_TARGET. There's no such concept for GETTEXTDATADIRS.

In particular we hit this with cantarell-fonts, where only
GETTEXTDATADIRS_FOR_BUILD is defined due to empty buildInputs.

see #50855

cc @Ericson2314 as this was introduced in 2110c0b

`role_post` is meant for cross compilation, eg. `CC_FOR_BUILD` and
`CC_FOR_TARGET`. There's no such concept for `GETTEXTDATADIRS`.

In particular we hit this with `cantarell-fonts`, where only
`GETTEXTDATADIRS_FOR_BUILD` is defined due to empty buildInputs.

see NixOS#50855
@Ericson2314
Copy link
Member

Ericson2314 commented Nov 21, 2018

The short answer is our gettext needs to be extended with this concept, rather than abandoning it. Don't worry, it should just be a simple wrapper script. I'm surprised that build even uses gettext since so binaries are produced. That should be investigated too. There may also be an opportunity to patch the fonts to fox the same issue. (I'm also improving cross support for meson, but as gettext doesn't seem to be automatically handled by it, at least in this case without C code (right?), I don't that that will help just yet.)

@Ericson2314
Copy link
Member

Ericson2314 commented Nov 21, 2018

I can get to fixing all that this weekend, perhaps

@hedning
Copy link
Contributor Author

hedning commented Nov 21, 2018

gettext is being used to produce some appstream data: https://hydra.nixos.org/build/84539462/log
which includes translations:

cat $(nix eval nixpkgs.cantarell-fonts --raw)/share/appdata/org.gnome.cantarell.metainfo.xml

@matthewbauer
Copy link
Member

Does it really matter how gettext data is built? Isn’t it the same everywhere?

@Ericson2314
Copy link
Member

Ericson2314 commented Nov 21, 2018

It's not how the data is built, it's what it's used for. 99% gettext is just used in the outputs, I.e runtime stuff. Here, since it's a build input, it's used for a build time thing, which is confusing. Gettext is also confused because there is a library component and executable component. That executable component is acting as a depsBuildBuild here.

@Mic92
Copy link
Member

Mic92 commented Nov 21, 2018

It would be good to maintain a list of common packages in our documentation and where they belong (nativeBuildInputs vs. buildInputs) and why to make things easier for new people.

@matthewbauer
Copy link
Member

I think we could make a flow chart on this. But basically it comes down to:

  • if it’s used as a library then it goes in buildInputs.
  • if it’s used as a compiler then it goes in nativeBuildInputs to produce cross binaries. depsBuildBuild to produce native binaries.
  • if it’s executed by the builder, then it goes in nativeBuildInputs.
  • all setup hooks should go in nativeBuildInputs.
  • if it’s an interpreter that will be needed by an installed script, it should go in buildInputs. otherwise it is probably only needed at build time and can go in nativeBuildInputs.

These are not mutually exclusive though. In some cases we will need more than one version of a software (native built vs. cross built).

@Ericson2314
Copy link
Member

Ericson2314 commented Nov 21, 2018

These are less clear than the FAQ, but actually exhaustive.

  • If it is used at build-time it's depsBuildXXX
  • If it is used at run-time it's depsHostXXX. [Stack linking doesn't effect this, even if it allows us to forget where things came from.]
  • If it is a tool and "acts" (e.g. helps build) on build-time stuff, then it's depsXXXBuild
  • If it is a tool and "acts" on run-time stuff, then it's depsXXXHost
  • if it is not a tool, it's depsXXX(XXX+1) (build + 1 == host, host +1 == target)
  • for backwards compatibility, use nativeBuildInputs instead of depsBuildHost and buildInputs instead of depsHostTarget.

This is in fact what the cross complication section of the manual says, in longer terms that no one wants to read. I have no problem with the convenience FAQ, but in the odd cases people have to full back on the full principles. strictDeps will help by forcing people to do the right thing in the native case so if they blindly debug, it comes out correctly.

@Ericson2314
Copy link
Member

gettext is both a tool and a library (libintl). It current follows the rules from the library's perspective, due to the way it is built. Maybe that should be changed, especially since glibc includes libintl.

@eburimu
Copy link
Contributor

eburimu commented Nov 22, 2018

I believe a note on pkgconfig should be added somewhere. If pkgconfig is used just for compiling (ie autoconf), it should go into nativeBuildInputs. On the other hand, if it is a library which generates .pc, than it should go into propagatedNativeBuildInputs because there will be a ref to pkgconfig.

@Mic92
Copy link
Member

Mic92 commented Nov 23, 2018

I summarized it in the wiki: https://nixos.wiki/wiki/Cross_Compiling#How_to_specify_dependencies

@Mic92
Copy link
Member

Mic92 commented Nov 23, 2018

Are there modernized equivalents to propagatedNativeBuildInputs and propagatedBuildInputs?

@matthewbauer
Copy link
Member

Those should be okay to use still. There was an issue in the disallowedReferences logic that means you really can’t use it on propagated inputs. All propagated inputs end up in the nix-support directory due to how the propagation logic works. Eventually i would like to handle propagation in Nix eval but it’s not a huge prio.

@Ericson2314
Copy link
Member

@Mic92 the modernized equivalences don't actually exist, as I didn't want to create two ways to do the same thing / start a renaming battle right off the bat. But they would be, respectively, depsBuildHostPropagated and depsHostTargetPropagated. (That Propagated suffix naming scheme is used for all the deps* I did actually create.)

@matthewbauer
Copy link
Member

I believe a note on pkgconfig should be added somewhere. If pkgconfig is used just for compiling (ie autoconf), it should go into nativeBuildInputs. On the other hand, if it is a library which generates .pc, than it should go into propagatedNativeBuildInputs because there will be a ref to pkgconfig.

I don't think it's worth propagating pkgconfig. There are multiple ways to resolve libraries, so just because a package supports pkgconfig, doesn't mean it won't also support .cmake or plain old LDFLAGS.

@Ericson2314
Copy link
Member

Ericson2314 commented Nov 28, 2018

Yes that's true. propagatedNativeBuildInputs in general is tool that imposes perhaps too many opinions downstream.

@mmahut
Copy link
Member

mmahut commented Aug 19, 2019

Are there any updates on this pull request, please?

@stale
Copy link

stale bot commented Jun 1, 2020

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
@SuperSandro2000
Copy link
Member

Closing due to inactivity. Feel free to reopen the discussion.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/use-buildinputs-or-nativebuildinputs-for-nix-shell/8464/2

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

9 participants