I'm a fedora packager - @negativo17 would you entertain a patch to dkms for fedora to implement the logic Gregory suggests above?  This would seem to be a bit at odds with the current packaging that assumes the presence of grubby.  I've forked the dkms package repo and would be happy to work on a mutually agreeable solution if you're short of time.

(In reply to Martin Jackson from comment #1)
> I'm a fedora packager - @negativo17 would you entertain a patch to dkms for
> fedora to implement the logic Gregory suggests above?  This would seem to be
> a bit at odds with the current packaging that assumes the presence of
> grubby.  I've forked the dkms package repo and would be happy to work on a
> mutually agreeable solution if you're short of time.

Hi Martin, absolutely. You can also push to Github and create a merge request on the official project, I have commit rights and can help you upstreaming it.

https://github.com/dell/dkms

Thanks.

I have submitted a PR and issue to the DKMS project:  https://github.com/dell/dkms/issues/119.  Once we work through that upstream we can work on the packaging in Fedora.

Thanks!

I've submitted https://github.com/dell/dkms/pull/118 upstream.  My intention, if the PR is accepted, to submit a follow-up PR to install the script that would now be upstream as /etc/kernel/install.d/40-dkms.install in F30+ and EL8 in the DKMS packaging.  The PR to DKMS upstream is currently pending review/merge.

Sounds great to me with maybe one caveat ... I think packagers are *supposed* to place their versions of their scripts under /usr so that, if so inclined, end users can override them by placing their own versions under /etc. I think the *proper* place for the packaged version of 40-dkms.install would be under /usr/lib/kernel/install.d, not /etc/kernel/install.d.

Just my two cents.

Thanks again for all your work in getting this patch moved forward.

(In reply to Martin Jackson from comment #4)
> I've submitted https://github.com/dell/dkms/pull/118 upstream.  My
> intention, if the PR is accepted, to submit a follow-up PR to install the
> script that would now be upstream as /etc/kernel/install.d/40-dkms.install
> in F30+ and EL8 in the DKMS packaging.  The PR to DKMS upstream is currently
> pending review/merge.

Merged, thanks for the PR and sorry for the delay. Regarding the packaging, beside the follow-up PR to the dkms package: if you are already a Fedora contributor I can give you full access to the dkms package. If not and you would like to become a contributor I can sponsor you.

(In reply to Gregory Lee Bartholomew from comment #5)
> Sounds great to me with maybe one caveat ... I think packagers are
> *supposed* to place their versions of their scripts under /usr so that, if
> so inclined, end users can override them by placing their own versions under
> /etc. I think the *proper* place for the packaged version of 40-dkms.install
> would be under /usr/lib/kernel/install.d, not /etc/kernel/install.d.

Are we sure a script with the same name in /etc overrides the one in /usr/lib? Otherwise we can add the script to /etc and mark it as config in the RPM spec file so it's not overwritten during package upgrades.

(In reply to Simone Caronni from comment #6)
> Are we sure a script with the same name in /etc overrides the one in
> /usr/lib? Otherwise we can add the script to /etc and mark it as config in
> the RPM spec file so it's not overwritten during package upgrades.

With just a quick glance at /bin/kernel-install it appears so:

...

dropindirs_sort()
{
    local suffix=$1; shift
    local -a files
    local f d i

    readarray -t files <<<"$(
        for d in "$@"; do
            for i in "$d/"*"$suffix"; do
                if [[ -e "$i" ]]; then
                    echo "${i##*/}"
                fi
            done
        done | sort -Vu
    )"

    for f in "${files[@]}"; do
        for d in "$@"; do
            if [[ -e "$d/$f" ]]; then
                echo "$d/$f"
                continue 2
            fi
        done
    done
}

...

readarray -t PLUGINS <<<"$(
    dropindirs_sort ".install" \
        "/etc/kernel/install.d" \
        "/usr/lib/kernel/install.d"
)"

...

I think the "contine 2" line will skip the second directory -- /usr/lib/kernel/install.d -- if a file by the same name exists in /etc/kernel/install.d.

I am a fedora contributor - FAS userid mhjacks (I'm new; eclipseo sponsored me about a month and a half ago).  I would be happy to be a contributor to the DKMS package, and once I get access we can pursue the work there.

I'm open to installing this either in /usr or /etc - maybe it would be more appropriate to install it in /usr?  The script isn't really a config file, so maybe it belongs in the /usr dropin dir after all.  Let me know if you feel very differently.

Thanks!

Is it appropriate to ask for a DKMS upstream release here?  It would simplify packaging, I think.  Thanks!

(In reply to Martin Jackson from comment #8)
> I am a fedora contributor - FAS userid mhjacks (I'm new; eclipseo sponsored
> me about a month and a half ago).  I would be happy to be a contributor to
> the DKMS package, and once I get access we can pursue the work there.

Hi Martin, I've added you as an admin in dkms.

> I'm open to installing this either in /usr or /etc - maybe it would be more
> appropriate to install it in /usr?  The script isn't really a config file,
> so maybe it belongs in the /usr dropin dir after all.  Let me know if you
> feel very differently.

Please check the latest commits:
https://src.fedoraproject.org/rpms/dkms/commits/master

Script is in /usr. As a personal preference please make sure that branches are in sync (makes my life much more easier). The package needs to work on RHEL 7/8 and Fedora (I stopped backporting things to RHEL 6 sometime ago).

(In reply to Martin Jackson from comment #9)
> Is it appropriate to ask for a DKMS upstream release here?  It would
> simplify packaging, I think.  Thanks!

For now I just added the %tag definition. Some information, ignore if it's redundant for you:

To download a release:

- Set "%global tag 1"
- Set version in the spec file (do not run rpmdev-bumpspec -n or it removes the conditional in the release tag)
- Run "rpmdev-bumpspec dkms.spec", changelog uses the release
- Run "spectool -g dkms.spec" to get the tarball
- Run "fedpkg new-sources tarball" to upload
- Commit

To use a snapshot:
- Set the commit to the commit id
- Set the date to the date of the commit
- Run "rpmdev-bumpspec dkms.spec", changelog uses the git snapshot format
- Run "spectool -g dkms.spec" to get the tarball
- Run "fedpkg new-sources tarball" to upload
- Commit

Otherwise feel free :)

Please note that fc32 has been branched, so rawhide is fc33.

Thanks!  I will work on updating the package appropriately for all currently supported releases, and I hope to get the first update into testing this weekend.  I'm planning to check which ones have install.d and install the script there in /usr; if el7 doesn't have it (for example) I'll conditionalize the install in %files.

OK, I've checked and I don't think I have any privileges on rpms/dkms - I don't see myself in members on the repo and bodhi won't let me submit updates to it.  I have tested the epel7 build on the 2.8.1-snapshot version and it installs cleanly - I'd be happy to sync that same version across f31 (which needs it), f30 and el8 (where it will work fine with or without grubby) and el7 (which doesn't really need it but doesn't look like it will hurt anything).

(In reply to Martin Jackson from comment #11)
> Thanks!  I will work on updating the package appropriately for all currently
> supported releases, and I hope to get the first update into testing this
> weekend.  I'm planning to check which ones have install.d and install the
> script there in /usr; if el7 doesn't have it (for example) I'll
> conditionalize the install in %files.

el7 has it.

(In reply to Martin Jackson from comment #12)
> OK, I've checked and I don't think I have any privileges on rpms/dkms - I
> don't see myself in members on the repo and bodhi won't let me submit
> updates to it.

I've just checked, you are not part of the packager group. Are you sure eclipseo sponsored you? Do you have some link where i can see the status? (package reviews, etc.). Otherwise I can sponsor you.

On the package I've set you as an admin, that's the highest you can get in privileges for a particular package.

> I have tested the epel7 build on the 2.8.1-snapshot version
> and it installs cleanly - I'd be happy to sync that same version across f31
> (which needs it), f30 and el8 (where it will work fine with or without
> grubby) and el7 (which doesn't really need it but doesn't look like it will
> hurt anything).

Not following here, I'd just say from a quick glance the package should work as is down to el7.

Thanks for looking into this further!

I'm pretty sure I am in the packager group:
Here's my introduction of git-up in bodhi:  https://bodhi.fedoraproject.org/users/mhjacks.  My FAS account shows be as belonging to the packagers group, and I own both rpms/git-up and rpms/pipx on src.fedoraproject.org; I'm also an admin of rpms/nagios-plugins-check-updates (which I've pushed updates to).

git-up, pipx and nagios-plugins-check-updates all show up in my dashboard on src.fedoraproject.org; dkms doesn't.  I'm admin on nagios-plugins-check-updates; owner on the other two.

dkms only shows you and praveenp as members.  I don't doubt you've done things and looked at things.  Should we file a ticket?

As far as epel7 packaging - I agree.  I don't think any changes are needed.  I wasn't sure if the new file would "get in the way" but it doesn't seem to.  (I'm still learning my way around RPM packaging.)

I see the f31/f30 updates as most pressing (on f31 in particular this causes problems for ZFS users, and I'm sure others); I agree that epel7 and epel8 should also be updated (there are no problem reports on those versions, but it's easier if all OS's are at the same level; a future el8 might make grubby optional and leave el8 users in the same position f31 users are in now).  I would be happy to push those updates, but I am not sure what to do about the access question.

OK, that's odd. Now I see you in the packager group and your permission disappeared in dkms. I've readded it. Can you check again?

Thanks.

FEDORA-2020-b5edd2ddf1 has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2020-b5edd2ddf1

dkms-2.8.1-3.20200214git5ca628c.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-843dbc8775

dkms-2.8.1-3.20200214git5ca628c.fc31 has been pushed to the Fedora 31 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-b5edd2ddf1

Hi Martin:

I just tested dkms-2.8.1-3.20200214git5ca628c.fc31, but it failed to build the zfs module because /usr/lib/kernel/install.d/40-dkms.install does not have the execute bits set. Line 134 of /bin/kernel-install requires that the script be marked executable:

...

        for f in "${PLUGINS[@]}"; do
            if [[ -x $f ]]; then
                [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \
                    echo "+$f add $KERNEL_VERSION $ENTRY_DIR_ABS $KERNEL_IMAGE ${INITRD_OPTIONS[@]}"
                "$f" add "$KERNEL_VERSION" "$ENTRY_DIR_ABS" "$KERNEL_IMAGE" "${INITRD_OPTIONS[@]}"
                x=$?
                if [[ $x == $SKIP_REMAINING ]]; then
                    ret=0
                    break
                fi
                ((ret+=$x))
            fi
        done

...

FEDORA-2020-d437098f26 has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2020-d437098f26

FEDORA-2020-42dbcf8d17 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2020-42dbcf8d17

FEDORA-EPEL-2020-e3eb34379f has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-e3eb34379f

FEDORA-EPEL-2020-34c412d66a has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-34c412d66a

The updates have been re-submitted.  I validated it both removes the old modules (from the kernel it's removing) and builds the new new modules (for the kernel it's installing).  I'm dreadfully sorry for the mixup - please re-test with the -4 version of the package if you can.  Thanks again for testing and reporting the issue!

dkms-2.8.1-4.20200214git5ca628c.el8 has been pushed to the Fedora EPEL 8 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-e3eb34379f

dkms-2.8.1-4.20200214git5ca628c.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2020-34c412d66a

dkms-2.8.1-4.20200214git5ca628c.fc31 has been pushed to the Fedora 31 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-d437098f26

dkms-2.8.1-4.20200214git5ca628c.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-42dbcf8d17

Hi Martin:

I just tested dkms-2.8.1-4.20200214git5ca628c.fc31 on a basic Fedora 31 installation and it seems to be working now.

Thanks again for your work in getting this long-standing bug finally fixed in Fedora!

dkms-2.8.1-4.20200214git5ca628c.fc31 has been pushed to the Fedora 31 stable repository. If problems still persist, please make note of it in this bug report.

dkms-2.8.1-4.20200214git5ca628c.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.

dkms-2.8.1-4.20200214git5ca628c.el8 has been pushed to the Fedora EPEL 8 stable repository. If problems still persist, please make note of it in this bug report.

dkms-2.8.1-4.20200214git5ca628c.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.