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

Fix dkms header install #2776

Closed
wants to merge 2 commits into from

Conversation

tomprince
Copy link
Contributor

New versions of dkms clean up the build directory after installing.

It appears that this was always intended, but had rm -rf "/path/to/build/*" (note the quotes), which prevented it from working.

This also depends on openzfs/spl#399

@tomprince tomprince force-pushed the fix-dkms-header-install branch from 607c17a to feaedbb Compare October 9, 2014 17:47
@ryao
Copy link
Contributor

ryao commented Oct 9, 2014

@behlendorf I have tested this in a fresh Fedora 20 VM and I am confident that this fix is correct. Feel free to add your pick of my Reviewed-by:/Signed-off-by: from either my @gentoo.org or work email addresses.

In particular, this changes the build system to move the autotools products and headers from /var/lib/dkms/spl/0.6.3/build to /var/lib/dkms/spl/0.6.3/3.16.3-200.fc20.x86_64/x86_64 right before the clean is done. That places them outside of harms way on the current Fedora Core 20 and ZoL release. That avoids a problem entirely. The second patch fixes a mistake when passing paths to make the build system look for the spl sources at /usr/src/spl-0.6.3 instead of /var/lib/dkms/spl/0.6.3/build on the current ZoL release. Note that I am using actual paths rather than ones with variables to provide more context on what is being done.

@behlendorf
Copy link
Contributor

@ryao Thanks guys for the quick patch and review. Let me get this tested for EL7 and merged. I'll almost certainly update the public repositories with a point fix too.

@behlendorf behlendorf added this to the 0.6.4 milestone Oct 9, 2014
@ryao
Copy link
Contributor

ryao commented Oct 9, 2014

@behlendorf The dkms package in the EPEL repository has not been yet updated with the upstream change, so it is presently unaffected. That will likely change in the future.

@behlendorf behlendorf closed this in fee48fd Oct 9, 2014
behlendorf pushed a commit that referenced this pull request Oct 9, 2014
Signed-off-by: Tom Prince <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #2776
@behlendorf
Copy link
Contributor

@ryao Yes, in fact only FC20 has thus far gotten the updated packages. I'm working on pushing a point release for the stable repositories now. With a little luck it will be in place before EPEL picks up the new DKMS.

behlendorf pushed a commit that referenced this pull request Oct 9, 2014
New versions of dkms clean up the build directory after installing.

It appears that this was always intended, but had rm -rf "/path/to/build/*"
(note the quotes), which prevented it from working.

Also, the build step is already installing stuff into the directory where
these files go, so installing our stuff there as part of build rather than
install makes sense.

Signed-off-by: Tom Prince <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #2776
behlendorf pushed a commit that referenced this pull request Oct 9, 2014
Signed-off-by: Tom Prince <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #2776
@behlendorf
Copy link
Contributor

@ryao @tomprince the public fc 18,19,20 and el 6, 7 stable repositories have been updated with a point release which contains this fix. Full source can be found in the following branches. Thanks guys.

https://github.com/zfsonlinux/spl/tree/spl-0.6.3-stable
https://github.com/zfsonlinux/zfs/tree/zfs-0.6.3-stable

behlendorf added a commit to behlendorf/zfs that referenced this pull request Oct 13, 2014
The source_tree variable in the previous commit had an extra $.
Remove it so that source_tree is expanded properly.  An identical
fix has been applied in the original patch to the stable branch.

Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#2776
@tomprince
Copy link
Contributor Author

@behlendorf Is there any chance of getting new RPMs in the zfs-testing repository, with this fix?

1 similar comment
@tomprince
Copy link
Contributor Author

@behlendorf Is there any chance of getting new RPMs in the zfs-testing repository, with this fix?

@behlendorf
Copy link
Contributor

@tomprince absolutely. In fact I thought this was already done. I've updated the repository accordingly.

ryao pushed a commit to ryao/zfs that referenced this pull request Nov 29, 2014
New versions of dkms clean up the build directory after installing.

It appears that this was always intended, but had rm -rf "/path/to/build/*"
(note the quotes), which prevented it from working.

Also, the build step is already installing stuff into the directory where
these files go, so installing our stuff there as part of build rather than
install makes sense.

Signed-off-by: Tom Prince <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#2776
ryao pushed a commit to ryao/zfs that referenced this pull request Nov 29, 2014
Signed-off-by: Tom Prince <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#2776
ryao pushed a commit to ryao/zfs that referenced this pull request Nov 29, 2014
The source_tree variable in the previous commit had an extra $.
Remove it so that source_tree is expanded properly.  An identical
fix has been applied in the original patch to the stable branch.

Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#2776
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.

3 participants