[PATCH v2] insane: add sanity checks to SRC_URI

[PATCH v2] insane: add sanity checks to SRC_URI

[Ross Burton]

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)

Re: [PATCH v2] insane: add sanity checks to SRC_URI

[Adrian Bunk]

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

Re: [PATCH v2] insane: add sanity checks to SRC_URI

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]

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
>

[-- Attachment #2: Type: text/html, Size: 3343 bytes --]

Re: [PATCH v2] insane: add sanity checks to SRC_URI

[Richard Purdie]

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

Re: [PATCH v2] insane: add sanity checks to SRC_URI

[Alexander Kanavin]

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