bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Oleg Pykhalov]

[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]

Hello Guix,

Currently, if aspell is installed in GuixOS system profile and not in
user profile then aspell cannot find dictionaries.

(define-public aspell
  (package
    ;; …
    (arguments
     `(#:phases
       (modify-phases %standard-phases
         (add-after 'install 'wrap-aspell
           (lambda* (#:key outputs #:allow-other-keys)
             (let ((bin/aspell (string-append (assoc-ref outputs "out")
                                              "/bin/aspell")))
               (wrap-program bin/aspell
                 '("ASPELL_CONF" "" =
                   ("${ASPELL_CONF:-\"dict-dir ${GUIX_PROFILE:-$HOME/.guix-profile}/lib/aspell\"}")))))))))
    ;; …
    ))

I have a solution on top of existing wrapper, but what is a good way to
implement the following?

--8<---------------cut here---------------start------------->8---
LIB_ASPELL=lib/aspell
HOME_PROFILE=$HOME/.guix-profile
SYSTEM_PROFILE=/run/current-system/profile

if [ -z ${GUIX_PROFILE} ]; then
    if [ -d $HOME_PROFILE/$LIB_ASPELL ]; then
        GUIX_PROFILE=$HOME_PROFILE
    elif [ -d $SYSTEM_PROFILE/$LIB_ASPELL ]; then
        GUIX_PROFILE=$SYSTEM_PROFILE
    fi
fi

export ASPELL_CONF="${ASPELL_CONF:-"dict-dir ${GUIX_PROFILE}/lib/aspell"}"
--8<---------------cut here---------------end--------------->8---

Thanks,
Oleg.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Ludovic Courtès]

Hello,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> Currently, if aspell is installed in GuixOS system profile and not in
> user profile then aspell cannot find dictionaries.

If you install both aspell and aspell-dict-* in the global profile, then
ASPELL_DICT_DIR will point to /run/current-system/profile/lib/aspell
instead of ~/.guix-profile/lib/aspell.  Likewise:

--8<---------------cut here---------------start------------->8---
$ guix environment --ad-hoc aspell aspell-dict-ru coreutils -C -- env | grep ASPELL

[...]

ASPELL_DICT_DIR=/gnu/store/hssihv9d5cq5hzh0kiwdwiv6qcrc921c-profile/lib/aspell
--8<---------------cut here---------------end--------------->8---

So I’m not sure we need a custom fix, do we?  :-)

Ludo’.

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Ludovic Courtès]

Hi Oleg,

ludo@gnu.org (Ludovic Courtès) skribis:

> Oleg Pykhalov <go.wigust@gmail.com> skribis:
>
>> Currently, if aspell is installed in GuixOS system profile and not in
>> user profile then aspell cannot find dictionaries.
>
> If you install both aspell and aspell-dict-* in the global profile, then
> ASPELL_DICT_DIR will point to /run/current-system/profile/lib/aspell
> instead of ~/.guix-profile/lib/aspell.  Likewise:
>
> $ guix environment --ad-hoc aspell aspell-dict-ru coreutils -C -- env | grep ASPELL
>
> [...]
>
> ASPELL_DICT_DIR=/gnu/store/hssihv9d5cq5hzh0kiwdwiv6qcrc921c-profile/lib/aspell
>
> So I’m not sure we need a custom fix, do we?  :-)

Oleg, can we close this bug?

Ludo’.

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Oleg Pykhalov]

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

Hello Ludovic,

Apologies for late reply.  I thought how to reproduce it properly.

ludo@gnu.org (Ludovic Courtès) writes:

>>> Currently, if aspell is installed in GuixOS system profile and not in
>>> user profile then aspell cannot find dictionaries.
>>
>> If you install both aspell and aspell-dict-* in the global profile, then
>> ASPELL_DICT_DIR will point to /run/current-system/profile/lib/aspell
>> instead of ~/.guix-profile/lib/aspell.  Likewise:
>>
>> $ guix environment --ad-hoc aspell aspell-dict-ru coreutils -C --
>> env | grep ASPELL
>>
>> [...]
>>
>> ASPELL_DICT_DIR=/gnu/store/hssihv9d5cq5hzh0kiwdwiv6qcrc921c-profile/lib/aspell
>>
>> So I’m not sure we need a custom fix, do we?  :-)
>
> Oleg, can we close this bug?

No,

--8<---------------cut here---------------start------------->8---
guix environment --ad-hoc aspell aspell-dict-ru coreutils -C -- aspell dicts
--8<---------------cut here---------------end--------------->8---

returns nothing, but need to return a list like

    en
    en-variant_0
    en-variant_1
    en-variant_2
    en-w_accents
    en-wo_accents
    en_AU
    en_AU-variant_0
    …

Thanks,
Oleg.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Ludovic Courtès]

Hi Oleg,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>>>> Currently, if aspell is installed in GuixOS system profile and not in
>>>> user profile then aspell cannot find dictionaries.
>>>
>>> If you install both aspell and aspell-dict-* in the global profile, then
>>> ASPELL_DICT_DIR will point to /run/current-system/profile/lib/aspell
>>> instead of ~/.guix-profile/lib/aspell.  Likewise:
>>>
>>> $ guix environment --ad-hoc aspell aspell-dict-ru coreutils -C --
>>> env | grep ASPELL
>>>
>>> [...]
>>>
>>> ASPELL_DICT_DIR=/gnu/store/hssihv9d5cq5hzh0kiwdwiv6qcrc921c-profile/lib/aspell
>>>
>>> So I’m not sure we need a custom fix, do we?  :-)
>>
>> Oleg, can we close this bug?
>
> No,
>
> guix environment --ad-hoc aspell aspell-dict-ru coreutils -C -- aspell dicts
>
> returns nothing, but need to return a list like
>
>     en
>     en-variant_0
>     en-variant_1
>     en-variant_2
>     en-w_accents
>     en-wo_accents
>     en_AU
>     en_AU-variant_0
>     …

Indeed.  I have:

--8<---------------cut here---------------start------------->8---
$ guix environment --ad-hoc aspell aspell-dict-ru -C -- aspell --config | grep -A3 dict-dir
# dict-dir (string)
#   location of the main word list
# default: <$ASPELL_DICT_DIR|home-dir/.guix-profile/lib/aspell> = /gnu/store/b411b0fbwc85fjnf0h030gdkbv1qdmqh-profile/lib/aspell
dict-dir /home/ludo/.guix-profile/lib/aspell

# encoding (string)
#   encoding to expect data to be in
--
# default: <actual-dict-dir> = /home/ludo/.guix-profile/lib/aspell/

# master (string)
#   base name of the main dictionary to use
--8<---------------cut here---------------end--------------->8---

The value that shows up after the “=” sign is as expected (it’s
$ASPELL_DICT_DIR), but the value shown on the line below is not.

Sounds like a problem in ‘aspell-dict-dir.patch’.

Ludo’.

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 2761 bytes --]

Hi Oleg & Pierre,

ludo@gnu.org (Ludovic Courtès) skribis:

> Indeed.  I have:
>
> $ guix environment --ad-hoc aspell aspell-dict-ru -C -- aspell --config | grep -A3 dict-dir
> # dict-dir (string)
> #   location of the main word list
> # default: <$ASPELL_DICT_DIR|home-dir/.guix-profile/lib/aspell> = /gnu/store/b411b0fbwc85fjnf0h030gdkbv1qdmqh-profile/lib/aspell
> dict-dir /home/ludo/.guix-profile/lib/aspell
>
> # encoding (string)
> #   encoding to expect data to be in
> --
> # default: <actual-dict-dir> = /home/ludo/.guix-profile/lib/aspell/
>
> # master (string)
> #   base name of the main dictionary to use
>
> The value that shows up after the “=” sign is as expected (it’s
> $ASPELL_DICT_DIR), but the value shown on the line below is not.
>
> Sounds like a problem in ‘aspell-dict-dir.patch’.

Actually the problem is the ‘wrap-aspell’ phase, which predates
‘aspell-dict-dir.patch’ and is unnecessary in the presence of this
patch.

In addition to removing the phase, the patch had to be adjusted so that
it would actually adjust the right-hand size of ‘|’ for ‘dict-dir’ (a
bug that apparently went unnoticed, probably because the ‘wrap-aspell’
phase was hiding it).

With the patch below, I get:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix environment --ad-hoc aspell aspell-dict-ru -C -- aspell --config |grep -A2 '^# dict-dir'
# dict-dir (string)
#   location of the main word list
# default: <$ASPELL_DICT_DIR|$HOME/.guix-profile/lib/aspell> = /gnu/store/adfc4l73nw3ccai3h5im21wblmrhfy4z-profile/lib/aspell
--8<---------------cut here---------------end--------------->8---

and:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix environment --ad-hoc aspell aspell-dict-ru -C -- aspell dicts
ru
ru-ye
ru-yeyo
ru-yo
--8<---------------cut here---------------end--------------->8---

… while still getting the intended result when running it outside ‘guix
environment’:

--8<---------------cut here---------------start------------->8---
$ (unset ASPELL_DICT_DIR;  ./pre-inst-env guix environment --ad-hoc aspell -K -- aspell --config) |grep -A2 '^# dict-dir'
# dict-dir (string)
#   location of the main word list
# default: <$ASPELL_DICT_DIR|$HOME/.guix-profile/lib/aspell> = /home/ludo/.guix-profile/lib/aspell
$ (unset ASPELL_DICT_DIR;  ./pre-inst-env guix environment --ad-hoc aspell -K -- aspell dicts) |wc -l
58
$ (unset HOME;  ./pre-inst-env guix environment --ad-hoc aspell -K -- aspell dicts) |wc -l
58
--8<---------------cut here---------------end--------------->8---

I’ll apply it on ‘core-updates’ if it’s fine with you.

Ludo’.


[-- Attachment #2: Type: text/x-patch, Size: 2890 bytes --]

diff --git a/gnu/packages/aspell.scm b/gnu/packages/aspell.scm
index af38c149c2..a5d02a453d 100644
--- a/gnu/packages/aspell.scm
+++ b/gnu/packages/aspell.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013, 2014, 2015, 2017, 2018 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013, 2014, 2015, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2016 John Darrington <jmd@gnu.org>
 ;;; Copyright © 2016, 2017, 2019 Efraim Flashner <efraim@flashner.co.il>
@@ -65,14 +65,6 @@
                  (("\"filter-path(.*)DICT_DIR" _ middle)
                   (string-append "\"filter-path" middle
                                  "\"" libdir "\"")))
-               #t)))
-         (add-after 'install 'wrap-aspell
-           (lambda* (#:key outputs #:allow-other-keys)
-             (let ((bin/aspell (string-append (assoc-ref outputs "out")
-                                              "/bin/aspell")))
-               (wrap-program bin/aspell
-                 '("ASPELL_CONF" "" =
-                   ("${ASPELL_CONF:-\"dict-dir ${GUIX_PROFILE:-$HOME/.guix-profile}/lib/aspell\"}")))
                #t))))))
     (inputs `(("perl" ,perl)))
 
diff --git a/gnu/packages/patches/aspell-default-dict-dir.patch b/gnu/packages/patches/aspell-default-dict-dir.patch
index 17a6ff606f..5f759a66d9 100644
--- a/gnu/packages/patches/aspell-default-dict-dir.patch
+++ b/gnu/packages/patches/aspell-default-dict-dir.patch
@@ -8,12 +8,34 @@ However it is necessary for applications that use libaspell since
 
 --- a/common/config.cpp
 +++ b/common/config.cpp
+@@ -651,7 +651,20 @@ namespace acommon {
+ 	  } else { // sep == '|'
+ 	    assert(replace[0] == '$');
+ 	    const char * env = getenv(replace.c_str()+1);
+-	    final_str += env ? env : second;
++	    if (env) {
++	      final_str += env;
++	    } else if (second[0] == '$') {
++	      // Expand the right-hand side of '|', which starts with a
++	      // reference to an environment variable.
++	      auto slash = second.find('/');
++	      String variable = second.substr(1, slash - 1);
++	      const char * env = getenv(variable.c_str());
++	      String value = env ? String(env) : "";
++	      value += second.substr(slash, second.size() - slash);
++	      final_str += value;
++	    } else {
++	      final_str += second;
++	    }
+ 	  }
+ 	  replace = "";
+ 	  in_replace = false;
 @@ -1349,6 +1349,9 @@ namespace acommon {
  #  define REPL     ".aspell.<lang>.prepl"
  #endif
  
 +#undef DICT_DIR
-+#define DICT_DIR "<$ASPELL_DICT_DIR|home-dir/.guix-profile/lib/aspell>"
++#define DICT_DIR "<$ASPELL_DICT_DIR|$HOME/.guix-profile/lib/aspell>"
 +
    static const KeyInfo config_keys[] = {
      // the description should be under 50 chars

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 95 bytes --]

Looks good to me, thanks for working on this!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Ludovic Courtès]

Hi,

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Looks good to me, thanks for working on this!

Pushed as 4554d6b77e0f306a3dc33a8c1f68c9a302967a08 (core-updates).

Thank you!

Ludo’.

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

I was hit by this as well, when installing dictionaries to  the
non-default profile.

As Ludo pointed out:

--8<---------------cut here---------------start------------->8---
$ aspell config
...

# dict-dir (string)
#   location of the main word list
# default: <$ASPELL_DICT_DIR|home-dir/.guix-profile/lib/aspell> = /home/ambrevar/.guix-extra-profiles/default/default/lib/aspell
dict-dir /home/ambrevar/.guix-profile/lib/aspell
...
--8<---------------cut here---------------end--------------->8---

Something may be wrong with our patch:

--8<---------------cut here---------------start------------->8---
+#undef DICT_DIR
+#define DICT_DIR "<$ASPELL_DICT_DIR|home-dir/.guix-profile/lib/aspell>"
+
--8<---------------cut here---------------end--------------->8---

I had a quick glimpse at Aspell's source code but nothing obvious came
out :p

Maybe ask upstream?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Pierre Neidhardt]

[-- Attachment #1.1: Type: text/plain, Size: 169 bytes --]

I just noticed that Nix has a different, possibly better approach to
this in the attached patch.

What do you think?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: data-dirs-from-nix-profiles.patch --]
[-- Type: text/x-patch, Size: 1181 bytes --]

diff --git a/common/info.cpp b/common/info.cpp
index 8291cc7..6216326 100644
--- a/common/info.cpp
+++ b/common/info.cpp
@@ -36,6 +36,7 @@
 #include "strtonum.hpp"
 #include "lock.hpp"
 #include "string_map.hpp"
+#include "file_util.hpp"
 
 #include "gettext.h"
 
@@ -495,6 +496,25 @@ namespace acommon {
     lst.clear();
     lst.add(config->retrieve("data-dir"));
     lst.add(config->retrieve("dict-dir"));
+    if (config->lookup("data-dir") == NULL && config->lookup("dict-dir") == NULL) {
+        const char* cprofiles = getenv("NIX_PROFILES");
+        if (cprofiles != NULL) {
+            char* profiles = strdup(cprofiles);
+            char* profile = profiles;
+            char* end = profile;
+            while (*end != '\0') {
+                if (*end == ' ') {
+                    *end = '\0';
+                    lst.add(add_possible_dir(profile, "lib/aspell"));
+                    profile = ++end;
+                } else {
+                    ++end;
+                }
+            }
+            lst.add(add_possible_dir(profile, "lib/aspell"));
+            free(profiles);
+        }
+    }
   }
 
   DictExt::DictExt(ModuleInfo * m, const char * e)

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Ludovic Courtès]

Hi,

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> I just noticed that Nix has a different, possibly better approach to
> this in the attached patch.
>
> What do you think?

I think that honoring an ‘ASPELL_DICT_DIR’ variable is more generic than
browsing a set of profiles (which ones?), especially with support for
search paths as available in Guix.  :-)

Ludo’.

bug#29686: gnu: aspell: Do not ignore GuixOS system profile.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> I think that honoring an ‘ASPELL_DICT_DIR’ variable is more generic than
> browsing a set of profiles (which ones?), especially with support for
> search paths as available in Guix.  :-)

The Nix patch code could be modified to browse ASPELL_DICT_DIR, which
isn't done at the moment in our package.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]