[PATCH] package.bbclass: handle links in sorted order

[PATCH] package.bbclass: handle links in sorted order

[Bill Randle]

When processing links, the directories are processed in unsorted order
which can result in cases like /var/lock -> /run/lock handled before
/var/run -> /run throwing an error for /var/run because /run already exists.
Change the link processing to ensure links are processed in sorted order of
the destination.

[YOCTO #9430]

Signed-off-by: Bill Randle <william.c.randle@intel.com>
---
 meta/classes/package.bbclass | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 4452e2f..894b729 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -727,6 +727,7 @@ python fixup_perms () {
     dvar = d.getVar('PKGD', True)
 
     fs_perms_table = {}
+    fs_link_table = {}
 
     # By default all of the standard directories specified in
     # bitbake.conf will get 0755 root:root.
@@ -773,24 +774,27 @@ python fixup_perms () {
                     continue
                 entry = fs_perms_entry(d.expand(line))
                 if entry and entry.path:
-                    fs_perms_table[entry.path] = entry
+                    if entry.link:
+                        fs_link_table[entry.link] = entry
+                    else:
+                        fs_perms_table[entry.path] = entry
             f.close()
 
     # Debug -- list out in-memory table
     #for dir in fs_perms_table:
     #    bb.note("Fixup Perms: %s: %s" % (dir, str(fs_perms_table[dir])))
+    #for link in fs_link_table:
+    #    bb.note("Fixup Perms: %s: %s" % (link, str(fs_link_table[link])))
 
     # We process links first, so we can go back and fixup directory ownership
     # for any newly created directories
-    for dir in fs_perms_table:
-        if not fs_perms_table[dir].link:
-            continue
-
+    # Process in sorted order so /run gets created before /run/lock, etc.
+    for link in sorted(fs_link_table):
+        dir = fs_link_table[link].path
         origin = dvar + dir
         if not (cpath.exists(origin) and cpath.isdir(origin) and not cpath.islink(origin)):
             continue
 
-        link = fs_perms_table[dir].link
         if link[0] == "/":
             target = dvar + link
             ptarget = link
@@ -810,9 +814,6 @@ python fixup_perms () {
         os.symlink(link, origin)
 
     for dir in fs_perms_table:
-        if fs_perms_table[dir].link:
-            continue
-
         origin = dvar + dir
         if not (cpath.exists(origin) and cpath.isdir(origin)):
             continue
-- 
2.5.0

Re: [PATCH] package.bbclass: handle links in sorted order

[Denys Dmytriyenko]

On Tue, Apr 12, 2016 at 08:22:21AM -0700, Bill Randle wrote:
> When processing links, the directories are processed in unsorted order
> which can result in cases like /var/lock -> /run/lock handled before
> /var/run -> /run throwing an error for /var/run because /run already exists.
> Change the link processing to ensure links are processed in sorted order of
> the destination.

Seems to resolve the issue for me.


> [YOCTO #9430]
> 
> Signed-off-by: Bill Randle <william.c.randle@intel.com>

Reported-by: Denys Dmytriyenko <denys@ti.com>
Tested-by: Denys Dmytriyenko <denys@ti.com>


> ---
>  meta/classes/package.bbclass | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index 4452e2f..894b729 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -727,6 +727,7 @@ python fixup_perms () {
>      dvar = d.getVar('PKGD', True)
>  
>      fs_perms_table = {}
> +    fs_link_table = {}
>  
>      # By default all of the standard directories specified in
>      # bitbake.conf will get 0755 root:root.
> @@ -773,24 +774,27 @@ python fixup_perms () {
>                      continue
>                  entry = fs_perms_entry(d.expand(line))
>                  if entry and entry.path:
> -                    fs_perms_table[entry.path] = entry
> +                    if entry.link:
> +                        fs_link_table[entry.link] = entry
> +                    else:
> +                        fs_perms_table[entry.path] = entry
>              f.close()
>  
>      # Debug -- list out in-memory table
>      #for dir in fs_perms_table:
>      #    bb.note("Fixup Perms: %s: %s" % (dir, str(fs_perms_table[dir])))
> +    #for link in fs_link_table:
> +    #    bb.note("Fixup Perms: %s: %s" % (link, str(fs_link_table[link])))
>  
>      # We process links first, so we can go back and fixup directory ownership
>      # for any newly created directories
> -    for dir in fs_perms_table:
> -        if not fs_perms_table[dir].link:
> -            continue
> -
> +    # Process in sorted order so /run gets created before /run/lock, etc.
> +    for link in sorted(fs_link_table):
> +        dir = fs_link_table[link].path
>          origin = dvar + dir
>          if not (cpath.exists(origin) and cpath.isdir(origin) and not cpath.islink(origin)):
>              continue
>  
> -        link = fs_perms_table[dir].link
>          if link[0] == "/":
>              target = dvar + link
>              ptarget = link
> @@ -810,9 +814,6 @@ python fixup_perms () {
>          os.symlink(link, origin)
>  
>      for dir in fs_perms_table:
> -        if fs_perms_table[dir].link:
> -            continue
> -
>          origin = dvar + dir
>          if not (cpath.exists(origin) and cpath.isdir(origin)):
>              continue
> -- 
> 2.5.0
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] package.bbclass: handle links in sorted order

[Dan McGregor]

On 12 April 2016 at 10:51, Denys Dmytriyenko <denis@denix.org> wrote:
> On Tue, Apr 12, 2016 at 08:22:21AM -0700, Bill Randle wrote:
>> When processing links, the directories are processed in unsorted order
>> which can result in cases like /var/lock -> /run/lock handled before
>> /var/run -> /run throwing an error for /var/run because /run already exists.
>> Change the link processing to ensure links are processed in sorted order of
>> the destination.
>
> Seems to resolve the issue for me.
>
>

For me it introduces a new issue. I use two perm files, the OE default
one, and one that changes two links to real directories. The new
packaging code complains that a link target exists as a directory,
even though a subsequent rule sets it up as a directory.

I think during parsing it should remove links as it finds directories
with the same path, and remove directories with the same path in the
order it sees them, that way when it does the in order processing it
uses the last definition in the permissions table.


>> [YOCTO #9430]
>>
>> Signed-off-by: Bill Randle <william.c.randle@intel.com>
>
> Reported-by: Denys Dmytriyenko <denys@ti.com>
> Tested-by: Denys Dmytriyenko <denys@ti.com>
>
>
>> ---
>>  meta/classes/package.bbclass | 19 ++++++++++---------
>>  1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
>> index 4452e2f..894b729 100644
>> --- a/meta/classes/package.bbclass
>> +++ b/meta/classes/package.bbclass
>> @@ -727,6 +727,7 @@ python fixup_perms () {
>>      dvar = d.getVar('PKGD', True)
>>
>>      fs_perms_table = {}
>> +    fs_link_table = {}
>>
>>      # By default all of the standard directories specified in
>>      # bitbake.conf will get 0755 root:root.
>> @@ -773,24 +774,27 @@ python fixup_perms () {
>>                      continue
>>                  entry = fs_perms_entry(d.expand(line))
>>                  if entry and entry.path:
>> -                    fs_perms_table[entry.path] = entry
>> +                    if entry.link:
>> +                        fs_link_table[entry.link] = entry
>> +                    else:
>> +                        fs_perms_table[entry.path] = entry
>>              f.close()
>>
>>      # Debug -- list out in-memory table
>>      #for dir in fs_perms_table:
>>      #    bb.note("Fixup Perms: %s: %s" % (dir, str(fs_perms_table[dir])))
>> +    #for link in fs_link_table:
>> +    #    bb.note("Fixup Perms: %s: %s" % (link, str(fs_link_table[link])))
>>
>>      # We process links first, so we can go back and fixup directory ownership
>>      # for any newly created directories
>> -    for dir in fs_perms_table:
>> -        if not fs_perms_table[dir].link:
>> -            continue
>> -
>> +    # Process in sorted order so /run gets created before /run/lock, etc.
>> +    for link in sorted(fs_link_table):
>> +        dir = fs_link_table[link].path
>>          origin = dvar + dir
>>          if not (cpath.exists(origin) and cpath.isdir(origin) and not cpath.islink(origin)):
>>              continue
>>
>> -        link = fs_perms_table[dir].link
>>          if link[0] == "/":
>>              target = dvar + link
>>              ptarget = link
>> @@ -810,9 +814,6 @@ python fixup_perms () {
>>          os.symlink(link, origin)
>>
>>      for dir in fs_perms_table:
>> -        if fs_perms_table[dir].link:
>> -            continue
>> -
>>          origin = dvar + dir
>>          if not (cpath.exists(origin) and cpath.isdir(origin)):
>>              continue
>> --
>> 2.5.0
>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] package.bbclass: handle links in sorted order

[Randle, William C]

On Wed, 2016-04-13 at 11:30 -0600, Dan McGregor wrote:

On 12 April 2016 at 10:51, Denys Dmytriyenko <denis@denix.org<mailto:denis@denix.org>> wrote:


On Tue, Apr 12, 2016 at 08:22:21AM -0700, Bill Randle wrote:


When processing links, the directories are processed in unsorted order
which can result in cases like /var/lock -> /run/lock handled before
/var/run -> /run throwing an error for /var/run because /run already exists.
Change the link processing to ensure links are processed in sorted order of
the destination.



Seems to resolve the issue for me.





For me it introduces a new issue. I use two perm files, the OE default
one, and one that changes two links to real directories. The new
packaging code complains that a link target exists as a directory,
even though a subsequent rule sets it up as a directory.

I think during parsing it should remove links as it finds directories
with the same path, and remove directories with the same path in the
order it sees them, that way when it does the in order processing it
uses the last definition in the permissions table.



Dan, can you provide an example permissions file that illustrates this? Preferably via the Bugzilla entry, or if not convenient, then here?







[YOCTO #9430]

Signed-off-by: Bill Randle <william.c.randle@intel.com<mailto:william.c.randle@intel.com>>



Reported-by: Denys Dmytriyenko <denys@ti.com<mailto:denys@ti.com>>
Tested-by: Denys Dmytriyenko <denys@ti.com<mailto:denys@ti.com>>

Re: [PATCH] package.bbclass: handle links in sorted order

[Dan McGregor]

On 13 April 2016 at 11:40, Randle, William C <william.c.randle@intel.com> wrote:
> On Wed, 2016-04-13 at 11:30 -0600, Dan McGregor wrote:
>
> On 12 April 2016 at 10:51, Denys Dmytriyenko <denis@denix.org> wrote:
>
> On Tue, Apr 12, 2016 at 08:22:21AM -0700, Bill Randle wrote:
>
> When processing links, the directories are processed in unsorted order
> which can result in cases like /var/lock -> /run/lock handled before
> /var/run -> /run throwing an error for /var/run because /run already exists.
> Change the link processing to ensure links are processed in sorted order of
> the destination.
>
>
> Seems to resolve the issue for me.
>
>
>
>
> For me it introduces a new issue. I use two perm files, the OE default
> one, and one that changes two links to real directories. The new
> packaging code complains that a link target exists as a directory,
> even though a subsequent rule sets it up as a directory.
>
> I think during parsing it should remove links as it finds directories
> with the same path, and remove directories with the same path in the
> order it sees them, that way when it does the in order processing it
> uses the last definition in the permissions table.
>
>
> Dan, can you provide an example permissions file that illustrates this?
> Preferably via the Bugzilla entry, or if not convenient, then here?
>
>

Yep, I just commented on the bug.

>
> [YOCTO #9430]
>
> Signed-off-by: Bill Randle <william.c.randle@intel.com>
>
>
> Reported-by: Denys Dmytriyenko <denys@ti.com>
> Tested-by: Denys Dmytriyenko <denys@ti.com>
>

[PATCH] package.bbclass: improve permission handling

[Dan McGregor]

From: Dan McGregor <dan.mcgregor@usask.ca>

Change fs_link_table to be keyed by path, just like fs_perms_table.
When a new entry is coming in for either table, remove any previous
entry for that path. This way later permission file entries override
earlier ones.

[YOCTO #9430]

Signed-off-by: Dan McGregor <dan.mcgregor@usask.ca>
---
 meta/classes/package.bbclass | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 894b729..76b9f86 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -775,9 +775,13 @@ python fixup_perms () {
                 entry = fs_perms_entry(d.expand(line))
                 if entry and entry.path:
                     if entry.link:
-                        fs_link_table[entry.link] = entry
+                        fs_link_table[entry.path] = entry
+                        if entry.path in fs_perms_table:
+                            fs_perms_table.pop(entry.path)
                     else:
                         fs_perms_table[entry.path] = entry
+                        if entry.path in fs_link_table:
+                            fs_link_table.pop(entry.path)
             f.close()
 
     # Debug -- list out in-memory table
@@ -789,8 +793,9 @@ python fixup_perms () {
     # We process links first, so we can go back and fixup directory ownership
     # for any newly created directories
     # Process in sorted order so /run gets created before /run/lock, etc.
-    for link in sorted(fs_link_table):
-        dir = fs_link_table[link].path
+    for entry in sorted(fs_link_table.values(), key=lambda x: x.link):
+        link = entry.link
+        dir = entry.path
         origin = dvar + dir
         if not (cpath.exists(origin) and cpath.isdir(origin) and not cpath.islink(origin)):
             continue
-- 
2.8.1

Re: [PATCH] package.bbclass: improve permission handling

[Randle, William C]

On Wed, 2016-04-13 at 16:17 -0600, Dan McGregor wrote:

From: Dan McGregor <dan.mcgregor@usask.ca<mailto:dan.mcgregor@usask.ca>>

Change fs_link_table to be keyed by path, just like fs_perms_table.
When a new entry is coming in for either table, remove any previous
entry for that path. This way later permission file entries override
earlier ones.

[YOCTO #9430]

Signed-off-by: Dan McGregor <dan.mcgregor@usask.ca<mailto:dan.mcgregor@usask.ca>>
---
 meta/classes/package.bbclass | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 894b729..76b9f86 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -775,9 +775,13 @@ python fixup_perms () {
                 entry = fs_perms_entry(d.expand(line))
                 if entry and entry.path:
                     if entry.link:
-                        fs_link_table[entry.link] = entry
+                        fs_link_table[entry.path] = entry
+                        if entry.path in fs_perms_table:
+                            fs_perms_table.pop(entry.path)
                     else:
                         fs_perms_table[entry.path] = entry
+                        if entry.path in fs_link_table:
+                            fs_link_table.pop(entry.path)
             f.close()

     # Debug -- list out in-memory table
@@ -789,8 +793,9 @@ python fixup_perms () {
     # We process links first, so we can go back and fixup directory ownership
     # for any newly created directories
     # Process in sorted order so /run gets created before /run/lock, etc.
-    for link in sorted(fs_link_table):
-        dir = fs_link_table[link].path
+    for entry in sorted(fs_link_table.values(), key=lambda x: x.link):
+        link = entry.link
+        dir = entry.path
         origin = dvar + dir
         if not (cpath.exists(origin) and cpath.isdir(origin) and not cpath.islink(origin)):
             continue
--
2.8.1



Thanks, Dan! That looks good to me. One thought, though. Since you're looking for previous entries in the alternate tables (perms in link table and links in perm table), should we also consider searching the same table (link in links and perm in perms) in case someone used a private/local perms file to override a default (e.g., change a symlink /var/foo -> /run/foo to /var/foo -> /var/baz). In this case, it would keep the last one found, kind of like you're doing here. Just a thought.

    -Bill