diff mbox series

[V2,2/2] scripts/cve: Restart the clone if the pull generate an exception

Message ID 20240903085745.2594893-2-michael@amarulasolutions.com
State New
Headers
Series [V2,1/2] scripts/cve: Avoid to do a complete clone of cve git repository |

Commit Message

Michael Trimarchi Sept. 3, 2024, 8:57 a.m. UTC 
If we are not able to pull from the directory, restart from a clean
clone. This can happen for corrupt repository or unfinished download

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
V1->V2: Adjust the commit message

---
 support/scripts/cve.py | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

Comments

'Abel Vesa' via Amarula Linux Sept. 3, 2024, 6:52 p.m. UTC | #1 
On Tue,  3 Sep 2024 10:57:45 +0200
Michael Trimarchi <michael@amarulasolutions.com> wrote:

> diff --git a/support/scripts/cve.py b/support/scripts/cve.py
> index dcb3a63925..6cd9aab963 100755
> --- a/support/scripts/cve.py
> +++ b/support/scripts/cve.py
> @@ -21,6 +21,7 @@ import datetime
>  import os
>  import distutils.version
>  import json
> +import shutil
>  import subprocess
>  import sys
>  import operator
> @@ -69,15 +70,21 @@ class CVE:
>  
>      @staticmethod
>      def download_nvd(nvd_git_dir):
> +        done = False
>          print(f"Updating from {NVD_BASE_URL}")
>          if os.path.exists(nvd_git_dir):
> -            subprocess.check_call(
> -                ["git", "pull", "--depth", "1"],
> -                cwd=nvd_git_dir,
> -                stdout=subprocess.DEVNULL,
> -                stderr=subprocess.DEVNULL,
> -            )
> -        else:
> +            try:
> +                subprocess.check_call(
> +                    ["git", "pull", "--depth", "1"],
> +                    cwd=nvd_git_dir,
> +                    stdout=subprocess.DEVNULL,
> +                    stderr=subprocess.DEVNULL,
> +                )
> +                done = True
> +            except:
> +                shutil.rmtree(nvd_git_dir)

The thing I'm worried about is that you can also get a failure of "git
pull" for example due to a network timeout or something like that,
which doesn't need a full git clone, but just a "try again" later...
and now we're going to wipe out the entire local clone, and try to
clone everything again. Is that a good idea?

Also, perhaps we need to show an error message if the "git pull"
failed, and saying we're falling back to a full clone, or something?

> +        if (not done):

	   if not done:


is sufficient, we are not writing C code here :-)

Thanks!

Thomas
Michael Trimarchi Sept. 3, 2024, 6:55 p.m. UTC | #2 
Hi Thomas

On Tue, Sep 3, 2024 at 8:52 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Tue,  3 Sep 2024 10:57:45 +0200
> Michael Trimarchi <michael@amarulasolutions.com> wrote:
>
> > diff --git a/support/scripts/cve.py b/support/scripts/cve.py
> > index dcb3a63925..6cd9aab963 100755
> > --- a/support/scripts/cve.py
> > +++ b/support/scripts/cve.py
> > @@ -21,6 +21,7 @@ import datetime
> >  import os
> >  import distutils.version
> >  import json
> > +import shutil
> >  import subprocess
> >  import sys
> >  import operator
> > @@ -69,15 +70,21 @@ class CVE:
> >
> >      @staticmethod
> >      def download_nvd(nvd_git_dir):
> > +        done = False
> >          print(f"Updating from {NVD_BASE_URL}")
> >          if os.path.exists(nvd_git_dir):
> > -            subprocess.check_call(
> > -                ["git", "pull", "--depth", "1"],
> > -                cwd=nvd_git_dir,
> > -                stdout=subprocess.DEVNULL,
> > -                stderr=subprocess.DEVNULL,
> > -            )
> > -        else:
> > +            try:
> > +                subprocess.check_call(
> > +                    ["git", "pull", "--depth", "1"],
> > +                    cwd=nvd_git_dir,
> > +                    stdout=subprocess.DEVNULL,
> > +                    stderr=subprocess.DEVNULL,
> > +                )
> > +                done = True
> > +            except:
> > +                shutil.rmtree(nvd_git_dir)
>
> The thing I'm worried about is that you can also get a failure of "git
> pull" for example due to a network timeout or something like that,
> which doesn't need a full git clone, but just a "try again" later...
> and now we're going to wipe out the entire local clone, and try to
> clone everything again. Is that a good idea?
>
> Also, perhaps we need to show an error message if the "git pull"
> failed, and saying we're falling back to a full clone, or something?
>
> > +        if (not done):
>
>            if not done:
>
>
> is sufficient, we are not writing C code here :-)

Sorry, I will ask my colleague to write python and send a better
strategy. I totally agree with you.

Anyway I have a plan to add more information on cve reporting, hope
that you like the idea.
Mostly I'm playing with parser for jenkins and love to add buildroot
too but I don't have enough
information to show

https://github.com/jenkinsci/analysis-model/pull/1085

Michael


>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
diff mbox series

Patch

diff --git a/support/scripts/cve.py b/support/scripts/cve.py
index dcb3a63925..6cd9aab963 100755
--- a/support/scripts/cve.py
+++ b/support/scripts/cve.py
@@ -21,6 +21,7 @@  import datetime
 import os
 import distutils.version
 import json
+import shutil
 import subprocess
 import sys
 import operator
@@ -69,15 +70,21 @@  class CVE:
 
     @staticmethod
     def download_nvd(nvd_git_dir):
+        done = False
         print(f"Updating from {NVD_BASE_URL}")
         if os.path.exists(nvd_git_dir):
-            subprocess.check_call(
-                ["git", "pull", "--depth", "1"],
-                cwd=nvd_git_dir,
-                stdout=subprocess.DEVNULL,
-                stderr=subprocess.DEVNULL,
-            )
-        else:
+            try:
+                subprocess.check_call(
+                    ["git", "pull", "--depth", "1"],
+                    cwd=nvd_git_dir,
+                    stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL,
+                )
+                done = True
+            except:
+                shutil.rmtree(nvd_git_dir)
+
+        if (not done):
             # Create the directory and its parents; git
             # happily clones into an empty directory.
             os.makedirs(nvd_git_dir)