[v2] insane: add sanity checks to SRC_URI

Message ID 20190517233743.9646-1-ross.burton@intel.com
State Accepted
Commit 21f84fcdd659544437fe393285c407e1e9432043
Headers show
Series
  • [v2] insane: add sanity checks to SRC_URI
Related show

Commit Message

Ross Burton May 17, 2019, 11:37 p.m.
The SRC_URI almost definitely shouldn't be using ${PN}, and GitHub */archive/*
tarballs are dynamically generated so the checksums will change over time.

Detect both of these, and emit a QA warning if found.

Signed-off-by: Ross Burton <ross.burton@intel.com>

---
 meta/classes/insane.bbclass | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

-- 
2.20.1 (Apple Git-117)

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Comments

Adrian Bunk May 18, 2019, 11:52 a.m. | #1
On Fri, May 17, 2019 at 04:37:43PM -0700, Ross Burton wrote:
> The SRC_URI almost definitely shouldn't be using ${PN}, and GitHub */archive/*

> tarballs are dynamically generated so the checksums will change over time.

>...


It might be worth mentioning that this is for archive, not releases.

GitHub release tarballs are stable, and they are preferable to commit ids.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Martin Jansa May 22, 2019, 3:48 p.m. | #2
Can we add an option to skip this with INSANE_SKIP?

It looks like QARECIPETEST doesn't use INSANE_SKIP or I don't see how.

Removing src-uri-bad from ERROR_QA/WARN_QA for some recipes works as well,
is it worth adding INSANE_SKIP for consistency with other checks or not?


On Sat, May 18, 2019 at 1:37 AM Ross Burton <ross.burton@intel.com> wrote:

> The SRC_URI almost definitely shouldn't be using ${PN}, and GitHub

