contrib/initramfs: switch to automake #6761

loli10K · 2017-10-15T06:10:50Z

Description

Use automake to build initramfs scripts and hooks.

Motivation and Context

On my Debian systems i can't update-initramfs -u successfully with ZFS ./configured with custom --prefix=:

root@linux:/usr/src/zfs# update-initramfs -u
update-initramfs: Generating /boot/initrd.img-4.6.4+
Error: /sbin/zdb is not executable.
Error: /sbin/zpool is not executable.
Error: /sbin/zfs is not executable.
E: /usr/share/initramfs-tools/hooks/zfs failed with return 2.
update-initramfs: failed for /boot/initrd.img-4.6.4+ with 2.
root@linux:/usr/src/zfs# head ./config.log
This file contains any messages produced by compilers while
running configure, to aid debugging if configure makes a mistake.

It was created by zfs configure 0.7.0, which was
generated by GNU Autoconf 2.69.  Invocation command line was

  $ ./configure --prefix=/usr --sysconfdir=/etc --libdir=/lib64 --enable-debug --with-spl=/usr/src/spl

## --------- ##
## Platform. ##
root@linux:/usr/src/zfs# which zdb zpool zfs
/usr/sbin/zdb
/usr/sbin/zpool
/usr/sbin/zfs
root@linux:/usr/src/zfs#

This PR seems to fix it:

root@linux:/usr/src/zfs# update-initramfs -u
update-initramfs: Generating /boot/initrd.img-4.6.4+
root@linux:/usr/src/zfs# lsinitramfs /boot/initrd.img-4.6.4+ | egrep '(zdb|zpool|zfs)'
usr/sbin/zfs
usr/sbin/zpool
usr/sbin/zdb
lib64/libzfs_core.so.1
lib64/libzpool.so.2
lib64/libzfs.so.2
scripts/zfs
scripts/local-top/zfs
sbin/mount.zfs
lib/modules/4.6.4+/extra/zfs
lib/modules/4.6.4+/extra/zfs/zfs.ko
etc/default/zfs
etc/zfs
etc/zfs/zfs-functions
etc/zfs/zpool.cache
root@linux:/usr/src/zfs#

How Has This Been Tested?

Local Debian box.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.
Change has been approved by a ZFS on Linux member.

behlendorf

This looks totally reasonable to me. Just a few nits.

behlendorf · 2017-10-16T23:54:06Z

contrib/initramfs/Makefile.am

-	$(top_srcdir)/contrib/initramfs/hooks/zfs \
-	$(top_srcdir)/contrib/initramfs/scripts/zfs \
+	$(top_srcdir)/contrib/initramfs/hooks/zfs.in \
+	$(top_srcdir)/contrib/initramfs/scripts/zfs.in \


Since you've added proper Makefile's for the hooks and scripts sub-directories you can do the EXTRA_DIST for the .in file in each respective Makefile. They should be removed entirely from here. You can use make dist and check the tarball as a way to verify they were properly included.

behlendorf · 2017-10-16T23:56:32Z

contrib/initramfs/scripts/Makefile.am

+
+EXTRA_DIST = \
+	$(top_srcdir)/contrib/initramfs/scripts/zfs.in
+	$(top_srcdir)/contrib/initramfs/scripts/local-top/zfs


What's with the local-top entry? This shouldn't be needed.

I actually don't know how to properly use autotools, i just copy-pasted most of these files from the other dirs.

If i understand correctly local-top/zfs is needed in either contrib/initramfs/scripts/Makefile.am or contrib/initramfs/Makefile.am, not both; since contrib/initramfs/scripts/Makefile.am is dealing with files in "contrib/initramfs/scripts" maybe it's better to remove local-top/zfs from the other Makefile.am?

The EXTRA_DIST variable is used to add additional files to the tarball which otherwise would be excluded because they're not directly referenced in a Makefile.am as _SOURCES, _HEADERS, _SCRIPTS, etc.

I think we have two pretty reasonable options here.

Keep everything needed to build and install this directory in one contrib/initramfsMakefile.am as it is now. You'd then need to add the new .in targets to contrib/initramfs/Makefile.am. Or,

Create a Makefile.am in each sub-directory which is responsible for only including, building, and installing the files in that directory. This would be my preference and basically the way you were going with this.

