diff mbox series

package_rpm: Fix backtrace on missing directory.

Message ID 20240223203051.1575654-1-jpuhlman@mvista.com
State New
Headers show
Series package_rpm: Fix backtrace on missing directory. | expand

Commit Message

Jeremy Puhlman Feb. 23, 2024, 8:30 p.m. UTC 
From: "Jeremy A. Puhlman" <jpuhlman@mvista.com>

This seems to work around the following issue. The recipe in quesiton
places all of its content in /opt, and we turn on license collection
globally. Systemd is turned on so usrmerge is also on. I doubt this
is the correct way to deal with this as it is not clear why it is
looking for /usr//usr in the first place. Not sure if its just a
wierd edge case or what.

File:
'/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
lineno: 527, function: write_specfile
     0523:            spec_scriptlets_bottom.append('')
     0524:
     0525:        # Now process files
     0526:        file_list = []
 *** 0527:        walk_files(root, file_list, conffiles, dirfiles)
     0528:        if not file_list and localdata.getVar('ALLOW_EMPTY',
False) != "1":
     0529:            bb.note("Not creating empty RPM package for %s" %
splitname)
     0530:        else:
     0531:            spec_files_bottom.append('%%files -n %s' %
splitname)
File:
'/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
lineno: 249, function: walk_files
     0245:                    target.append(get_attr(dir) + '%dir "' +
escape_chars(p) + '"')
     0246:            else:
     0247:                # packages own only empty directories or
explict directory.
     0248:                # This will prevent the overlapping of
security permission.
 *** 0249:                attr = get_attr(path)
     0250:                if path and not files and not dirs:
     0251:                    target.append(attr + '%dir "' +
escape_chars(path) + '"')
     0252:                elif path and path in dirfiles:
     0253:                    target.append(attr + '%dir "' +
escape_chars(path) + '"')
File:
'/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
lineno: 203, function: get_attr
     0199:        # of the walk, the isdir() test would then fail and
the walk code would assume its a file
     0200:        # hence we check for the names in files too.
     0201:        for rootpath, dirs, files in os.walk(walkpath):
     0202:            def get_attr(path):
 *** 0203:                stat_f = os.stat(rootpath + "/" + path,
follow_symlinks=False)
     0204:                mode = stat.S_IMODE(stat_f.st_mode)
     0205:                try:
     0206:                    owner =
pwd.getpwuid(stat_f.st_uid).pw_name
     0207:                except Exception as e:
Exception: FileNotFoundError: [Errno 2] No such file or directory:
'/build/tmp/work/corei7-64-montavista-linux/mvtest/2.0/packages-split/mvtest-lic/opt//opt'

Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>
---
 meta/classes-global/package_rpm.bbclass | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Alexander Kanavin Feb. 23, 2024, 9:20 p.m. UTC | #1 
Somewhere in the overall function the paths aren't formed correctly,
but this patch is very much not a correct fix either, and should be
marked as such. If you can provide a minimal reproducer recipe, that
would help; I can't say what is wrong otherwise.

You might want to insert debugging statements to see where opt//opt
formation is happening.

Alex

On Fri, 23 Feb 2024 at 21:31, Jeremy Puhlman via
lists.openembedded.org <jpuhlman=mvista.com@lists.openembedded.org>
wrote:
>
> From: "Jeremy A. Puhlman" <jpuhlman@mvista.com>
>
> This seems to work around the following issue. The recipe in quesiton
> places all of its content in /opt, and we turn on license collection
> globally. Systemd is turned on so usrmerge is also on. I doubt this
> is the correct way to deal with this as it is not clear why it is
> looking for /usr//usr in the first place. Not sure if its just a
> wierd edge case or what.
>
> File:
> '/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
> lineno: 527, function: write_specfile
>      0523:            spec_scriptlets_bottom.append('')
>      0524:
>      0525:        # Now process files
>      0526:        file_list = []
>  *** 0527:        walk_files(root, file_list, conffiles, dirfiles)
>      0528:        if not file_list and localdata.getVar('ALLOW_EMPTY',
> False) != "1":
>      0529:            bb.note("Not creating empty RPM package for %s" %
> splitname)
>      0530:        else:
>      0531:            spec_files_bottom.append('%%files -n %s' %
> splitname)
> File:
> '/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
> lineno: 249, function: walk_files
>      0245:                    target.append(get_attr(dir) + '%dir "' +
> escape_chars(p) + '"')
>      0246:            else:
>      0247:                # packages own only empty directories or
> explict directory.
>      0248:                # This will prevent the overlapping of
> security permission.
>  *** 0249:                attr = get_attr(path)
>      0250:                if path and not files and not dirs:
>      0251:                    target.append(attr + '%dir "' +
> escape_chars(path) + '"')
>      0252:                elif path and path in dirfiles:
>      0253:                    target.append(attr + '%dir "' +
> escape_chars(path) + '"')
> File:
> '/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
> lineno: 203, function: get_attr
>      0199:        # of the walk, the isdir() test would then fail and
> the walk code would assume its a file
>      0200:        # hence we check for the names in files too.
>      0201:        for rootpath, dirs, files in os.walk(walkpath):
>      0202:            def get_attr(path):
>  *** 0203:                stat_f = os.stat(rootpath + "/" + path,
> follow_symlinks=False)
>      0204:                mode = stat.S_IMODE(stat_f.st_mode)
>      0205:                try:
>      0206:                    owner =
> pwd.getpwuid(stat_f.st_uid).pw_name
>      0207:                except Exception as e:
> Exception: FileNotFoundError: [Errno 2] No such file or directory:
> '/build/tmp/work/corei7-64-montavista-linux/mvtest/2.0/packages-split/mvtest-lic/opt//opt'
>
> Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>
> ---
>  meta/classes-global/package_rpm.bbclass | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass
> index 2e3e4e8c79..a6885ac74e 100644
> --- a/meta/classes-global/package_rpm.bbclass
> +++ b/meta/classes-global/package_rpm.bbclass
> @@ -200,6 +200,8 @@ python write_specfile () {
>          # hence we check for the names in files too.
>          for rootpath, dirs, files in os.walk(walkpath):
>              def get_attr(path):
> +                if not os.path.exists(rootpath + "/" + path):
> +                   return ""
>                  stat_f = os.stat(rootpath + "/" + path, follow_symlinks=False)
>                  mode = stat.S_IMODE(stat_f.st_mode)
>                  try:
> @@ -243,7 +245,7 @@ python write_specfile () {
>                      p = path + '/' + dir
>                      # All packages own the directories their files are in...
>                      target.append(get_attr(dir) + '%dir "' + escape_chars(p) + '"')
> -            elif path:
> +            elif path and os.path.exists(path):
>                  # packages own only empty directories or explict directory.
>                  # This will prevent the overlapping of security permission.
>                  attr = get_attr(path)
> @@ -257,7 +259,7 @@ python write_specfile () {
>                  p = path + '/' + file
>                  if conffiles.count(p):
>                      target.append(attr + '%config "' + escape_chars(p) + '"')
> -                else:
> +                elif attr:
>                      target.append(attr + '"' + escape_chars(p) + '"')
>
>      # Prevent the prerm/postrm scripts from being run during an upgrade
> --
> 2.31.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196109): https://lists.openembedded.org/g/openembedded-core/message/196109
> Mute This Topic: https://lists.openembedded.org/mt/104536226/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Jeremy Puhlman Feb. 23, 2024, 10:58 p.m. UTC | #2 
On 2/23/2024 1:20 PM, Alexander Kanavin wrote:
> Somewhere in the overall function the paths aren't formed correctly,
> but this patch is very much not a correct fix either, and should be
> marked as such.
Is there a preferred way to do that? I mention in the commit message 
that is not
the right way to handle it and followed up with a reply to the patch 
mentioning
as such.

>   If you can provide a minimal reproducer recipe, that
> would help; I can't say what is wrong otherwise.
I will try and reduce it down later this evening, but the recipe in 
question is here:
https://github.com/MontaVista-OpenSourceTechnology/meta-qa/blob/master/meta-qa-framework/recipes-qatest/mvtest/mvtest_2.0.bb
Using stock poky/meta-oe/meta-python and that repo causes the issue with 
mvtest.

The recipe is fairly basic, nothing really crazy, which is what is 
baffling about the error.
>
> You might want to insert debugging statements to see where opt//opt
> formation is happening.
Tried poking at  that for a while and really didn't make much progress 
before I got
pulled off to something else.

>
> Alex
>
> On Fri, 23 Feb 2024 at 21:31, Jeremy Puhlman via
> lists.openembedded.org <jpuhlman=mvista.com@lists.openembedded.org>
> wrote:
>> From: "Jeremy A. Puhlman" <jpuhlman@mvista.com>
>>
>> This seems to work around the following issue. The recipe in quesiton
>> places all of its content in /opt, and we turn on license collection
>> globally. Systemd is turned on so usrmerge is also on. I doubt this
>> is the correct way to deal with this as it is not clear why it is
>> looking for /usr//usr in the first place. Not sure if its just a
>> wierd edge case or what.
>>
>> File:
>> '/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
>> lineno: 527, function: write_specfile
>>       0523:            spec_scriptlets_bottom.append('')
>>       0524:
>>       0525:        # Now process files
>>       0526:        file_list = []
>>   *** 0527:        walk_files(root, file_list, conffiles, dirfiles)
>>       0528:        if not file_list and localdata.getVar('ALLOW_EMPTY',
>> False) != "1":
>>       0529:            bb.note("Not creating empty RPM package for %s" %
>> splitname)
>>       0530:        else:
>>       0531:            spec_files_bottom.append('%%files -n %s' %
>> splitname)
>> File:
>> '/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
>> lineno: 249, function: walk_files
>>       0245:                    target.append(get_attr(dir) + '%dir "' +
>> escape_chars(p) + '"')
>>       0246:            else:
>>       0247:                # packages own only empty directories or
>> explict directory.
>>       0248:                # This will prevent the overlapping of
>> security permission.
>>   *** 0249:                attr = get_attr(path)
>>       0250:                if path and not files and not dirs:
>>       0251:                    target.append(attr + '%dir "' +
>> escape_chars(path) + '"')
>>       0252:                elif path and path in dirfiles:
>>       0253:                    target.append(attr + '%dir "' +
>> escape_chars(path) + '"')
>> File:
>> '/build/../layers/poky/meta/classes-global/package_rpm.bbclass',
>> lineno: 203, function: get_attr
>>       0199:        # of the walk, the isdir() test would then fail and
>> the walk code would assume its a file
>>       0200:        # hence we check for the names in files too.
>>       0201:        for rootpath, dirs, files in os.walk(walkpath):
>>       0202:            def get_attr(path):
>>   *** 0203:                stat_f = os.stat(rootpath + "/" + path,
>> follow_symlinks=False)
>>       0204:                mode = stat.S_IMODE(stat_f.st_mode)
>>       0205:                try:
>>       0206:                    owner =
>> pwd.getpwuid(stat_f.st_uid).pw_name
>>       0207:                except Exception as e:
>> Exception: FileNotFoundError: [Errno 2] No such file or directory:
>> '/build/tmp/work/corei7-64-montavista-linux/mvtest/2.0/packages-split/mvtest-lic/opt//opt'
>>
>> Signed-off-by: Jeremy A. Puhlman <jpuhlman@mvista.com>
>> ---
>>   meta/classes-global/package_rpm.bbclass | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass
>> index 2e3e4e8c79..a6885ac74e 100644
>> --- a/meta/classes-global/package_rpm.bbclass
>> +++ b/meta/classes-global/package_rpm.bbclass
>> @@ -200,6 +200,8 @@ python write_specfile () {
>>           # hence we check for the names in files too.
>>           for rootpath, dirs, files in os.walk(walkpath):
>>               def get_attr(path):
>> +                if not os.path.exists(rootpath + "/" + path):
>> +                   return ""
>>                   stat_f = os.stat(rootpath + "/" + path, follow_symlinks=False)
>>                   mode = stat.S_IMODE(stat_f.st_mode)
>>                   try:
>> @@ -243,7 +245,7 @@ python write_specfile () {
>>                       p = path + '/' + dir
>>                       # All packages own the directories their files are in...
>>                       target.append(get_attr(dir) + '%dir "' + escape_chars(p) + '"')
>> -            elif path:
>> +            elif path and os.path.exists(path):
>>                   # packages own only empty directories or explict directory.
>>                   # This will prevent the overlapping of security permission.
>>                   attr = get_attr(path)
>> @@ -257,7 +259,7 @@ python write_specfile () {
>>                   p = path + '/' + file
>>                   if conffiles.count(p):
>>                       target.append(attr + '%config "' + escape_chars(p) + '"')
>> -                else:
>> +                elif attr:
>>                       target.append(attr + '"' + escape_chars(p) + '"')
>>
>>       # Prevent the prerm/postrm scripts from being run during an upgrade
>> --
>> 2.31.1
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#196109): https://lists.openembedded.org/g/openembedded-core/message/196109
>> Mute This Topic: https://lists.openembedded.org/mt/104536226/1686489
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
Alexander Kanavin Feb. 24, 2024, 8:39 a.m. UTC | #3 
On Fri, 23 Feb 2024 at 23:58, Jeremy Puhlman <jpuhlman@mvista.com> wrote:
> Is there a preferred way to do that? I mention in the commit message
> that is not
> the right way to handle it and followed up with a reply to the patch
> mentioning
> as such.

You can simply use 'git send-email --rfc' to put [RFC PATCH] into the
subject. And have the word 'workaround' in the header too.

Alex
diff mbox series

Patch

diff --git a/meta/classes-global/package_rpm.bbclass b/meta/classes-global/package_rpm.bbclass
index 2e3e4e8c79..a6885ac74e 100644
--- a/meta/classes-global/package_rpm.bbclass
+++ b/meta/classes-global/package_rpm.bbclass
@@ -200,6 +200,8 @@  python write_specfile () {
         # hence we check for the names in files too.
         for rootpath, dirs, files in os.walk(walkpath):
             def get_attr(path):
+                if not os.path.exists(rootpath + "/" + path):
+                   return "" 
                 stat_f = os.stat(rootpath + "/" + path, follow_symlinks=False)
                 mode = stat.S_IMODE(stat_f.st_mode)
                 try:
@@ -243,7 +245,7 @@  python write_specfile () {
                     p = path + '/' + dir
                     # All packages own the directories their files are in...
                     target.append(get_attr(dir) + '%dir "' + escape_chars(p) + '"')
-            elif path:
+            elif path and os.path.exists(path):
                 # packages own only empty directories or explict directory.
                 # This will prevent the overlapping of security permission.
                 attr = get_attr(path)
@@ -257,7 +259,7 @@  python write_specfile () {
                 p = path + '/' + file
                 if conffiles.count(p):
                     target.append(attr + '%config "' + escape_chars(p) + '"')
-                else:
+                elif attr:
                     target.append(attr + '"' + escape_chars(p) + '"')
 
     # Prevent the prerm/postrm scripts from being run during an upgrade