| Message ID | 20190517233743.9646-1-ross.burton@intel.com |
|---|---|
| State | Accepted |
| Commit | 21f84fcdd659544437fe393285c407e1e9432043 |
| Headers | show |
| Series | [v2] insane: add sanity checks to SRC_URI | expand |
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
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't use INSANE_SKIP or I don'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 <<a href="mailto:ross.burton@intel.com" target="_blank">ross.burton@intel.com</a>> 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'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 <<a href="mailto:ross.burton@intel.com" target="_blank">ross.burton@intel.com</a>><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 = "True"<br> WARN_QA ?= "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> "<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] = "package_qa_check_src_uri"<br> +def package_qa_check_src_uri(pn, d, messages):<br> + import re<br> +<br> + if "${PN}" in d.getVar("SRC_URI", False):<br> + package_qa_handle_error("src-uri-bad", "%s: SRC_URI uses PN not BPN" % pn, d)<br> +<br> + pn = d.getVar("SRC_URI")<br> + if re.search(r"github\.com/.+/.+/archive/.+", pn):<br> + package_qa_handle_error("src-uri-bad", "%s: SRC_URI uses unstable GitHub archives" % 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
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
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
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 () {
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