Does option 2 also include sub-directories? In this case it's better to leave local-top/zfs in this Makefile ("contrib/initramfs/scripts/Makefile.am"), right?

Yes, you'd either want a Makefile.am in each sub-directory which is responsible for the files in that directory. Like most of the rest of the tree.

initramfs/Makefile.am initramfs/conf-hooks.d/Makefile.am initramfs/hooks/Makefile.am initramfs/scripts/Makefile.am initramfs/scripts/local-top/Makefile.am

or, a single initramfs/Makefile.am responsible for all the sub-directories as it is now.

Either are reasonable options but we should avoid doing something in between which would be confusing.

codecov · 2017-10-17T03:24:10Z

Codecov Report

Merging #6761 into master will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #6761      +/-   ##
==========================================
+ Coverage    75.3%   75.35%   +0.05%     
==========================================
  Files         297      297              
  Lines       94441    94441              
==========================================
+ Hits        71116    71164      +48     
+ Misses      23325    23277      -48

Flag	Coverage Δ
#kernel	`74.58% <ø> (+0.05%)`	⬆️
#user	`67.65% <ø> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
cmd/zed/zed_disk_event.c	`81.15% <0%> (-2.18%)`	⬇️
module/zfs/spa_errlog.c	`92.3% <0%> (-1.54%)`	⬇️
module/zfs/vdev_disk.c	`70.46% <0%> (-1.43%)`	⬇️
module/zfs/dsl_scan.c	`82.22% <0%> (-1.41%)`	⬇️
module/zfs/dmu_traverse.c	`94.48% <0%> (-0.69%)`	⬇️
module/zfs/zil.c	`92.8% <0%> (-0.64%)`	⬇️
module/zfs/zio.c	`92.53% <0%> (-0.45%)`	⬇️
module/zfs/vdev_label.c	`95.01% <0%> (-0.4%)`	⬇️
module/zfs/dmu_objset.c	`90.28% <0%> (-0.35%)`	⬇️
module/zfs/ddt.c	`96.75% <0%> (-0.19%)`	⬇️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df1f129...a06602d. Read the comment docs.

behlendorf · 2017-10-17T18:33:28Z

contrib/initramfs/scripts/Makefile.am

+
+EXTRA_DIST = \
+	$(top_srcdir)/contrib/initramfs/scripts/zfs.in
+	$(top_srcdir)/contrib/initramfs/scripts/local-top/zfs


The EXTRA_DIST variable is used to add additional files to the tarball which otherwise would be excluded because they're not directly referenced in a Makefile.am as _SOURCES, _HEADERS, _SCRIPTS, etc.

I think we have two pretty reasonable options here.

Keep everything needed to build and install this directory in one contrib/initramfsMakefile.am as it is now. You'd then need to add the new .in targets to contrib/initramfs/Makefile.am. Or,

Create a Makefile.am in each sub-directory which is responsible for only including, building, and installing the files in that directory. This would be my preference and basically the way you were going with this.

Conan-Kudo · 2017-11-01T16:50:49Z

Do we need corresponding changes for dracut? In addition, aren't Signed-off-by lines supposed to contain real, authoritative names?

loli10K · 2017-11-04T14:30:06Z

Do we need corresponding changes for dracut?

I can't comment on this, i'm only fixing this for my Debian8 box which i think only uses the initramfs code.

Conan-Kudo · 2017-11-04T14:31:23Z

@loli10K As Debian can use either initramfs-tools or dracut, and Debian will eventually move to dracut, it'd be good to check to see if similar fixes are needed there.

loli10K · 2017-11-04T14:38:14Z

it'd be good to check to see if similar fixes are needed there.

I'll be sure to push additional dracut fixes in a follow-up PR as soon as i'm in a position to test/verify them.

EDIT: merge conflicts resolved.

Signed-off-by: loli10K <ezomori.nozomu@gmail.com>

behlendorf · 2017-11-06T18:31:18Z

Sounds good, let's get this fixed in the current PR and follow up with the needed dracut changes.

Use automake to build initramfs scripts and hooks. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: loli10K <ezomori.nozomu@gmail.com> Closes openzfs#6761

Use automake to build initramfs scripts and hooks. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: loli10K <ezomori.nozomu@gmail.com> Closes openzfs#6761 (cherry picked from commit cb3b041)

