Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce 'dkms unbuild' command + related fixes #117

Merged
merged 10 commits into from
Feb 19, 2020

Conversation

evelikov
Copy link
Collaborator

This MR implements the "unbuild" action, as mentioned in #111

evelikov-work and others added 10 commits January 17, 2020 16:31
The commands require root access, otherwise we'll fail with errors like
the following. Add the explicit root check to avoid that.

mkdir: cannot create directory ‘/var/lib/dkms/nvidia’: Permission denied
ln: failed to create symbolic link '/var/lib/dkms/nvidia/435.21/source': No such file or directory

Signed-off-by: Emil Velikov <[email protected]>
Flesh out the --all check & error message, out of have_one_kernel()
We'll reuse the function, in other places which support multiple
kernel/arch combinations, yet --all is not supported nor does it make
sense.

Signed-off-by: Emil Velikov <[email protected]>
Using --all with add, simply cannot work: the presence of the module is
used to determine the list of "all" kernels/arches.

Technically it is possible to have --all working with build/install,
when the module is already added.

Doing that is a bit more involving than the time I have at the moment,
so for now explicitly error out. The option is not supported, as per the
manual.

Note: the option --all in not allowed with a few other actions. I've
left those out for now since they're not ones I could realistically test

Signed-off-by: Emil Velikov <[email protected]>
Use the check_module_args(), is_module_{added,built,install}() functions
instead of open-coding them. Actually create trivial wrappers around the
latter so we don't need to duplicate the lenghty error messages.

Note that kernels_arches_default and read_conf_or_die() are safe to
remove.

The former is a remnant from the times before check_module_args() was
introduced, while read_conf_or_die() is an explicit part of
is_module_built().

Signed-off-by: Emil Velikov <[email protected]>
Just like many other actions - we can use --all with 'uninstall' with a
trivial change.

Signed-off-by: Emil Velikov <[email protected]>
The argument $1 has been missing since the helper was introduced.

Signed-off-by: Emil Velikov <[email protected]>
The message talks about "number of arguments", which is incorrect.
Additionally actions like `dkms add` do not require the explicit module
and module-version.

State what we're after - the missing the module information.

Signed-off-by: Emil Velikov <[email protected]>
We'll reuse the function in the newcomming unbuild_module.

Signed-off-by: Emil Velikov <[email protected]>
The code executed is invariant of the kernel/arch combination. Thus we
can move it outside of the loop.

Signed-off-by: Emil Velikov <[email protected]>
Add an action "unbuild" to align with the existing "build" one.

This provides for a complete symmetry - add/remove, build/unbuild and
install/uninstall.

Apart from the seeming cosmetic affect, this function is required when
the binary kernel module(s) produced with "dkms build" must be pruned.

Without this command that is not possible since using:
 - "dkms uninstall" will remove the symlinks/copies produced with
"dkms install", while
 - "dkms remove" will erase all references of the dkms module. Hence
"dkms status" will have no references about the module.

This allows for a proper fix for dell#20
while avoiding regressions like dell#77

Signed-off-by: Emil Velikov <[email protected]>
@scaronni
Copy link
Collaborator

Looks fine to me.

Copy link
Contributor

@superm1 superm1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too

@evelikov
Copy link
Collaborator Author

Thanks everyone. Can anyone press the merge button :-)

@superm1 superm1 merged commit c4f3db2 into dell:master Feb 19, 2020
@eli-schwartz
Copy link

This breaks valid use cases due to the incorrect root check. See #134

@evelikov evelikov deleted the sent/unbuild branch October 10, 2021 02:22
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.

5 participants