[b, c, d][PATCH v2] UBUNTU: [Packaging] buildinfo -- include origin package mark to modules files

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[b, c, d][PATCH v2] UBUNTU: [Packaging] buildinfo -- include origin package mark to modules files

Marcelo Henrique Cerri
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 <[hidden email]>
---

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(-)

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
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [b, c, d][PATCH v2] UBUNTU: [Packaging] buildinfo -- include origin package mark to modules files

Seth Forshee
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 <[hidden email]>

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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [b, c, d][PATCH v2] UBUNTU: [Packaging] buildinfo -- include origin package mark to modules files

Stefan Bader-2
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 <[hidden email]>
>
> 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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [b, c, d][PATCH v2] UBUNTU: [Packaging] buildinfo -- include origin package mark to modules files

Marcelo Henrique Cerri
In reply to this post by Seth Forshee
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 <[hidden email]>
>
> 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

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [b, c, d][PATCH v2] UBUNTU: [Packaging] buildinfo -- include origin package mark to modules files

Seth Forshee
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 <[hidden email]>
> >
> > 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

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NACK: [b,c,d][PATCH v2] UBUNTU: [Packaging] buildinfo -- include origin package mark to modules files

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
I will provide a v3.

--
Regards,
Marcelo

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 <[hidden email]>
> ---
>
> 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(-)
>
> 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
> --
> 2.17.1
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (499 bytes) Download Attachment