Use automake to build initramfs scripts and hooks. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: loli10K <ezomori.nozomu@gmail.com> Closes openzfs#6761 (cherry picked from commit cb3b041) Signed-off-by: Neal Gompa <ngompa@datto.com> Requires-spl: spl-0.7-release

Use automake to build initramfs scripts and hooks. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: loli10K <ezomori.nozomu@gmail.com> Closes openzfs#6761

Use automake to build initramfs scripts and hooks. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: loli10K <ezomori.nozomu@gmail.com> Closes #6761

Use automake to build initramfs scripts and hooks. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: loli10K <ezomori.nozomu@gmail.com> Closes openzfs#6761

loli10K added the Status: Work in Progress Not yet ready for general review label Oct 15, 2017

loli10K force-pushed the contrib-initramfs-automake branch from 161f8b0 to 2342444 Compare October 15, 2017 08:03

loli10K removed the Status: Work in Progress Not yet ready for general review label Oct 15, 2017

behlendorf approved these changes Oct 16, 2017

View reviewed changes

behlendorf requested changes Oct 17, 2017

View reviewed changes

loli10K force-pushed the contrib-initramfs-automake branch 2 times, most recently from b1d7b47 to 23c6988 Compare October 17, 2017 19:30

behlendorf added the Status: Work in Progress Not yet ready for general review label Oct 18, 2017

loli10K force-pushed the contrib-initramfs-automake branch 2 times, most recently from 21e4f22 to 6ee7c1b Compare October 24, 2017 05:28

loli10K removed the Status: Work in Progress Not yet ready for general review label Oct 24, 2017

loli10K force-pushed the contrib-initramfs-automake branch from 6ee7c1b to 36fc16e Compare November 1, 2017 09:09

contrib/initramfs: switch to automake

a06602d

Signed-off-by: loli10K <ezomori.nozomu@gmail.com>

loli10K force-pushed the contrib-initramfs-automake branch from 36fc16e to a06602d Compare November 4, 2017 14:46

behlendorf approved these changes Nov 6, 2017

View reviewed changes

behlendorf merged commit cb3b041 into openzfs:master Nov 7, 2017

loli10K deleted the contrib-initramfs-automake branch November 8, 2017 06:15

tonyhutter pushed a commit that referenced this pull request Mar 4, 2019

contrib/initramfs: switch to automake

2428fbb

Use automake to build initramfs scripts and hooks. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: loli10K <ezomori.nozomu@gmail.com> Closes #6761

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

contrib/initramfs: switch to automake #6761

contrib/initramfs: switch to automake #6761

loli10K commented Oct 15, 2017

behlendorf left a comment •

edited

Loading

behlendorf Oct 16, 2017

behlendorf Oct 16, 2017

loli10K Oct 17, 2017

behlendorf Oct 17, 2017

loli10K Oct 17, 2017

behlendorf Oct 17, 2017

codecov bot commented Oct 17, 2017 •

edited

Loading

behlendorf Oct 17, 2017

Conan-Kudo commented Nov 1, 2017

loli10K commented Nov 4, 2017

Conan-Kudo commented Nov 4, 2017

loli10K commented Nov 4, 2017 •

edited

Loading

behlendorf commented Nov 6, 2017

contrib/initramfs: switch to automake #6761

contrib/initramfs: switch to automake #6761

Conversation

loli10K commented Oct 15, 2017

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

behlendorf left a comment • edited Loading

Choose a reason for hiding this comment

behlendorf Oct 16, 2017

Choose a reason for hiding this comment

behlendorf Oct 16, 2017

Choose a reason for hiding this comment

loli10K Oct 17, 2017

Choose a reason for hiding this comment

behlendorf Oct 17, 2017

Choose a reason for hiding this comment

loli10K Oct 17, 2017

Choose a reason for hiding this comment

behlendorf Oct 17, 2017

Choose a reason for hiding this comment

codecov bot commented Oct 17, 2017 • edited Loading

Codecov Report

behlendorf Oct 17, 2017

Choose a reason for hiding this comment

Conan-Kudo commented Nov 1, 2017

loli10K commented Nov 4, 2017

Conan-Kudo commented Nov 4, 2017

loli10K commented Nov 4, 2017 • edited Loading

behlendorf commented Nov 6, 2017

behlendorf left a comment •

edited

Loading

codecov bot commented Oct 17, 2017 •

edited

Loading

loli10K commented Nov 4, 2017 •

edited

Loading