cve-check: move update_symlinks to a library

Message ID 20220603085827.2214734-1-rybczynska@gmail.com
State Accepted, archived
Commit debd37abcdde8788761ebdb4a05bc61f7394cbb8
Headers show
Series cve-check: move update_symlinks to a library | expand

Commit Message

Marta Rybczynska June 3, 2022, 8:58 a.m. UTC
Move the function to a library, it could be useful in other places.

Signed-off-by: Marta Rybczynska <marta.rybczynska@huawei.com>
---
 meta/classes/cve-check.bbclass | 11 +++--------
 meta/lib/oe/cve_check.py       | 10 ++++++++++
 2 files changed, 13 insertions(+), 8 deletions(-)

Comments

Ernst Persson June 4, 2022, 6:15 a.m. UTC | #1
It's not specially related to cve_check, you could put it in
lib/oe/utils.py or something like that perhaps?

Regards
//Ernst

Den fre 3 juni 2022 kl 10:58 skrev Marta Rybczynska <rybczynska@gmail.com>:

> Move the function to a library, it could be useful in other places.
>
> Signed-off-by: Marta Rybczynska <marta.rybczynska@huawei.com>
> ---
>  meta/classes/cve-check.bbclass | 11 +++--------
>  meta/lib/oe/cve_check.py       | 10 ++++++++++
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/meta/classes/cve-check.bbclass
> b/meta/classes/cve-check.bbclass
> index c5c22c4c51..5eb0622d00 100644
> --- a/meta/classes/cve-check.bbclass
> +++ b/meta/classes/cve-check.bbclass
> @@ -80,16 +80,10 @@ CVE_CHECK_LAYER_INCLUDELIST ??= ""
>  # set to "alphabetical" for version using single alphabetical character
> as increment release
>  CVE_VERSION_SUFFIX ??= ""
>
> -def update_symlinks(target_path, link_path):
> -    if link_path != target_path and os.path.exists(target_path):
> -        if os.path.exists(os.path.realpath(link_path)):
> -            os.remove(link_path)
> -        os.symlink(os.path.basename(target_path), link_path)
> -
>  def generate_json_report(d, out_path, link_path):
>      if os.path.exists(d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")):
>          import json
> -        from oe.cve_check import cve_check_merge_jsons
> +        from oe.cve_check import cve_check_merge_jsons, update_symlinks
>
>          bb.note("Generating JSON CVE summary")
>          index_file = d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")
> @@ -110,6 +104,7 @@ def generate_json_report(d, out_path, link_path):
>  python cve_save_summary_handler () {
>      import shutil
>      import datetime
> +    from oe.cve_check import update_symlinks
>
>      cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
>
> @@ -179,7 +174,7 @@ python cve_check_write_rootfs_manifest () {
>      import shutil
>      import json
>      from oe.rootfs import image_list_installed_packages
> -    from oe.cve_check import cve_check_merge_jsons
> +    from oe.cve_check import cve_check_merge_jsons, update_symlinks
>
>      if d.getVar("CVE_CHECK_COPY_FILES") == "1":
>          deploy_file = d.getVar("CVE_CHECK_RECIPE_FILE")
> diff --git a/meta/lib/oe/cve_check.py b/meta/lib/oe/cve_check.py
> index dc7d2e2826..aa06497727 100644
> --- a/meta/lib/oe/cve_check.py
> +++ b/meta/lib/oe/cve_check.py
> @@ -163,3 +163,13 @@ def cve_check_merge_jsons(output, data):
>              return
>
>      output["package"].append(data["package"][0])
> +
> +def update_symlinks(target_path, link_path):
> +    """
> +    Update a symbolic link link_path to point to target_path.
> +    Remove the link and recreate it if exist and is different.
> +    """
> +    if link_path != target_path and os.path.exists(target_path):
> +        if os.path.exists(os.path.realpath(link_path)):
> +            os.remove(link_path)
> +        os.symlink(os.path.basename(target_path), link_path)
> --
> 2.33.0
>
>
Pavel Zhukov June 6, 2022, 6:03 a.m. UTC | #2
Looks like update_symplinks is widely used accros OE. Quick grepping gave me this:
[1]. I'm sure there're more occurences. 
This should be somewhere in more accesible place (oe-core libraries).


[1] 
meta/lib/oeqa/runexported.py:            os.remove(sshloglink)
meta/lib/oeqa/runexported.py-        os.symlink(self.sshlog, sshloglink)
--
meta/lib/oeqa/selftest/context.py:                os.remove(output_link)
meta/lib/oeqa/selftest/context.py-            os.symlink(args.output_log, output_link)
--
meta/lib/oeqa/runtime/cases/ltp.py:            os.remove(cls.ltptest_log_dir_link)
meta/lib/oeqa/runtime/cases/ltp.py-        os.symlink(os.path.basename(cls.ltptest_log_dir), cls.ltptest_log_dir_link)
--
meta/lib/oeqa/runtime/cases/ptest.py:            os.remove(ptest_log_dir_link)
meta/lib/oeqa/runtime/cases/ptest.py-        os.symlink(os.path.basename(ptest_log_dir), ptest_log_dir_link)
--
meta/lib/oeqa/utils/decorators.py:            os.remove(linkfile)
meta/lib/oeqa/utils/decorators.py-        os.symlink(logfile, linkfile)
--
meta/lib/oeqa/core/context.py:            os.remove(output_link)
meta/lib/oeqa/core/context.py-        os.symlink(args.output_log, output_link)
--
meta/classes/baremetal-image.bbclass:            os.remove(manifest_link)
meta/classes/baremetal-image.bbclass-        os.symlink(os.path.basename(manifest_name), manifest_link)
--
meta/classes/qemuboot.bbclass:           os.remove(qemuboot_link)
meta/classes/qemuboot.bbclass-        os.symlink(os.path.basename(qemuboot), qemuboot_link)
--
meta/classes/cve-check.bbclass:            os.remove(link_path)
meta/classes/cve-check.bbclass-        os.symlink(os.path.basename(target_path), link_path)
--
meta/classes/rootfs-postcommands.bbclass:                os.remove(manifest_link)
meta/classes/rootfs-postcommands.bbclass-            os.symlink(os.path.basename(manifest_name), manifest_link)
--
meta/classes/rootfs-postcommands.bbclass:                os.remove(testdata_link)
meta/classes/rootfs-postcommands.bbclass-            os.symlink(os.path.basename(testdata_name), testdata_link)

"Marta Rybczynska" <rybczynska@gmail.com> writes:

> Move the function to a library, it could be useful in other places.
>
> Signed-off-by: Marta Rybczynska <marta.rybczynska@huawei.com>
> ---
>  meta/classes/cve-check.bbclass | 11 +++--------
>  meta/lib/oe/cve_check.py       | 10 ++++++++++
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
> index c5c22c4c51..5eb0622d00 100644
> --- a/meta/classes/cve-check.bbclass
> +++ b/meta/classes/cve-check.bbclass
> @@ -80,16 +80,10 @@ CVE_CHECK_LAYER_INCLUDELIST ??= ""
>  # set to "alphabetical" for version using single alphabetical character as increment release
>  CVE_VERSION_SUFFIX ??= ""
>  
> -def update_symlinks(target_path, link_path):
> -    if link_path != target_path and os.path.exists(target_path):
> -        if os.path.exists(os.path.realpath(link_path)):
> -            os.remove(link_path)
> -        os.symlink(os.path.basename(target_path), link_path)
> -
>  def generate_json_report(d, out_path, link_path):
>      if os.path.exists(d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")):
>          import json
> -        from oe.cve_check import cve_check_merge_jsons
> +        from oe.cve_check import cve_check_merge_jsons, update_symlinks
>  
>          bb.note("Generating JSON CVE summary")
>          index_file = d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")
> @@ -110,6 +104,7 @@ def generate_json_report(d, out_path, link_path):
>  python cve_save_summary_handler () {
>      import shutil
>      import datetime
> +    from oe.cve_check import update_symlinks
>  
>      cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
>  
> @@ -179,7 +174,7 @@ python cve_check_write_rootfs_manifest () {
>      import shutil
>      import json
>      from oe.rootfs import image_list_installed_packages
> -    from oe.cve_check import cve_check_merge_jsons
> +    from oe.cve_check import cve_check_merge_jsons, update_symlinks
>  
>      if d.getVar("CVE_CHECK_COPY_FILES") == "1":
>          deploy_file = d.getVar("CVE_CHECK_RECIPE_FILE")
> diff --git a/meta/lib/oe/cve_check.py b/meta/lib/oe/cve_check.py
> index dc7d2e2826..aa06497727 100644
> --- a/meta/lib/oe/cve_check.py
> +++ b/meta/lib/oe/cve_check.py
> @@ -163,3 +163,13 @@ def cve_check_merge_jsons(output, data):
>              return
>  
>      output["package"].append(data["package"][0])
> +
> +def update_symlinks(target_path, link_path):
> +    """
> +    Update a symbolic link link_path to point to target_path.
> +    Remove the link and recreate it if exist and is different.
> +    """
> +    if link_path != target_path and os.path.exists(target_path):
> +        if os.path.exists(os.path.realpath(link_path)):
> +            os.remove(link_path)
> +        os.symlink(os.path.basename(target_path), link_path)
Marta Rybczynska June 8, 2022, 7:51 a.m. UTC | #3
On Mon, Jun 6, 2022 at 8:08 AM Pavel Zhukov <pavel@zhukoff.net> wrote:
>
>
> Looks like update_symplinks is widely used accros OE. Quick grepping gave me this:
> [1]. I'm sure there're more occurences.
> This should be somewhere in more accesible place (oe-core libraries).
>
>

This is a valid point Pavel and Ernst. The patch has landed in master
so I will do a followup.
It seems that this could avoid a number of bugs (see the latest fix
from Davide).

Regards,
Marta
Ernst Persson June 8, 2022, 9:24 a.m. UTC | #4
There's also a big create_symlinks() in image.bbclass that could use this
(partially).

//E

Den ons 8 juni 2022 kl 09:51 skrev Marta Rybczynska <rybczynska@gmail.com>:

> On Mon, Jun 6, 2022 at 8:08 AM Pavel Zhukov <pavel@zhukoff.net> wrote:
> >
> >
> > Looks like update_symplinks is widely used accros OE. Quick grepping
> gave me this:
> > [1]. I'm sure there're more occurences.
> > This should be somewhere in more accesible place (oe-core libraries).
> >
> >
>
> This is a valid point Pavel and Ernst. The patch has landed in master
> so I will do a followup.
> It seems that this could avoid a number of bugs (see the latest fix
> from Davide).
>
> Regards,
> Marta
>
Richard Purdie June 8, 2022, 12:27 p.m. UTC | #5
On Wed, 2022-06-08 at 09:51 +0200, Marta Rybczynska wrote:
> On Mon, Jun 6, 2022 at 8:08 AM Pavel Zhukov <pavel@zhukoff.net> wrote:
> > 
> > 
> > Looks like update_symplinks is widely used accros OE. Quick grepping gave me this:
> > [1]. I'm sure there're more occurences.
> > This should be somewhere in more accesible place (oe-core libraries).
> > 
> > 
> 
> This is a valid point Pavel and Ernst. The patch has landed in master
> so I will do a followup.
> It seems that this could avoid a number of bugs (see the latest fix
> from Davide).

Thanks, I was meaning to say that I merged it since it does
incrementally improve things but I'd be happy to see this handled in
other places more centrally too. We have see a few similar bugs
recently.

Further incremental improvements very welcome!

Cheers,

Richard

Patch

diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
index c5c22c4c51..5eb0622d00 100644
--- a/meta/classes/cve-check.bbclass
+++ b/meta/classes/cve-check.bbclass
@@ -80,16 +80,10 @@  CVE_CHECK_LAYER_INCLUDELIST ??= ""
 # set to "alphabetical" for version using single alphabetical character as increment release
 CVE_VERSION_SUFFIX ??= ""
 
-def update_symlinks(target_path, link_path):
-    if link_path != target_path and os.path.exists(target_path):
-        if os.path.exists(os.path.realpath(link_path)):
-            os.remove(link_path)
-        os.symlink(os.path.basename(target_path), link_path)
-
 def generate_json_report(d, out_path, link_path):
     if os.path.exists(d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")):
         import json
-        from oe.cve_check import cve_check_merge_jsons
+        from oe.cve_check import cve_check_merge_jsons, update_symlinks
 
         bb.note("Generating JSON CVE summary")
         index_file = d.getVar("CVE_CHECK_SUMMARY_INDEX_PATH")
@@ -110,6 +104,7 @@  def generate_json_report(d, out_path, link_path):
 python cve_save_summary_handler () {
     import shutil
     import datetime
+    from oe.cve_check import update_symlinks
 
     cve_tmp_file = d.getVar("CVE_CHECK_TMP_FILE")
 
@@ -179,7 +174,7 @@  python cve_check_write_rootfs_manifest () {
     import shutil
     import json
     from oe.rootfs import image_list_installed_packages
-    from oe.cve_check import cve_check_merge_jsons
+    from oe.cve_check import cve_check_merge_jsons, update_symlinks
 
     if d.getVar("CVE_CHECK_COPY_FILES") == "1":
         deploy_file = d.getVar("CVE_CHECK_RECIPE_FILE")
diff --git a/meta/lib/oe/cve_check.py b/meta/lib/oe/cve_check.py
index dc7d2e2826..aa06497727 100644
--- a/meta/lib/oe/cve_check.py
+++ b/meta/lib/oe/cve_check.py
@@ -163,3 +163,13 @@  def cve_check_merge_jsons(output, data):
             return
 
     output["package"].append(data["package"][0])
+
+def update_symlinks(target_path, link_path):
+    """
+    Update a symbolic link link_path to point to target_path.
+    Remove the link and recreate it if exist and is different.
+    """
+    if link_path != target_path and os.path.exists(target_path):
+        if os.path.exists(os.path.realpath(link_path)):
+            os.remove(link_path)
+        os.symlink(os.path.basename(target_path), link_path)