Skip to content

Conversation

@bodgerer
Copy link
Contributor

@bodgerer bodgerer commented Jan 14, 2025

"dkms autoinstall" does not obey BUILD_DEPENDS (and presumably AUTOINSTALL) directives when placed in /etc/dkms/$module.conf, causing unexpected ordering and build failures.

The read_conf family of routines rely upon variables module and module_version (possibly others?) being already suitably set. Arguably they shouldn't, but this is a minimal change that corrects autoinstall's BUILD_DEPENDS processing.

There may be other places where this is needed.

Tested against dkms 3.1.4

"dkms autoinstall" does not obey BUILD_DEPENDS (and presumably
AUTOINSTALL) directives when placed in /etc/dkms/$module.conf, causing
unexpected ordering and build failures.

The read_conf family of routines rely upon variables module and
module_version (possibly others?) being already suitably set. Arguably
they shouldn't, but this is a minimal change that corrects autoinstall's
BUILD_DEPENDS processing.

There may be other places where this is needed.

Tested against dkms 3.1.4
@scaronni scaronni self-assigned this Jan 15, 2025
@scaronni scaronni merged commit 695028a into dkms-project:master Jan 15, 2025
26 checks passed
@scaronni
Copy link
Member

I think these are the only two variables required, as the function needs to run while iterating over $m + $v. It's also the only case where that function is called in a loop.

Thanks for the merge request!

@bodgerer
Copy link
Contributor Author

bodgerer commented Jan 15, 2025 via email

@bac0n
Copy link

bac0n commented Mar 8, 2025

I think parsing of /etc/dkms/*.conf should unset the complete array before trying assign new directives, as of now you have to match the number of array members as it assigns array members by index (basically merging), or you can get really odd behavior, e.g., overriding BUILT_MODULE_NAME or some other array, isn't really possible. It's couple of rev. behind, but I rewrote safe_source to unset the directives (arrays) before declaring them (I think it's a more sane/expected bevavior):

safe_source(){
    # $1 = file to source
    # $@ = environment variables to echo out
    mapfile -t < <(
        local -a "${@:2}"
        . "$1" > /dev/null 2>&1
        for e in "${@:2}"; do
            for i in $(eval echo "\${!$e[@]}"); do
                v=${e}[$i]
                printf '%s[%s]="%s"\n' "$e" "$i" "${!v}"
            done
        done
    )

    unset "${MAPFILE[@]%%[[=]*}" && \
    eval "${MAPFILE[@]}" || {
        echo "Error! Could not parse .conf directives." >&2
        return 1
    }
    # ....
}

@anbe42
Copy link
Collaborator

anbe42 commented Mar 10, 2025

@bac0n Please file a new issue or PR, otherwise this comment on a closed PR tends to get forgotten ... Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants