| Message ID | 20190828122604.6829-2-mfo@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for ZFS debug symbols | expand |
On Wed, Aug 28, 2019 at 09:25:59AM -0300, Mauricio Faria de Oliveira wrote: > BugLink: https://bugs.launchpad.net/bugs/1840704 > > The 'find .ko | sed | while read module' loop has the $(pkgdir) path > hardcoded in a couple places to reconstruct the path 'sed' destroyed. > > Remove that 'sed' expression to destroy the first components of the > absolute pathname and get its '/lib/modules/'-based path with shell. > > This is needed for the next patch. > > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> > --- > debian/rules.d/2-binary-arch.mk | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > index 083b64772439..730373e93e53 100644 > --- a/debian/rules.d/2-binary-arch.mk > +++ b/debian/rules.d/2-binary-arch.mk > @@ -282,16 +282,17 @@ ifneq ($(skipdbg),true) > INSTALL_MOD_PATH=$(dbgpkgdir)/usr/lib/debug > # Add .gnu_debuglink sections to each stripped .ko > # pointing to unstripped verson > - find $(pkgdir) -name '*.ko' | sed 's|$(pkgdir)||'| while read module ; do \ > + find $(pkgdir) -name '*.ko' | while read path_module ; do \ > + module="/lib/modules/$${path_module#*/lib/modules/}"; \ Why not do this instead? module=$${path_module#$(pkgdir)} I don't think it makes a difference in practice, I just find it easier to parse. I had to stare at what you wrote for a bit to convince myself that the result was the same as what sed was doing previously, whereas the above is more obviously equivalent. Thanks, Seth
On Thu, Sep 5, 2019 at 1:06 PM Seth Forshee <seth.forshee@canonical.com> wrote: > > On Wed, Aug 28, 2019 at 09:25:59AM -0300, Mauricio Faria de Oliveira wrote: > > BugLink: https://bugs.launchpad.net/bugs/1840704 > > > > The 'find .ko | sed | while read module' loop has the $(pkgdir) path > > hardcoded in a couple places to reconstruct the path 'sed' destroyed. > > > > Remove that 'sed' expression to destroy the first components of the > > absolute pathname and get its '/lib/modules/'-based path with shell. > > > > This is needed for the next patch. > > > > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> > > --- > > debian/rules.d/2-binary-arch.mk | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > index 083b64772439..730373e93e53 100644 > > --- a/debian/rules.d/2-binary-arch.mk > > +++ b/debian/rules.d/2-binary-arch.mk > > @@ -282,16 +282,17 @@ ifneq ($(skipdbg),true) > > INSTALL_MOD_PATH=$(dbgpkgdir)/usr/lib/debug > > # Add .gnu_debuglink sections to each stripped .ko > > # pointing to unstripped verson > > - find $(pkgdir) -name '*.ko' | sed 's|$(pkgdir)||'| while read module ; do \ > > + find $(pkgdir) -name '*.ko' | while read path_module ; do \ > > + module="/lib/modules/$${path_module#*/lib/modules/}"; \ > > Why not do this instead? > > module=$${path_module#$(pkgdir)} > This would again depend on $(pkgdir) inside the loop, and the intention is to generalize that for the next patch. The next patch needs this dependency not to exist, so that $(dbgpkgdir) can be iterated over too in the loop. Hope this clarifies things. Thanks for reviewing! > I don't think it makes a difference in practice, I just find it easier > to parse. I had to stare at what you wrote for a bit to convince myself > that the result was the same as what sed was doing previously, whereas > the above is more obviously equivalent. > > Thanks, > Seth
On Thu, Sep 5, 2019 at 3:02 PM Mauricio Faria de Oliveira <mfo@canonical.com> wrote: > > On Thu, Sep 5, 2019 at 1:06 PM Seth Forshee <seth.forshee@canonical.com> wrote: > > > > On Wed, Aug 28, 2019 at 09:25:59AM -0300, Mauricio Faria de Oliveira wrote: > > > BugLink: https://bugs.launchpad.net/bugs/1840704 > > > > > > The 'find .ko | sed | while read module' loop has the $(pkgdir) path > > > hardcoded in a couple places to reconstruct the path 'sed' destroyed. > > > > > > Remove that 'sed' expression to destroy the first components of the > > > absolute pathname and get its '/lib/modules/'-based path with shell. > > > > > > This is needed for the next patch. > > > > > > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> > > > --- > > > debian/rules.d/2-binary-arch.mk | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > > index 083b64772439..730373e93e53 100644 > > > --- a/debian/rules.d/2-binary-arch.mk > > > +++ b/debian/rules.d/2-binary-arch.mk > > > @@ -282,16 +282,17 @@ ifneq ($(skipdbg),true) > > > INSTALL_MOD_PATH=$(dbgpkgdir)/usr/lib/debug > > > # Add .gnu_debuglink sections to each stripped .ko > > > # pointing to unstripped verson > > > - find $(pkgdir) -name '*.ko' | sed 's|$(pkgdir)||'| while read module ; do \ > > > + find $(pkgdir) -name '*.ko' | while read path_module ; do \ > > > + module="/lib/modules/$${path_module#*/lib/modules/}"; \ > > > > Why not do this instead? > > > > module=$${path_module#$(pkgdir)} > > > > This would again depend on $(pkgdir) inside the loop, > and the intention is to generalize that for the next patch. > > The next patch needs this dependency not to exist, > so that $(dbgpkgdir) can be iterated over too in the loop. ^ oops, s/dbgpkgdir/pkgdir_ex/ (for linux-modules-extras). > > Hope this clarifies things. Thanks for reviewing! > > > I don't think it makes a difference in practice, I just find it easier > > to parse. I had to stare at what you wrote for a bit to convince myself > > that the result was the same as what sed was doing previously, whereas > > the above is more obviously equivalent. > > > > Thanks, > > Seth > > > > -- > Mauricio Faria de Oliveira
On Thu, Sep 05, 2019 at 03:06:38PM -0300, Mauricio Faria de Oliveira wrote: > On Thu, Sep 5, 2019 at 3:02 PM Mauricio Faria de Oliveira > <mfo@canonical.com> wrote: > > > > On Thu, Sep 5, 2019 at 1:06 PM Seth Forshee <seth.forshee@canonical.com> wrote: > > > > > > On Wed, Aug 28, 2019 at 09:25:59AM -0300, Mauricio Faria de Oliveira wrote: > > > > BugLink: https://bugs.launchpad.net/bugs/1840704 > > > > > > > > The 'find .ko | sed | while read module' loop has the $(pkgdir) path > > > > hardcoded in a couple places to reconstruct the path 'sed' destroyed. > > > > > > > > Remove that 'sed' expression to destroy the first components of the > > > > absolute pathname and get its '/lib/modules/'-based path with shell. > > > > > > > > This is needed for the next patch. > > > > > > > > Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> > > > > --- > > > > debian/rules.d/2-binary-arch.mk | 7 ++++--- > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > > > index 083b64772439..730373e93e53 100644 > > > > --- a/debian/rules.d/2-binary-arch.mk > > > > +++ b/debian/rules.d/2-binary-arch.mk > > > > @@ -282,16 +282,17 @@ ifneq ($(skipdbg),true) > > > > INSTALL_MOD_PATH=$(dbgpkgdir)/usr/lib/debug > > > > # Add .gnu_debuglink sections to each stripped .ko > > > > # pointing to unstripped verson > > > > - find $(pkgdir) -name '*.ko' | sed 's|$(pkgdir)||'| while read module ; do \ > > > > + find $(pkgdir) -name '*.ko' | while read path_module ; do \ > > > > + module="/lib/modules/$${path_module#*/lib/modules/}"; \ > > > > > > Why not do this instead? > > > > > > module=$${path_module#$(pkgdir)} > > > > > > > This would again depend on $(pkgdir) inside the loop, > > and the intention is to generalize that for the next patch. > > > > The next patch needs this dependency not to exist, > > so that $(dbgpkgdir) can be iterated over too in the loop. > > ^ oops, s/dbgpkgdir/pkgdir_ex/ (for linux-modules-extras). > > > > Hope this clarifies things. Thanks for reviewing! Yes it does, thanks! > > > > > I don't think it makes a difference in practice, I just find it easier > > > to parse. I had to stare at what you wrote for a bit to convince myself > > > that the result was the same as what sed was doing previously, whereas > > > the above is more obviously equivalent. > > > > > > Thanks, > > > Seth > > > > > > > > -- > > Mauricio Faria de Oliveira > > > > -- > Mauricio Faria de Oliveira
diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk index 083b64772439..730373e93e53 100644 --- a/debian/rules.d/2-binary-arch.mk +++ b/debian/rules.d/2-binary-arch.mk @@ -282,16 +282,17 @@ ifneq ($(skipdbg),true) INSTALL_MOD_PATH=$(dbgpkgdir)/usr/lib/debug # Add .gnu_debuglink sections to each stripped .ko # pointing to unstripped verson - find $(pkgdir) -name '*.ko' | sed 's|$(pkgdir)||'| while read module ; do \ + find $(pkgdir) -name '*.ko' | while read path_module ; do \ + module="/lib/modules/$${path_module#*/lib/modules/}"; \ if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \ $(CROSS_COMPILE)objcopy \ --add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \ - $(pkgdir)/$$module; \ + $$path_module; \ if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config; then \ $(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \ $(MODSECKEY) \ $(MODPUBKEY) \ - $(pkgdir)/$$module; \ + $$path_module; \ fi; \ fi; \ done
BugLink: https://bugs.launchpad.net/bugs/1840704 The 'find .ko | sed | while read module' loop has the $(pkgdir) path hardcoded in a couple places to reconstruct the path 'sed' destroyed. Remove that 'sed' expression to destroy the first components of the absolute pathname and get its '/lib/modules/'-based path with shell. This is needed for the next patch. Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> --- debian/rules.d/2-binary-arch.mk | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)