gtk: search immodules.cache in $NIX_PROFILE

[ericsagnes]

This a fix for #14019 with a different approach than #14417.
See #14417 comments for details.

This PR triggers the rebuild of gtk+3 and gtk+2 and all the packages that depends on them.

Description

GTK im module cache

GTK application need to know the input method on the system to be able to use them.
For that purpose, gtk uses a cache file (immodule.cache) that list all the paths to the input method library of the matching gtk version (this will be important later).

The cache has to be updated everytime a new input method is installed or removed on the system by running the command gtk-query-immodules-*.

The default cache path of gtk is in the gtk package lib/ folder, making it incompatible with nix immutable packages.

There are a few ways to override the default immodules.cache file,
the default logic is (before the patch):

For gtk2

1. Look at GTK_IM_MODULE_FILE, if present use it
2. Look at gtkrc file im_module_file setting, if present use it
3. Use the default cache file that is inside gtk2 package lib/gtk-2.0/2.10.0/immodules.cache

For gtk3 (all the logic is deprecated and just for backward compatibility)

1. Look at GTK_IM_MODULE_FILE, if present use it
2. Look at gtkrc file im_module_file setting, if present use it
3. Use the default cache file that is inside gtk3 package lib/gtk-3.0/3.0.0/immodules.cache

Problems with Nix

So the problems that we are facing with Nix are:

• default cache file location is in an immutable package -> we cannot update it, so we cannot use it
• using GTK_IM_MODULE_FILE to specify a ad-hoc cache file is fine, but the problem is gtk2 and gtk3 will use the same file, and every entry must point the input method library matching the gtk version
  -> we cannot make gtk2 and gtk3 work at the same time

Solutions

The first approach (#14417) was to patch gtk3 to make it look for GTK3_IM_MODULE_FILE and use it if present, that allowed to have 2 different cache files, GTK3_IM_MODULE_FILE for gtk3 and GTK_IM_MODULE_FILE for gtk2.

There was a problem though, that is if a package installed via nix-env and not updated after a nixos-rebuild will still read GTK_IM_MODULE_FILE that point to an input method gtk2 library, that would result in the application crashing.

From there, @ttuegel suggested that we could use NIX_PROFILES as it was done for Qt.

That lead us to the current implementation based on NIX_PROFILES.

So the new logic is (after the patch):

For gtk2

1. Look at GTK_IM_MODULE_FILE, if present use it
2. New: Look at NIX_PROFILES, and for each entry, check if etc/gtk-2.0/immodules.cache exists, use if present (the last entry having the file will have priority) [1]
3. Look at the im_module_file setting in the gtkrc file, if present use it
4. Use the default cache file that is inside gtk2 package lib/gtk-2.0/2.10.0/immodules.cache

For gtk3 (all the logic is deprecated and just for backward compatibility)

1. Look at GTK_IM_MODULE_FILE, if present use it
2. New: Look at NIX_PROFILES, and for each entry, check if etc/gtk-3.0/immodules.cache exists, use if present (the last entry having the file will have priority) [1]
3. Look at the im_module_file setting in the gtkrc file, if present use it (the code is here, but I am not even sure that gtk3 is looking for a gtkrc file in the first place)
4. Use the default cache file that is inside gtk3 package lib/gtk-3.0/3.0.0/immodules.cache

[1] Regarding NIX_PROFILES

The NIX_PROFILES variable is defined by reversing the list of entries in the environment.profiles.
If not changed, the order is /run/current-system/sw /nix/var/nix/profiles/default $HOME/.nix-profile, from generic to specific.

The order seems arbitrary (in the meaning that there is no mention about it in the option description) and can be changed.
But we can consider that few users are changing it, and if they do we can just hope that they respect the specific -> generic order (NIX_PROFILES use the reversed list of environment.profiles).

Anyway, in a normal use case there shouldn't be a cache file anywhere else than /run/current-system/sw as it is generated by the module system.
If there is a cache in more that one location of the $NIX_PROFILE, the user did it on purpose and in my opinion, we can consider that he knows what he is doing.

Things done

• Tested using sandboxing (nix-build --option build-use-chroot true or nix.useChroot on NixOS)
• Built on platform(s)
  • NixOS
  • OS X
  • Linux
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

cc @ttuegel @taku0 @astsmtl @abbradar

---

• 2014-04-16: rebased on master to fix trivial conflicts in pkgs/development/libraries/gtk+/3.x.nix.

[mention-bot]

By analyzing the blame information on this pull request, we identified @taku0, @ttuegel and @urkud to be potential reviewers

[abbradar]

Looks good to me and this solution also sounds better. I'll wait for others to test it just in case but if I understand correctly this shouldn't break any setups anymore.

[gebner]

This leaks memory in a few places:

• paths, just add a g_strfreev(paths) at the end.
• cachePath needs to be freed as well.
• What should happen when more than one profile contains the etc/gtk-3.0/immodules.cache directory?

(When running programs in valgrind to check for memory leaks, it is nice if there are no false positives from system libraries. Or more realistically, as few as possible.)

[ttuegel]

What should happen when more than one profile contains the etc/gtk-3.0/immodules.cache directory?

In that case, the entry that occurs last in NIX_PROFILES should be used. IIUC, that's what this patch does, but my C is rusty. 😄

[gebner]

Then all the previous entries will be leaked, three times in total: first because of the allocation in g_strsplit, then g_build_filename, and finally g_strdup--they all allocate their own copy.

Aside from that, shouldn't the environment variable GTK_IM_MODULE_FILE take precedence if it is set?

[ericsagnes]

@gebner Thanks for the memory leak information, I added g_strfreev(paths); and g_free(cachePath);, is that enough to fix it?

Aside from that, shouldn't the environment variable GTK_IM_MODULE_FILE take precedence if it is set?

It will, as the check for the GTK_IM_MODULE_FILE is after this patch logic and update result.

The code before patching is:

gchar *
gtk_rc_get_im_module_file (void)
{
  const gchar *var = g_getenv ("GTK_IM_MODULE_FILE");
  gchar *result = NULL;

// patch comes here

  if (var)
    result = g_strdup (var);

  if (!result)
    {
      if (im_module_file)
    result = g_strdup (im_module_file);
      else
        result = gtk_rc_make_default_dir ("immodules.cache");
    }

  return result;
}

PS: I have almost no C experience so if you can write a better patch I will be very happy to use it instead :)

