| Message ID | 20181205171644.29784-1-marcelo.cerri@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [b,c,d,v2] UBUNTU: [Packaging] buildinfo -- include origin package mark to modules files | expand |
On Wed, Dec 05, 2018 at 03:16:44PM -0200, Marcelo Henrique Cerri wrote: > BugLink: http://bugs.launchpad.net/bugs/1806380 > > Include a flag at the end of each line of the > "debian.$branch/abi/*/$arch/$flavour.modules" file indicating the > package that each module is currently shipped in. > > This will cause the build to fail when a module is silently moved from > or to the linux-modules-extra package. > > Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com> Doesn't module-check also need to be updated to avoid breaking use of modules.ignore? Or is the expectation that we will start appending the "extra" as needed when adding modules to that file? I'm also curious whether you considered splitting them out into multiple abi files, something like generic.modules and generic.extras.modules, and if so why you settled on using a flag instead. Should we extract the flag name from the package name rather than hardcoding it like you've done here. This may be overthinking it, since I don't know whether we would need to track abi for any additional module packages nor what those packages would be named. Seth
On 10.12.18 21:50, Seth Forshee wrote: > On Wed, Dec 05, 2018 at 03:16:44PM -0200, Marcelo Henrique Cerri wrote: >> BugLink: http://bugs.launchpad.net/bugs/1806380 >> >> Include a flag at the end of each line of the >> "debian.$branch/abi/*/$arch/$flavour.modules" file indicating the >> package that each module is currently shipped in. >> >> This will cause the build to fail when a module is silently moved from >> or to the linux-modules-extra package. >> >> Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com> > > Doesn't module-check also need to be updated to avoid breaking use of > modules.ignore? Or is the expectation that we will start appending the > "extra" as needed when adding modules to that file? > > I'm also curious whether you considered splitting them out into multiple > abi files, something like generic.modules and generic.extras.modules, > and if so why you settled on using a flag instead. > > Should we extract the flag name from the package name rather than > hardcoding it like you've done here. This may be overthinking it, since > I don't know whether we would need to track abi for any additional > module packages nor what those packages would be named. > > Seth > > I'd like to see some reply to Seth's concerns/questions. -Stefan
Sorry, I totally missed that one. Answers bellow. On Mon, Dec 10, 2018 at 02:50:19PM -0600, Seth Forshee wrote: > On Wed, Dec 05, 2018 at 03:16:44PM -0200, Marcelo Henrique Cerri wrote: > > BugLink: http://bugs.launchpad.net/bugs/1806380 > > > > Include a flag at the end of each line of the > > "debian.$branch/abi/*/$arch/$flavour.modules" file indicating the > > package that each module is currently shipped in. > > > > This will cause the build to fail when a module is silently moved from > > or to the linux-modules-extra package. > > > > Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com> > > Doesn't module-check also need to be updated to avoid breaking use of > modules.ignore? Or is the expectation that we will start appending the > "extra" as needed when adding modules to that file? I believe so. Good catch. The kernels that I have tested do not use ignore.modules. I can update the patch with the fix to module-check. > > I'm also curious whether you considered splitting them out into multiple > abi files, something like generic.modules and generic.extras.modules, > and if so why you settled on using a flag instead. > Yes. My only concern about that approach is that we would multiple the number of files. Not a big problem. Do you see any benefits of having a separated file for that? > Should we extract the flag name from the package name rather than > hardcoding it like you've done here. This may be overthinking it, since > I don't know whether we would need to track abi for any additional > module packages nor what those packages would be named. > That can be done. However we already hard code the names of the packages we use to extract that information ("$(pkgdir_bin) $(pkgdir) $(pkgdir_ex)"). > Seth > -- Regards, Marcelo
On Mon, Feb 04, 2019 at 10:13:50AM -0200, Marcelo Henrique Cerri wrote: > Sorry, I totally missed that one. Answers bellow. > > On Mon, Dec 10, 2018 at 02:50:19PM -0600, Seth Forshee wrote: > > On Wed, Dec 05, 2018 at 03:16:44PM -0200, Marcelo Henrique Cerri wrote: > > > BugLink: http://bugs.launchpad.net/bugs/1806380 > > > > > > Include a flag at the end of each line of the > > > "debian.$branch/abi/*/$arch/$flavour.modules" file indicating the > > > package that each module is currently shipped in. > > > > > > This will cause the build to fail when a module is silently moved from > > > or to the linux-modules-extra package. > > > > > > Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com> > > > > Doesn't module-check also need to be updated to avoid breaking use of > > modules.ignore? Or is the expectation that we will start appending the > > "extra" as needed when adding modules to that file? > > I believe so. Good catch. The kernels that I have tested do not use > ignore.modules. I can update the patch with the fix to module-check. Thanks. > > > > I'm also curious whether you considered splitting them out into multiple > > abi files, something like generic.modules and generic.extras.modules, > > and if so why you settled on using a flag instead. > > > > Yes. My only concern about that approach is that we would multiple the > number of files. Not a big problem. Do you see any benefits of having > a separated file for that? I don't think we should worry too much about whether or not it adds files to the abi. I was more curious than anything though, I don't have a strong opinion. > > > Should we extract the flag name from the package name rather than > > hardcoding it like you've done here. This may be overthinking it, since > > I don't know whether we would need to track abi for any additional > > module packages nor what those packages would be named. > > > > That can be done. However we already hard code the names of the > packages we use to extract that information ("$(pkgdir_bin) $(pkgdir) > $(pkgdir_ex)"). Ok, whatever makes the most sense. Thanks, Seth
I will provide a v3.
diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk index 20f744c012a9..974dbf04d2f7 100644 --- a/debian/rules.d/2-binary-arch.mk +++ b/debian/rules.d/2-binary-arch.mk @@ -407,8 +407,16 @@ endif $(builddir)/build-$*/Module.symvers | sort > $(abidir)/$* # Build the final ABI modules information. - find $(pkgdir_bin) $(pkgdir) $(pkgdir_ex) -name \*.ko | \ - sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort > $(abidir)/$*.modules + > "$(abidir)/$*.modules"; \ + for dir in "$(pkgdir_bin)" "$(pkgdir)" "$(pkgdir_ex)"; do \ + case "$$dir" in \ + *extra*) mark=' extra';; \ + *) mark='';; \ + esac; \ + find "$$dir" -name \*.ko | \ + sed -e 's/.*\/\([^\/]*\)\.ko/\1'"$$mark"'/' >> "$(abidir)/$*.modules"; \ + done; \ + sort -o "$(abidir)/$*.modules" "$(abidir)/$*.modules" # Build the final ABI firmware information. find $(pkgdir_bin) $(pkgdir) $(pkgdir_ex) -name \*.ko | \ diff --git a/debian/scripts/misc/getabis b/debian/scripts/misc/getabis index 42690b0311e2..5d9daa023813 100755 --- a/debian/scripts/misc/getabis +++ b/debian/scripts/misc/getabis @@ -65,8 +65,20 @@ getall() { echo -n "extracting$prefixes..." for filename in $filenames do - dpkg-deb --extract $filename tmp + dpkg-deb --extract "$filename" tmp + # Extract the modules list, so we can mark each line + # with its origin. + case "$filename" in + *extra*) mark=' extra';; + *) mark='';; + esac + files=$(dpkg-deb --vextract "$filename" tmp) + [ "$?" -ne 0 ] && continue + echo "$files" | sed -n -e '/.*\/\([^\/]*\)\.ko/s//\1'"$mark"'/p' >> \ + "$abidir/$arch/$sub.modules" + done + sort -o "$abidir/$arch/$sub.modules" "$abidir/$arch/$sub.modules" # FORM 1: linux-image et al extracted here. if [ -d tmp/boot ]; then echo -n "images..." @@ -83,9 +95,6 @@ getall() { else echo -n "NO RETPOLINE FILE..." fi - (cd tmp; find lib/modules/$verabi-$sub/kernel -name '*.ko') | \ - sed -e 's/.*\/\([^\/]*\)\.ko/\1/' | sort > \ - $abidir/$arch/$sub.modules ( cd tmp; # Prevent exposing some errors when called by python scripts. SIGPIPE seems to get
BugLink: http://bugs.launchpad.net/bugs/1806380 Include a flag at the end of each line of the "debian.$branch/abi/*/$arch/$flavour.modules" file indicating the package that each module is currently shipped in. This will cause the build to fail when a module is silently moved from or to the linux-modules-extra package. Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com> --- v2: Just fixed the initialization of "mark" in debian/scripts/misc/getabis. Updated the same initialization in debian/rules.d/2-binary-arch.mk for consistency. --- debian/rules.d/2-binary-arch.mk | 12 ++++++++++-- debian/scripts/misc/getabis | 17 +++++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-)