Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 901205 - Allow to specify modules by path
Summary: Allow to specify modules by path
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Hosted Projects
Classification: Unclassified
Component: eselect (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Gentoo eselect Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-14 12:01 UTC by Florian Schmaus
Modified: 2023-03-20 17:27 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments
0001-Add-ESELECT_EXTRA_MODULE_PATH-variable.patch (0001-Add-ESELECT_EXTRA_MODULE_PATH-variable.patch,1.10 KB, patch)
2023-03-14 12:01 UTC, Florian Schmaus
Details | Diff
0001-Add-update-action-to-kernel-module.patch (0001-Add-update-action-to-kernel-module.patch,2.47 KB, patch)
2023-03-14 12:04 UTC, Florian Schmaus
Details | Diff
0001-Allow-to-specify-modules-by-path.patch (0001-Allow-to-specify-modules-by-path.patch,846 bytes, patch)
2023-03-14 14:26 UTC, Florian Schmaus
Details | Diff
0001-Allow-to-specify-modules-by-path.patch (0001-Allow-to-specify-modules-by-path.patch,935 bytes, patch)
2023-03-14 15:00 UTC, Florian Schmaus
Details | Diff
0001-Allow-to-specify-modules-by-path.patch (0001-Allow-to-specify-modules-by-path.patch,934 bytes, patch)
2023-03-14 16:23 UTC, Florian Schmaus
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Schmaus gentoo-dev 2023-03-14 12:01:28 UTC
Created attachment 857649 [details, diff]
0001-Add-ESELECT_EXTRA_MODULE_PATH-variable.patch

Add support for an variable that allows to prepend to ESELECT_MODULES_PATH for developing purposes.

Branch also at https://gitlab.gentoo.org/flow/eselect/-/tree/extra-module-path
Comment 1 Florian Schmaus gentoo-dev 2023-03-14 12:04:51 UTC
Created attachment 857653 [details, diff]
0001-Add-update-action-to-kernel-module.patch
Comment 2 Ulrich Müller gentoo-dev 2023-03-14 13:43:42 UTC
(In reply to Florian Schmaus from comment #0)
> Created attachment 857649 [details, diff] [details, diff]
> 0001-Add-ESELECT_EXTRA_MODULE_PATH-variable.patch

I am slightly confused. The above looks like the intended patch?


(In reply to Florian Schmaus from comment #1)
> Created attachment 857653 [details, diff] [details, diff]
> 0001-Add-update-action-to-kernel-module.patch

This is the same as the patch attached to bug 901209 comment #0?


Anyway, eselect has been moving away from environment variables (e.g. ROOT), so I'm somewhat skeptical on introducing new ones. In the case of the module path it may even have security implications. Also, we already have ~/.eselect/modules/ for user-specific modules.

How about the following instead? It would allow to specify an absolute path for the module:

--- a/libs/core.bash.in
+++ b/libs/core.bash.in
@@ -69,6 +69,14 @@
 # Find module and echo its filename. Die if module doesn't exist.
 find_module() {
        local modname=$1 modpath
+
+       if [[ ${modname} == */* ]]; then
+               [[ ${modname} == *.eselect && -f ${modname} ]] \
+                       || die -q "Can't load module ${modname}"
+               echo "${modname}"
+               return
+       fi
+
        for modpath in "${ESELECT_MODULES_PATH[@]}"; do
                if [[ -f ${modpath}/${modname}.eselect ]]; then
                        echo "${modpath}/${modname}.eselect"
Comment 3 Florian Schmaus gentoo-dev 2023-03-14 14:25:28 UTC
(In reply to Ulrich Müller from comment #2)
> (In reply to Florian Schmaus from comment #0)
> > Created attachment 857649 [details, diff] [details, diff] [details, diff]
> > 0001-Add-ESELECT_EXTRA_MODULE_PATH-variable.patch
> I am slightly confused. The above looks like the intended patch?

No, I attached the wrong patch. :/

> How about the following instead? It would allow to specify an absolute path
> for the module:

That is also fine. In fact, it is even better. FWIW, I believe it also allows to specify relative paths for the module, as long as the path contains a slash. However, I believe we can allow simply specifying the module by path, be it an absolute or a relative one, by simply checking if modname ends with '.eselect' and if it is a valid path to a file. This would allow to simplify the code to

diff --git a/libs/core.bash.in b/libs/core.bash.in
index 740354e2748b..3d1f0e58b3e1 100644
--- a/libs/core.bash.in
+++ b/libs/core.bash.in
@@ -69,6 +69,12 @@ die() {
 # Find module and echo its filename. Die if module doesn't exist.
 find_module() {
    local modname=$1 modpath
+
+   if [[ ${modname} == *.eselect && -f ${modname} ]]; then
+       echo "${modname}"
+       return
+   fi
+
    for modpath in "${ESELECT_MODULES_PATH[@]}"; do
        if [[ -f ${modpath}/${modname}.eselect ]]; then
            echo "${modpath}/${modname}.eselect"
Comment 4 Florian Schmaus gentoo-dev 2023-03-14 14:26:17 UTC
Created attachment 857675 [details, diff]
0001-Allow-to-specify-modules-by-path.patch
Comment 5 Ulrich Müller gentoo-dev 2023-03-14 14:40:14 UTC
(In reply to Florian Schmaus from comment #3)
> +   if [[ ${modname} == *.eselect && -f ${modname} ]]; then

Let's be extra careful and explicitly require a slash (if the module is in the current directory, the user will have specify it as ./foo.eselect). That is, the test should be:

[[ ${modname} == */*.eselect && -f ${modname} ]]

> +       echo "${modname}"
> +       return
> +   fi
Comment 6 Ulrich Müller gentoo-dev 2023-03-14 14:43:48 UTC
Thinking about it, I still prefer my patch from comment #2. If there's a / in modname, then it should immediately error out when the module is not found, and not proceed into the loop below.
Comment 7 Florian Schmaus gentoo-dev 2023-03-14 15:00:29 UTC
Created attachment 857693 [details, diff]
0001-Allow-to-specify-modules-by-path.patch

How about this:

--- a/libs/core.bash.in
+++ b/libs/core.bash.in
@@ -69,6 +69,16 @@ die() {
 # Find module and echo its filename. Die if module doesn't exist.
 find_module() {
    local modname=$1 modpath
+
+   if [[ ${modname} == *.eselect && -f ${modname} ]]; then
+       echo "${modname}"
+       return
+   fi
+
+   if [[ ${modname} == */* ]]; then
+       die -q "Can't load module ${modname}"
+   fi
+
    for modpath in "${ESELECT_MODULES_PATH[@]}"; do
        if [[ -f ${modpath}/${modname}.eselect ]]; then
            echo "${modpath}/${modname}.eselect"
Comment 8 Ulrich Müller gentoo-dev 2023-03-14 15:20:47 UTC
(In reply to Florian Schmaus from comment #7)

I'd like the logic to be similar as for commands executed by the shell, with "." not in PATH. From Bash documentation:

| 3. If the name [...] contains no slashes, Bash searches each element of
| '$PATH' for a directory containing an executable file by that name. [...]
|
| 4. If the search is successful, or if the command name contains one or
| more slashes, the shell executes the named program [...]

This will leave no ambiguities. (Theoretically, someone could have a module foo.eselect.eselect in ESELECT_MODULES_PATH and a module foo.eselect in the current working dir.)
Comment 9 Florian Schmaus gentoo-dev 2023-03-14 16:23:29 UTC
Created attachment 857697 [details, diff]
0001-Allow-to-specify-modules-by-path.patch

Fair point, thanks for elaborating. Updated patch.
Comment 10 Larry the Git Cow gentoo-dev 2023-03-14 16:53:38 UTC Comment hidden (obsolete)
Comment 11 Larry the Git Cow gentoo-dev 2023-03-14 16:55:37 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/eselect.git/commit/?id=c7e76fc0b9add97eda8cf406cbd168a1997e9ae3

commit c7e76fc0b9add97eda8cf406cbd168a1997e9ae3
Author:     Florian Schmaus <flow@gentoo.org>
AuthorDate: 2023-03-14 14:23:07 +0000
Commit:     Ulrich Müller <ulm@gentoo.org>
CommitDate: 2023-03-14 16:54:40 +0000

    Allow to specify modules by path
    
    * libs/core.bash.in (find_module): Allow to specify an absolute
    path as the module's filename.
    
    Bug: https://bugs.gentoo.org/901205
    Signed-off-by: Florian Schmaus <flow@gentoo.org>
    Signed-off-by: Ulrich Müller <ulm@gentoo.org>

 ChangeLog         |  5 +++++
 libs/core.bash.in | 11 ++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)
Comment 12 Larry the Git Cow gentoo-dev 2023-03-20 17:27:47 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=015d7e1648ba32072eed926ee3009a3adf9037a1

commit 015d7e1648ba32072eed926ee3009a3adf9037a1
Author:     Ulrich Müller <ulm@gentoo.org>
AuthorDate: 2023-03-20 17:27:09 +0000
Commit:     Ulrich Müller <ulm@gentoo.org>
CommitDate: 2023-03-20 17:27:09 +0000

    app-admin/eselect: add 1.4.22
    
    Closes: https://bugs.gentoo.org/901205
    Closes: https://bugs.gentoo.org/901209
    Signed-off-by: Ulrich Müller <ulm@gentoo.org>

 app-admin/eselect/Manifest              |  1 +
 app-admin/eselect/eselect-1.4.22.ebuild | 58 +++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)