[gebner]

Thanks for the memory leak information, I added g_strfreev(paths); and g_free(cachePath);, is that enough to fix it?

Almost, there is still a leak when 1) there is more than one nix profile containing the immodule.cache file, and 2) if the environment variable GTK_IM_MODULE_FILE is set.

I think the easiest fix is to:

1. Move the NIX_PROFILES check below the original one and change the if to if (nixProfilesEnv && !result).
2. Free the result variable before it is overwritten, i.e. if (result) g_free(result) right before the assignment.

That should fix all of the memory leaks.

[ericsagnes]

Done, thanks again for all the advice!

[ericsagnes]

@taku0 Did you had time to test this PR?

Also, maybe it would be better to add the mass-rebuild label to avoid this to get merged by inadvertence.

[cstrahan]

Just to see if I'm reading this right:

I have:

$ echo $NIX_PROFILES
/run/current-system/sw /nix/var/nix/profiles/default /home/cstrahan/.nix-profile

So this would takes the last $path such that $path/etc/gtk-$version/immodules.cache exists?

[cstrahan]

This might not be a big concern, but there is this to keep in mind:

~/.mutt (master) λ bash -c "echo \$NIX_PROFILES"
/run/current-system/sw /nix/var/nix/profiles/default /home/cstrahan/.nix-profile

~/.mutt (master) λ sudo bash -c "echo \$NIX_PROFILES"

