Improve 'make' command substitution

[alsgnet]

'make' may not be necessary the first word in the build command line. If it is then 'make' substitution fails and '-j' option is not passed to the 'make' command. For example MAKE directive array may be declared as:

MAKE[0]="source my_environment && make ..."

It causes parallel build to be ignored. The patch imroves parsing of the build command and substitutes 'make' regardless its position in the command line.

Change-Id: Id7aefdb5db87821849d5785cdc4b86cb69c259c4

[evelikov]

Commit does what it says on the tin.

Although I am curious what is the use-case for having "source my_environment" as part of the make command. Does it have to happen at this point and not earlier - say before dkms is invoked? Does it need to exist in the first place - what does "my_environment" contain?

Generally, we've been moving away from having an shell logic, knowledge or generally code-flow in the conf files and treating them as pure data. A pie in the sky idea of mine is to deprecate "BUILD" and "CLEAN" with "MODDIR" and optionally "BUILD_ARGS", where dkms itself will issue $(MAKE) -C /lib/modules/$(kernel_ver)/build M=$(MODDIR) ${BUILD_ARGS) clean and friends.

Above said, I'm inclined to WONTFIX this, although I would be open to look at your module and provide some tips. If things are fairly convoluted I would be open to reconsider this PR.

[alsgnet]

I'm working on a distro that has vendor supplied kernels built with different compilers so I need a way to override the compiler toolchain. The driver build is called by invoking common.postinst script from a package. It is not manual build by calling the dkms script directly. The environment file (my_environment as it is in my example) is prepared by PRE_BUILD script by extracting compiler version string from a kernel that dkms is currently working on.

[evelikov]

Can you provide a minimal example that demonstrates the PRE_BUILD, environment and makefile?

Off the top of my head, I think #298 should make all that complexity go away, although without specific details that's just a guess.

[alsgnet]

Regardless the fact that a suggested patch may resolve a problem with building a module on a system where installed kernels built with different compilers the patch actually does what is says in the title. It is more robust to parse the command line than relay on a sub-string matching. So, I would split the goals in two:

1. My patch as an improvement
2. Use compiler collection in dkms as it is indicated in Workaround for activating rhel's software collection toolchain #299 with an attempt to implement it in Automatically detecting compiler & linker used by kernel #298

Answering your questions, here is a simple demonstration of one of the solutions to switch compilers with PRE_BUILD script as a helper. Let's use an example in #299 on Oracle Linux 8 with RHEL gcc toolset. PRE_BUILD may contain something like this:

for f in $(find /opt/rh -type f -a -name gcc); do if strings /boot/vmlinuz-$KERNELVER | grep -q "$($f --version | head -1)"; then . ${f%/*}/../../../enable break fi done echo "PATH=$PATH" >my_environment
and "my_environment" can be used in conjunction with make:

MAKE[0]=". my_environment && make ...

There is a bunch of variables that RHEL gcc toolset sets up in "enable" file but only PATH is needed to build a kernel module successfully. A Makefile you requested isn't important. It could be a simple as:

obj-m += hello.o
I briefly looked through #298 and I think the approach is OK but implementation isn't complete. The author is mostly focused on compiler detection logic but not on actual outcome of that detection. In #299 he suggested to create either links to "gcc" of use a wrapper in /usr/local/bin to make it work. Maybe it is OK for an individual developer but it is unacceptable for production use. Let's say if I'm serving some customers and I supply them with a packaged driver. What would they say if I requested them to create links to every gcc in the system prior to install my package? They would find another supplier, I guess.

I think the idea behind #298 implementation should be like iteration of all available compilers in the $PATH and setting PATH locally in dkms for the "make" based on detection instead of passing CC and LD. This logic is missing. I tried the patch in #298 and it works with a wrapper in /usr/local/bin. Without wrapper (or links) it doesn't pick the right gcc even if both are in the $PATH. Only the first found is used for all kernel in the system which is incorrect.

[alsgnet]

I'm not sure why text formatting is broken. Let me try again:

for f in $(find /opt/rh -type f -a -name gcc); do
    if strings /boot/vmlinuz-$KERNELVER | grep -q "$($f --version | head -1)"; then
        . ${f%/*}/../../../enable
        break
    fi
done

echo "PATH=$PATH" >my_environment

[anbe42]

The documentation says

KERNELRELEASE will be automatically appended to MAKE[#]. If you want to
suppress this behavior, you can quote the make command: 'make'.
The same holds for injecting a -j# option, but is not documented.

Does this behavior still work with the proposed patch?

[anbe42]

Ideally we should set MAKEFLAGS=-j$parallel_jobs instead of "injecting" that flag into the MAKE command. At least if MAKEFLAGS is unset. And we probably shouldn't mess around with parallelism at all if there is an outer make already in control of it.

Which leaves the question: How can I write a dkms.conf which ensures that the MAKE command gets run "sequentially", i.e. with something equivalent to -j1 to build a module with a horribly broken build system? This needs to override any -j option given to dkms or autodetected $parallel_jobs value.

Not that I have a concrete example in mind that needs this, but we shouldn't force parallelism without having a way to opt out. Defaulting to parallelism is a good idea. (Manually running dkms -j1 ... is not a way to opt out with dkms autoinstall etc. being called from various hook scripts.)

[alsgnet]

@anbe42 ,

Does this behavior still work with the proposed patch?

The patch doesn't change any current behavior. It only suggests to use more accurate parsing to drop a restriction that "make" has to be the first word in the command.

As for make options like "-j" or KERNELRELEASE=. In general it is not about options but a possibility to switch the compiler when building a dkms module in OS like Oracle 8 that uses different gcc for its installed by default kernels (4.18 and 5.15).

[TriMoon]

No matter if you guys decide if it is preferable to change the detection logic:
Please find a solution without adding awk as a dependency for the dkms package...

[alsgnet]

Why? To request such things you have to provide strong arguments that you didn't. Also, awk is already used by dkms script as well as many other tools such as grep, sed etc. Should dependencies be dropped on all of them?

[TriMoon]

@alsgnet Stating what you claim and providing proof of that are two different things...
I don't see awk as a dependency for dkms on my system...

• https://packages.ubuntu.com/mantic/dkms

[alsgnet]

@TriMoon , it is off-topic. Please raise your questions with a packages vendor which is Canonical in your case.

[TriMoon]

@alsgnet please press reset wait 30s then retry again.
Doesn't make any sense with repesct to what you wrote either doesn't it? same with your redirect to the biggest Linux distro maker...

Anyhow idc anymore what and how you code, might as well try python/lisp or even javascript 🤦‍♀️ 🖖

[evelikov]

Good questions @anbe42. I've extended #316 to cover your concerns - anyone feel free to take over that issue.

@alsgnet is the Oracle/RH toolchain your main driving factor for this or there are others?

[alsgnet]

@evelikov , for now yes. Currently my make command in the MAKE[] array is like I described:

MAKE[0]=""source my_environment && make ..."

but I have to re-pass "-jxxx" and KERNELRELEASE= to the "make" because substitution in dkms script doesn't work e.g:

MAKE[0]=""source my_environment && make -jxxx KERNELRELEASE= ..."

Generated by PRE_BUILD script "my_environment" file contains only PATH variable that could be either PATH=$PATH or PATH=/opt/rh/gcc-toolset-11/root/bin:$PATH depending on a kernel is being built.

[evelikov]

@alsgnet if you feel like tackling #316 that should resolve the parallelism problem. We can worry about other problems as they arise.

[anbe42]

What about

MAKE[0]="source my_environment && make ${parallel_jobs+-j$parallel_jobs} KERNELRELEASE=${kernelver} ..."

No need to reinvent the values, just use the variables provided by dkms in your custom command.

BTW, is there documentation which variables can be used in MAKE[0] etc?

[evelikov]

Using parallel_jobs would work, but it's a bit of a hack since it's an internal variable. We can change/remove the variable the future.

is there documentation which variables can be used in MAKE[0] etc?

There is none. At the moment one can use anything from env or the script-local in BUILD/PRE_BUILD/CLEAN, although we should restrict at some point.