-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix "make install" with DESTDIR set #16995
base: master
Are you sure you want to change the base?
Conversation
"DESTDIR=/path/to/target/root/ make install" may fail when installing to a root that contains an existing lib/modules structure. When run as root we may even affect the wrong kernel (the build system's one, or, if running a different version, some other directory in /lib/modules, but not the desired one installed in DESTDIR). Add a missing reference to the INSTALL_MOD_PATH root when calling "depmod" during "make install" Also add a switch "DONT_DELETE_MODULES_FILES=1" that skips the removal of files named "modules.*" prior to running depmod. Signed-off-by: Christian Kohlschütter <[email protected]> Closes openzfs#16994
ba6ea6e
to
44e9f97
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I've had a similar workaround (the -b
part) for over a year for my local dev VMs. I have no opinion on the "don't delete" flag other than its useful and nonintrusive.
@@ -93,7 +93,7 @@ modules_install-Linux: modules_uninstall-Linux-legacy | |||
$(if @KERNEL_ARCH@,ARCH=@KERNEL_ARCH@) \ | |||
KERNELRELEASE=@LINUX_VERSION@ | |||
@# Remove extraneous build products when packaging | |||
if [ -n "$(DESTDIR)" ]; then \ | |||
if [ -n "$(DESTDIR)" ] && [ "$(DONT_DELETE_MODULES_FILES)" != "1" ]; then \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fairly sure we added this originally to avoid adding extraneous files to deb and rpm packages. I'm fine with not changing the behavior for now, but maybe the thing to do is flip this login around and set a flag so we only purge this extra files when building a package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless you have strong objections, I'd suggest flipping this logic at a later point. This will allow the fix to get merged now, without causing any unexpected side-effects. The logic flip would have to be communicated with integrators, so it's probably best to do that aligned with a major release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it's worth, I test built RPMs with both vanilla master
and with this PR, and the RPM filelists were the same.
"DESTDIR=/path/to/target/root/ make install" may fail when installing to a root that contains an existing lib/modules structure. When run as root we may even affect the wrong kernel (the build system's one, or, if running a different version, some other directory in /lib/modules, but not the desired one installed in DESTDIR).
Add a missing reference to the INSTALL_MOD_PATH root when calling "depmod" during "make install"
Also add a switch "DONT_DELETE_MODULES_FILES=1" that skips the removal of files named "modules.*" prior to running depmod.
Signed-off-by: Christian Kohlschütter [email protected]
Closes #16994
Motivation and Context
Fixes an important build issue when installing to DESTDIR.
see issue #16994
Description
In addition to the details in #16994:
I'm sure there's a reason why the current makefile deletes files starting with "modules.*", so I'm only adding a switch that can disable that functionality when not desired.
With this change, I can now install zfs nto my target root as follows:
How Has This Been Tested?
Tested on Alpine Linux 3.21.2 using the following zfs build instructions
Types of changes
Checklist:
Signed-off-by
.