| Message ID | 20210413123638.26707-1-juergh@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Unstable] UBUNTU: [Packaging] Add module versions to the ABI | expand |
On Tue, Apr 13, 2021 at 02:36:38PM +0200, Juerg Haefliger wrote: > Add module versions to the ABI (or '-' if the module doesn't report a > version) and make sure the module check only looks at the module name > and ignores the trailing version. same here -- see "debian: Drop versioned ABI directory names" thread.
On Tue, Apr 13, 2021 at 02:36:38PM +0200, Juerg Haefliger wrote: > Add module versions to the ABI (or '-' if the module doesn't report a > version) and make sure the module check only looks at the module name > and ignores the trailing version. > > This info ends up in the buildinfo package which can be used by meta > packages so that they don't have to build-depend on the huge modules > package to determine versions of in-tree DKMS modules. > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> This doesn't seem like a very extensible way of doing things if we ever need to add additional information from modinfo later. We're already inclding the firmware information separately in the fwinfo file. I wonder how much it would bloat the abi size to include all of the modinfo data, similar to what's in modules.builtin.modinfo already (which we would probably also want to pull in as part of this information). It could probably replace the fwinfo and flavour.modules files though as they would now contain redundant information. Seth
On 19.04.21 22:25, Seth Forshee wrote: > On Tue, Apr 13, 2021 at 02:36:38PM +0200, Juerg Haefliger wrote: >> Add module versions to the ABI (or '-' if the module doesn't report a >> version) and make sure the module check only looks at the module name >> and ignores the trailing version. >> >> This info ends up in the buildinfo package which can be used by meta >> packages so that they don't have to build-depend on the huge modules >> package to determine versions of in-tree DKMS modules. >> >> Signed-off-by: Juerg Haefliger <juergh@canonical.com> > > This doesn't seem like a very extensible way of doing things if we ever > need to add additional information from modinfo later. We're already > inclding the firmware information separately in the fwinfo file. I > wonder how much it would bloat the abi size to include all of the > modinfo data, similar to what's in modules.builtin.modinfo already > (which we would probably also want to pull in as part of this > information). It could probably replace the fwinfo and flavour.modules > files though as they would now contain redundant information. I am not sure I understand the relation between fwinfo and <flavour>.modinfo. The former to my knowledge is just a list list of all firmware files that are included in the kernel binary. I must admit I cannot remember even what that info is used for. The *.modinfo files list modules and related info. Actually right now, only lists module names. And adding a version field is extending this. With <space> as separator between fields, we could have other separators within those. What I think is a good idea is to think about adding info for builtin modules. That would at least get rid of the additional tweaking when making a module a builtin (right now it vanishes from the abi perspective). -Stefan > > Seth >
On Mon, 19 Apr 2021 15:25:56 -0500 Seth Forshee <seth.forshee@canonical.com> wrote: > On Tue, Apr 13, 2021 at 02:36:38PM +0200, Juerg Haefliger wrote: > > Add module versions to the ABI (or '-' if the module doesn't report a > > version) and make sure the module check only looks at the module name > > and ignores the trailing version. > > > > This info ends up in the buildinfo package which can be used by meta > > packages so that they don't have to build-depend on the huge modules > > package to determine versions of in-tree DKMS modules. > > > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> > > This doesn't seem like a very extensible way of doing things if we ever > need to add additional information from modinfo later. We could turn this into a CSV list and add more fields as necessary but I agree it's not very flexible. > We're already > inclding the firmware information separately in the fwinfo file. I > wonder how much it would bloat the abi size to include all of the > modinfo data, similar to what's in modules.builtin.modinfo Huh? Where does this file live? > already > (which we would probably also want to pull in as part of this > information). It could probably replace the fwinfo and flavour.modules > files though as they would now contain redundant information. Yes, the more I think about it the more I believe we should include all modinfo data and let the consumer decide which data to use rather than trying to be 'smart' at buildinfo build time (just to realize later that we should also have included XYZ). I'll play around with it some more. ...Juerg > Seth
On Tue, Apr 20, 2021 at 12:38:03PM +0200, Juerg Haefliger wrote: > On Mon, 19 Apr 2021 15:25:56 -0500 > Seth Forshee <seth.forshee@canonical.com> wrote: > > > On Tue, Apr 13, 2021 at 02:36:38PM +0200, Juerg Haefliger wrote: > > > Add module versions to the ABI (or '-' if the module doesn't report a > > > version) and make sure the module check only looks at the module name > > > and ignores the trailing version. > > > > > > This info ends up in the buildinfo package which can be used by meta > > > packages so that they don't have to build-depend on the huge modules > > > package to determine versions of in-tree DKMS modules. > > > > > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> > > > > This doesn't seem like a very extensible way of doing things if we ever > > need to add additional information from modinfo later. > > We could turn this into a CSV list and add more fields as necessary but I > agree it's not very flexible. > > > > We're already > > inclding the firmware information separately in the fwinfo file. I > > wonder how much it would bloat the abi size to include all of the > > modinfo data, similar to what's in modules.builtin.modinfo > > Huh? Where does this file live? /usr/lib/modules/$(uname -r)/modules.builtin.modinfo It's like a text file but fields are separated by nulls rather than newlines, so run it through "tr '\0' '\n'" if you want to look at it. > > > > already > > (which we would probably also want to pull in as part of this > > information). It could probably replace the fwinfo and flavour.modules > > files though as they would now contain redundant information. > > Yes, the more I think about it the more I believe we should include all > modinfo data and let the consumer decide which data to use rather than trying > to be 'smart' at buildinfo build time (just to realize later that we should > also have included XYZ). I'll play around with it some more. > > ...Juerg > > > > Seth >
On Tue, Apr 20, 2021 at 09:14:45AM +0200, Stefan Bader wrote: > On 19.04.21 22:25, Seth Forshee wrote: > > On Tue, Apr 13, 2021 at 02:36:38PM +0200, Juerg Haefliger wrote: > > > Add module versions to the ABI (or '-' if the module doesn't report a > > > version) and make sure the module check only looks at the module name > > > and ignores the trailing version. > > > > > > This info ends up in the buildinfo package which can be used by meta > > > packages so that they don't have to build-depend on the huge modules > > > package to determine versions of in-tree DKMS modules. > > > > > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> > > > > This doesn't seem like a very extensible way of doing things if we ever > > need to add additional information from modinfo later. We're already > > inclding the firmware information separately in the fwinfo file. I > > wonder how much it would bloat the abi size to include all of the > > modinfo data, similar to what's in modules.builtin.modinfo already > > (which we would probably also want to pull in as part of this > > information). It could probably replace the fwinfo and flavour.modules > > files though as they would now contain redundant information. > > > I am not sure I understand the relation between fwinfo and > <flavour>.modinfo. The former to my knowledge is just a list list of all > firmware files that are included in the kernel binary. I must admit I cannot > remember even what that info is used for. I use to identify firmware files which we need to bring back to LTS releases to support hwe kernels. Aside from that I don't know of anything it is used for. > The *.modinfo files list modules and related info. Actually right now, only > lists module names. And adding a version field is extending this. With > <space> as separator between fields, we could have other separators within > those. I think you mean the *.modules files. modules.builtin.modinfo is an artifact of the kernel build which contains the modinfo data from the modules built into the kernel, since you cannot run modinfo on the kernel image. > What I think is a good idea is to think about adding info for builtin > modules. That would at least get rid of the additional tweaking when making > a module a builtin (right now it vanishes from the abi perspective). > > -Stefan > > > > Seth > > > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On Tue, 20 Apr 2021 09:14:45 +0200 Stefan Bader <stefan.bader@canonical.com> wrote: > On 19.04.21 22:25, Seth Forshee wrote: > > On Tue, Apr 13, 2021 at 02:36:38PM +0200, Juerg Haefliger wrote: > >> Add module versions to the ABI (or '-' if the module doesn't report a > >> version) and make sure the module check only looks at the module name > >> and ignores the trailing version. > >> > >> This info ends up in the buildinfo package which can be used by meta > >> packages so that they don't have to build-depend on the huge modules > >> package to determine versions of in-tree DKMS modules. > >> > >> Signed-off-by: Juerg Haefliger <juergh@canonical.com> > > > > This doesn't seem like a very extensible way of doing things if we ever > > need to add additional information from modinfo later. We're already > > inclding the firmware information separately in the fwinfo file. I > > wonder how much it would bloat the abi size to include all of the > > modinfo data, similar to what's in modules.builtin.modinfo already > > (which we would probably also want to pull in as part of this > > information). It could probably replace the fwinfo and flavour.modules > > files though as they would now contain redundant information. > > > I am not sure I understand the relation between fwinfo and <flavour>.modinfo. > The former to my knowledge is just a list list of all firmware files that are > included in the kernel binary. It's a list of fw blobs our modules want to load. > I must admit I cannot remember even what that > info is used for. Probably to figure out if all required fw blobs are provided by linux-firmware. > The *.modinfo files list modules and related info. Actually right now, only > lists module names. And adding a version field is extending this. With <space> > as separator between fields, we could have other separators within those. > What I think is a good idea is to think about adding info for builtin modules. Yes. Good point. > That would at least get rid of the additional tweaking when making a module a > builtin (right now it vanishes from the abi perspective). I'm leaning towards Seth's idea of 'simply' adding all info from modinfo to the ABI for both 'real' and built-in modules. ...Juerg > -Stefan > > > > Seth > > > >
Will send a follow-on RFC. ...Juerg > Add module versions to the ABI (or '-' if the module doesn't report a > version) and make sure the module check only looks at the module name > and ignores the trailing version. > > This info ends up in the buildinfo package which can be used by meta > packages so that they don't have to build-depend on the huge modules > package to determine versions of in-tree DKMS modules. > > Signed-off-by: Juerg Haefliger <juergh@canonical.com> > --- > debian/rules.d/2-binary-arch.mk | 6 +++++- > debian/scripts/module-check | 3 +++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > index 5ae9a989207c..c8ca29e7b739 100644 > --- a/debian/rules.d/2-binary-arch.mk > +++ b/debian/rules.d/2-binary-arch.mk > @@ -461,7 +461,11 @@ endif > > # Build the final ABI modules information. > find $(pkgdir_bin) $(pkgdir) $(pkgdir_ex) -name \*.ko | \ > - sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort > $(abidir)/$*.modules > + while read ko; do \ > + basename=$${ko##*/}; \ > + version=$$(/sbin/modinfo -F version "$${ko}" || true); \ > + echo "$${basename%.ko} $${version:--}"; \ > + done | sort > $(abidir)/$*.modules > > # Build the final ABI firmware information. > find $(pkgdir_bin) $(pkgdir) $(pkgdir_ex) -name \*.ko | \ > diff --git a/debian/scripts/module-check b/debian/scripts/module-check > index c754ea368cfb..4955211affbd 100755 > --- a/debian/scripts/module-check > +++ b/debian/scripts/module-check > @@ -42,6 +42,7 @@ if (-f "$prev_abidir/../modules.ignore") { > while (<IGNORE>) { > chomp; > next if /\s*#/; > + $_ = (split)[0]; > $modules_ignore{$_} = 1; > $ignore++; > } > @@ -56,6 +57,7 @@ open(NEW, "< $abidir/$flavour.modules") or > die "Could not open $abidir/$flavour.modules"; > while (<NEW>) { > chomp; > + $_ = (split)[0]; > $modules{$_} = 1; > $new_count++; > } > @@ -69,6 +71,7 @@ open(OLD, "< $prev_abidir/$flavour.modules") or > die "Could not open $prev_abidir/$flavour.modules"; > while (<OLD>) { > chomp; > + $_ = (split)[0]; > if (not defined($modules{$_})) { > print "\n" if not $missing; > $missing++;
diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk index 5ae9a989207c..c8ca29e7b739 100644 --- a/debian/rules.d/2-binary-arch.mk +++ b/debian/rules.d/2-binary-arch.mk @@ -461,7 +461,11 @@ endif # Build the final ABI modules information. find $(pkgdir_bin) $(pkgdir) $(pkgdir_ex) -name \*.ko | \ - sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort > $(abidir)/$*.modules + while read ko; do \ + basename=$${ko##*/}; \ + version=$$(/sbin/modinfo -F version "$${ko}" || true); \ + echo "$${basename%.ko} $${version:--}"; \ + done | sort > $(abidir)/$*.modules # Build the final ABI firmware information. find $(pkgdir_bin) $(pkgdir) $(pkgdir_ex) -name \*.ko | \ diff --git a/debian/scripts/module-check b/debian/scripts/module-check index c754ea368cfb..4955211affbd 100755 --- a/debian/scripts/module-check +++ b/debian/scripts/module-check @@ -42,6 +42,7 @@ if (-f "$prev_abidir/../modules.ignore") { while (<IGNORE>) { chomp; next if /\s*#/; + $_ = (split)[0]; $modules_ignore{$_} = 1; $ignore++; } @@ -56,6 +57,7 @@ open(NEW, "< $abidir/$flavour.modules") or die "Could not open $abidir/$flavour.modules"; while (<NEW>) { chomp; + $_ = (split)[0]; $modules{$_} = 1; $new_count++; } @@ -69,6 +71,7 @@ open(OLD, "< $prev_abidir/$flavour.modules") or die "Could not open $prev_abidir/$flavour.modules"; while (<OLD>) { chomp; + $_ = (split)[0]; if (not defined($modules{$_})) { print "\n" if not $missing; $missing++;
Add module versions to the ABI (or '-' if the module doesn't report a version) and make sure the module check only looks at the module name and ignores the trailing version. This info ends up in the buildinfo package which can be used by meta packages so that they don't have to build-depend on the huge modules package to determine versions of in-tree DKMS modules. Signed-off-by: Juerg Haefliger <juergh@canonical.com> --- debian/rules.d/2-binary-arch.mk | 6 +++++- debian/scripts/module-check | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-)