I would wonder if it would make sense to first check a variable along the lines of $GTK3_IM_MODULE_FILE, and if that environment variables doesn't exist, default to /etc/gtk-3.0/immodules.cache. In NixOS, we wouldn't need to export $GTK3_IM_MODULE_FILE, since things would still work by default. Then, if someone wants to override the cache (or they're on another distro), they can do so explicitly.

How does that sound?

[ericsagnes]

@cstrahan Thanks for the label! I updated the description so the issue will be easier to understand for persons that are not following it from #14417 and #14019.

So this would takes the last $path such that $path/etc/gtk-$version/immodules.cache exists?

Correct. That was unclear so I added the details in the issue description.

I would wonder if it would make sense to first check a variable along the lines of $GTK3_IM_MODULE_FILE

$GTK3_IM_MODULE_FILE was problematic, so it was abandoned in favor of the NIX_PROFILES. (details are in the second point of Problems with Nix in the issue description or the previous PR #14417)

~/.mutt (master) λ sudo bash -c "echo $NIX_PROFILES"

Are you using NixOS or Nix only? I find that NIX_PROFILES being not set for root quite surprising. My understanding was that nixos-rebuild has to be run as root and so it will generate NIX_PROFILES for root too, but I might be wrong.

[cstrahan]

$GTK3_IM_MODULE_FILE was problematic, so it was abandoned in favor of the NIX_PROFILES. (details are in the second point of Problems with Nix in the issue description or the previous PR #14417)

Could you quote the portion you're referring to?

Are you using NixOS or Nix only? I find that NIX_PROFILES being not set for root quite surprising. My understanding was that nixos-rebuild has to be run as root and so it will generate NIX_PROFILES for root too, but I might be wrong.

NixOS. sudo clears the existing environment variables, and because I'm not starting bash as a login shell, that environment variable does not get set. As opposed to:

$ sudo bash -i -c "echo \$NIX_PROFILES"
/run/current-system/sw /nix/var/nix/profiles/default /root/.nix-profile

[cstrahan]

... but my original example was to show that running sudo some-gtk-app might not behave as expected, due to $NIX_PROFILES being unset.

[ericsagnes]

Could you quote the portion you're referring to?

Sorry, the 6 comments starting from this one.

...and broke pasystray. Backtrace shows that pasystray's GTK+3 loads UIM plugin for GTK+2 and gets SIGABRT later. We need to test and fix upgrade path for GTK+3 apps before merging this.

Wild guess -- maybe you use pasystray from nix-env without having it rebuilt? This patch would break all such software that uses old (unpatched) GTK+3.

---

sudo clears the existing environment variables,

I see, that was something I was unaware of, thanks for explaining.
It is a fair concern. Right now I cannot think of a good way to address it.
We can add a note in the manual explaining that when running gtk app as root the user should use gksu or gksudo -k.

[ttuegel]

It is a fair concern. Right now I cannot think of a good way to address it.
We can add a note in the manual explaining that when running gtk app as root the user should use gksu or gksudo -k.

In general, one should not expect any GUI program, GTK or otherwise, NixOS or otherwise, to work with sudo. All of the toolkits and X itself rely far too heavily on environment settings. This is compounded by the fact that everything mysteriously depends on D-Bus nowadays 😃

[astsmtl]

Any objections to merge this?

[ericsagnes]

@astsmtl Personally, I think it is ready to merge.
It would be nice to have confirmation that it solves @taku0 problem with uim, and maybe also to gather a few more opinions to be sure it is the approach we want to stick with before merging.

Speaking of that, I noticed in the manual that mass rebuild PR should be made against staging, should I close this one and open a new PR against staging?

[abbradar]

This looks fine to me and I'm willing to merge it to staging manually if others agree it's ready. @cstrahan @ttuegel @taku0?

[ttuegel]

@abbradar I'm fine with this going into staging.

[taku0]

uim still has the problem but I'm fine with merging since it is not critical.

[ericsagnes]

@cstrahan Are you fine with this being merged in current state?

For the record there was another PR trying to fix what this one is fixing (#15254) today, so it would be nice if we could merge this soon.

[abbradar]

Ugh, I got immersed in other things to do and forgot this one. I'll merge in ~12 hours unless someone votes nay.

On May 6, 2016 12:24:41 PM GMT+03:00, Eric Sagnes notifications@github.com wrote:

@cstrahan Are you fine with this being merged in current state?

For the record there was another PR trying to fix what this one is
fixing (#15254) today, so it would be nice if we could merge this soon.

---

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#14568 (comment)

Nikolay.

[abbradar]

Closed by c99b050

[ericsagnes]

Thanks!

[sorpaas]

Hi @abbradar, I still cannot use fcitx on GTK after applying this commit. What should I do to debug?

[abbradar]

Please tell us:

1. stdout output;
2. Your configuration (IME-related);
3. Whether only fcitx doesn't work or all the GTK.

[ericsagnes]

@sorpaas After applying the patch did you rebuild your configuration with nixos-rebuild switch -I nixpkgs=path/to/the/patched/nixpkgs?

This commit is quite heavy as it will trigger the source build of gtk2 and gtk3 and all their reverse dependencies, thing that can keep the cpu busy many hours on a normal machine.
So in my opinion, it is better to wait it to reach a channel so we can use the binary cache.

In the mean time, if you want to use fcitx you can:

• revert a91161a
• use xim like you proposed in input-method module: use xim for GTK Fcitx #15254
• use a version of nixpkgs anterior to a91161a

All of these should be quite cheap and trigger only a few rebuilds.

Sorry for the inconvenience, I will update and close #14019 as soon as I see this commit reaching the unstable channel.

[sorpaas]

Hi, I managed to fix this issue. I turned out to be some mis-configurations of my user profile. Sorry about the noise!