> */archive/*

> tarballs are dynamically generated so the checksums will change over time.

>

> Detect both of these, and emit a QA warning if found.

>

> Signed-off-by: Ross Burton <ross.burton@intel.com>

> ---

>  meta/classes/insane.bbclass | 13 ++++++++++++-

>  1 file changed, 12 insertions(+), 1 deletion(-)

>

> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass

> index 9ca5aefe544..59bb8be5470 100644

> --- a/meta/classes/insane.bbclass

> +++ b/meta/classes/insane.bbclass

> @@ -25,7 +25,7 @@ QA_SANE = "True"

>  WARN_QA ?= "ldflags useless-rpaths rpaths staticdev libdir

> xorg-driver-abi \

>              textrel already-stripped incompatible-license files-invalid \

>              installed-vs-shipped compile-host-path install-host-path \

> -            pn-overrides infodir build-deps \

> +            pn-overrides infodir build-deps src-uri-bad \

>              unknown-configure-option symlink-to-sysroot multilib \

>              invalid-packageconfig host-user-contaminated uppercase-pn

> patch-fuzz \

>              "

> @@ -898,6 +898,17 @@ def package_qa_check_host_user(path, name, d, elf,

> messages):

>              return False

>      return True

>

> +QARECIPETEST[src-uri-bad] = "package_qa_check_src_uri"

> +def package_qa_check_src_uri(pn, d, messages):

> +    import re

> +

> +    if "${PN}" in d.getVar("SRC_URI", False):

> +        package_qa_handle_error("src-uri-bad", "%s: SRC_URI uses PN not

> BPN" % pn, d)

> +

> +    pn = d.getVar("SRC_URI")

> +    if re.search(r"github\.com/.+/.+/archive/.+", pn):

> +        package_qa_handle_error("src-uri-bad", "%s: SRC_URI uses unstable

> GitHub archives" % pn, d)

> +

>

>  # The PACKAGE FUNC to scan each package

>  python do_package_qa () {

> --

> 2.20.1 (Apple Git-117)

>

> --

> _______________________________________________

> Openembedded-core mailing list

> Openembedded-core@lists.openembedded.org

> http://lists.openembedded.org/mailman/listinfo/openembedded-core

>
<div dir="ltr">Can we add an option to skip this with INSANE_SKIP?<div><br></div><div>It looks like QARECIPETEST doesn&#39;t use INSANE_SKIP or I don&#39;t see how.</div><div><br></div><div>Removing src-uri-bad from ERROR_QA/WARN_QA for some recipes works as well, is it worth adding INSANE_SKIP for consistency with other checks or not?</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, May 18, 2019 at 1:37 AM Ross Burton &lt;<a href="mailto:ross.burton@intel.com" target="_blank">ross.burton@intel.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The SRC_URI almost definitely shouldn&#39;t be using ${PN}, and GitHub */archive/*<br>
tarballs are dynamically generated so the checksums will change over time.<br>
<br>
Detect both of these, and emit a QA warning if found.<br>
<br>
Signed-off-by: Ross Burton &lt;<a href="mailto:ross.burton@intel.com" target="_blank">ross.burton@intel.com</a>&gt;<br>

---<br>
 meta/classes/insane.bbclass | 13 ++++++++++++-<br>
 1 file changed, 12 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass<br>
index 9ca5aefe544..59bb8be5470 100644<br>
--- a/meta/classes/insane.bbclass<br>
+++ b/meta/classes/insane.bbclass<br>
@@ -25,7 +25,7 @@ QA_SANE = &quot;True&quot;<br>
 WARN_QA ?= &quot;ldflags useless-rpaths rpaths staticdev libdir xorg-driver-abi \<br>
             textrel already-stripped incompatible-license files-invalid \<br>
             installed-vs-shipped compile-host-path install-host-path \<br>
-            pn-overrides infodir build-deps \<br>
+            pn-overrides infodir build-deps src-uri-bad \<br>
             unknown-configure-option symlink-to-sysroot multilib \<br>
             invalid-packageconfig host-user-contaminated uppercase-pn patch-fuzz \<br>
             &quot;<br>
@@ -898,6 +898,17 @@ def package_qa_check_host_user(path, name, d, elf, messages):<br>
             return False<br>
     return True<br>
<br>
+QARECIPETEST[src-uri-bad] = &quot;package_qa_check_src_uri&quot;<br>
+def package_qa_check_src_uri(pn, d, messages):<br>
+    import re<br>
+<br>
+    if &quot;${PN}&quot; in d.getVar(&quot;SRC_URI&quot;, False):<br>
+        package_qa_handle_error(&quot;src-uri-bad&quot;, &quot;%s: SRC_URI uses PN not BPN&quot; % pn, d)<br>
+<br>
+    pn = d.getVar(&quot;SRC_URI&quot;)<br>
+    if re.search(r&quot;github\.com/.+/.+/archive/.+&quot;, pn):<br>
+        package_qa_handle_error(&quot;src-uri-bad&quot;, &quot;%s: SRC_URI uses unstable GitHub archives&quot; % pn, d)<br>
+<br>
<br>
 # The PACKAGE FUNC to scan each package<br>
 python do_package_qa () {<br>
-- <br>
2.20.1 (Apple Git-117)<br>
<br>
-- <br>
_______________________________________________<br>
Openembedded-core mailing list<br>
<a href="mailto:Openembedded-core@lists.openembedded.org" target="_blank">Openembedded-core@lists.openembedded.org</a><br>
<a href="http://lists.openembedded.org/mailman/listinfo/openembedded-core" rel="noreferrer" target="_blank">http://lists.openembedded.org/mailman/listinfo/openembedded-core</a><br>
</blockquote></div>
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Richard Purdie May 22, 2019, 10:23 p.m. | #3
On Wed, 2019-05-22 at 17:48 +0200, Martin Jansa wrote:
> Can we add an option to skip this with INSANE_SKIP?

> 

> It looks like QARECIPETEST doesn't use INSANE_SKIP or I don't see

> how.

> 

> Removing src-uri-bad from ERROR_QA/WARN_QA for some recipes works as

> well, is it worth adding INSANE_SKIP for consistency with other

> checks or not?


Ultimately I'd say that all these checks should work with INSANE_SKIP.
I was just wondering about this issue myself for the chkconfig
autobuilder warnings...

Cheers,

Richard

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Alexander Kanavin May 23, 2019, 9:24 a.m. | #4
On Thu, 23 May 2019 at 00:24, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> Ultimately I'd say that all these checks should work with INSANE_SKIP.

> I was just wondering about this issue myself for the chkconfig

> autobuilder warnings...


chkconfig can be easily addressed by switching to git:// I think. Patch coming.

Alex
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Patch

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 9ca5aefe544..59bb8be5470 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -25,7 +25,7 @@  QA_SANE = "True"
 WARN_QA ?= "ldflags useless-rpaths rpaths staticdev libdir xorg-driver-abi \
             textrel already-stripped incompatible-license files-invalid \
             installed-vs-shipped compile-host-path install-host-path \
-            pn-overrides infodir build-deps \
+            pn-overrides infodir build-deps src-uri-bad \
             unknown-configure-option symlink-to-sysroot multilib \
             invalid-packageconfig host-user-contaminated uppercase-pn patch-fuzz \
             "
@@ -898,6 +898,17 @@  def package_qa_check_host_user(path, name, d, elf, messages):
             return False
     return True
 
+QARECIPETEST[src-uri-bad] = "package_qa_check_src_uri"
+def package_qa_check_src_uri(pn, d, messages):
+    import re
+
+    if "${PN}" in d.getVar("SRC_URI", False):
+        package_qa_handle_error("src-uri-bad", "%s: SRC_URI uses PN not BPN" % pn, d)
+
+    pn = d.getVar("SRC_URI")
+    if re.search(r"github\.com/.+/.+/archive/.+", pn):
+        package_qa_handle_error("src-uri-bad", "%s: SRC_URI uses unstable GitHub archives" % pn, d)
+
 
 # The PACKAGE FUNC to scan each package
 python do_package_qa () {