| Message ID | 20211201230827.35080-2-matthew.weber@collins.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [1/3] utils/test-pkg: add support for externals | expand |
Matthew, All, On 2021-12-01 17:08 -0600, Matthew Weber via buildroot spake thusly: > Add flag to allow the return value to be incremented when a skip > happens, treating the overall run as a failure. > > This flag could be used when using test-pkg for local validation of > downstream packages, for example in a user's br2-external. An example of > this could be a user choosing to use test-pkg as part of a CI > infrastructure where the fragment and toolchains are tightly controlled > to test a package against multiple toolchains easily. > > In this case, the user would want to lower the rate of false-positives > of that fragment/toolchain configuration if a build is instead skipped > (which may happen in the case of buildroot version bumps where > dependencies could change). I am a bit skeptical about this use-case. As I understand it, wht is most interesting in a CI setup for internal development, is that *all* internal packages do build togetrher and form a working sysem *as a whole*. As such, a CI setup would build the defconfig file(s) for a project (and run the integration test-suite), rather than build each package individually Building each package individually with test-pkg would have the disadvantage that it takes more time overall, as all the common deps are build as many times as there are packages to test, and package are also dependencies one of the others, so most packages would be built more than once as well... So, I am really not convinced... Regards, Yann E. MORIN. > Signed-off-by: Matthew Weber <matthew.weber@collins.com> > --- > utils/test-pkg | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/utils/test-pkg b/utils/test-pkg > index d0472f176b..97957008fa 100755 > --- a/utils/test-pkg > +++ b/utils/test-pkg > @@ -18,8 +18,8 @@ main() { > local -a toolchains > local pkg_br_name > > - o='hakc:d:n:p:r:t:e:' > - O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:,externals:,' > + o='hakc:d:n:p:r:t:e:f' > + O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:,externals:,fail-on-skip' > opts="$(getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}")" > eval set -- "${opts}" > > @@ -29,6 +29,7 @@ main() { > number=0 > mode=0 > prepare_only=0 > + fail_on_skip=0 > toolchains_csv="${TOOLCHAINS_CSV}" > while [ ${#} -gt 0 ]; do > case "${1}" in > @@ -65,6 +66,9 @@ main() { > (-e|--externals) > BR2_EXTERNALS="${2}"; shift 2 > ;; > + (-f|--fail-on-skip) > + fail_on_skip=1; shift 1 > + ;; > (--) > shift; break > ;; > @@ -147,7 +151,12 @@ main() { > printf "%d builds, %d skipped, %d build failed, %d legal-info failed\n" \ > ${nb} ${nb_skip} ${nb_fail} ${nb_legal} > > - return $((nb_fail + nb_legal)) > + nb_result=$((nb_fail + nb_legal)) > + if [ ${fail_on_skip} ]; then > + nb_result=$((nb_result + nb_skip)) > + fi > + > + return $nb_result > } > > build_one() { > @@ -279,6 +288,9 @@ Options: > Externals to be used as part of the build process. Packages from > within these externals may be tested. > > + -f, --fail-on-skip > + If any builds are skipped, return a negative exit value. > + > Example: > > Testing libcec would require a config snippet that contains: > -- > 2.17.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
On 01/01/2022 22:08, Yann E. MORIN wrote: > Matthew, All, > > On 2021-12-01 17:08 -0600, Matthew Weber via buildroot spake thusly: >> Add flag to allow the return value to be incremented when a skip >> happens, treating the overall run as a failure. >> >> This flag could be used when using test-pkg for local validation of >> downstream packages, for example in a user's br2-external. An example of >> this could be a user choosing to use test-pkg as part of a CI >> infrastructure where the fragment and toolchains are tightly controlled >> to test a package against multiple toolchains easily. >> >> In this case, the user would want to lower the rate of false-positives >> of that fragment/toolchain configuration if a build is instead skipped >> (which may happen in the case of buildroot version bumps where >> dependencies could change). > > I am a bit skeptical about this use-case. > > As I understand it, wht is most interesting in a CI setup for internal > development, is that *all* internal packages do build togetrher and form > a working sysem *as a whole*. > > As such, a CI setup would build the defconfig file(s) for a project (and > run the integration test-suite), rather than build each package > individually I don't think the intention is necessarily to build each package individually. Although it actually does make a whole lot of sense to do that, because it helps to check if the dependencies are set correctly. So, the obvious way to take the approach that Yann proposes is to have a list of defconfigs that enable all the packages in the external with the various toolchains. However, that still doesn't solve the issue that Matt faces, because any package can silently disappear from the config if its dependencies are no longer met. But indeed, it would make a *whole* lot of sense to add a check to %_defconfig that everything in the defconfig still appears in the generated .config file. If it doesn't, there is something really wrong... So yeah, I tend to agree that this is not the approach we should have upstream. Note that in CI it's very easy to check the output of test-pkg and search for SKIPPED lines. So it's not that this feature is really *needed* to land upstream. Therefore, I've marked the series as Rejected. As always, you're welcome to argue why we should include it after all. Regards, Arnout > > Building each package individually with test-pkg would have the > disadvantage that it takes more time overall, as all the common deps are > build as many times as there are packages to test, and package are also > dependencies one of the others, so most packages would be built more > than once as well... > > So, I am really not convinced... > > Regards, > Yann E. MORIN. > >> Signed-off-by: Matthew Weber <matthew.weber@collins.com> >> --- >> utils/test-pkg | 18 +++++++++++++++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/utils/test-pkg b/utils/test-pkg >> index d0472f176b..97957008fa 100755 >> --- a/utils/test-pkg >> +++ b/utils/test-pkg >> @@ -18,8 +18,8 @@ main() { >> local -a toolchains >> local pkg_br_name >> >> - o='hakc:d:n:p:r:t:e:' >> - O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:,externals:,' >> + o='hakc:d:n:p:r:t:e:f' >> + O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:,externals:,fail-on-skip' >> opts="$(getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}")" >> eval set -- "${opts}" >> >> @@ -29,6 +29,7 @@ main() { >> number=0 >> mode=0 >> prepare_only=0 >> + fail_on_skip=0 >> toolchains_csv="${TOOLCHAINS_CSV}" >> while [ ${#} -gt 0 ]; do >> case "${1}" in >> @@ -65,6 +66,9 @@ main() { >> (-e|--externals) >> BR2_EXTERNALS="${2}"; shift 2 >> ;; >> + (-f|--fail-on-skip) >> + fail_on_skip=1; shift 1 >> + ;; >> (--) >> shift; break >> ;; >> @@ -147,7 +151,12 @@ main() { >> printf "%d builds, %d skipped, %d build failed, %d legal-info failed\n" \ >> ${nb} ${nb_skip} ${nb_fail} ${nb_legal} >> >> - return $((nb_fail + nb_legal)) >> + nb_result=$((nb_fail + nb_legal)) >> + if [ ${fail_on_skip} ]; then >> + nb_result=$((nb_result + nb_skip)) >> + fi >> + >> + return $nb_result >> } >> >> build_one() { >> @@ -279,6 +288,9 @@ Options: >> Externals to be used as part of the build process. Packages from >> within these externals may be tested. >> >> + -f, --fail-on-skip >> + If any builds are skipped, return a negative exit value. >> + >> Example: >> >> Testing libcec would require a config snippet that contains: >> -- >> 2.17.1 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@buildroot.org >> https://lists.buildroot.org/mailman/listinfo/buildroot >
diff --git a/utils/test-pkg b/utils/test-pkg index d0472f176b..97957008fa 100755 --- a/utils/test-pkg +++ b/utils/test-pkg @@ -18,8 +18,8 @@ main() { local -a toolchains local pkg_br_name - o='hakc:d:n:p:r:t:e:' - O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:,externals:,' + o='hakc:d:n:p:r:t:e:f' + O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:,externals:,fail-on-skip' opts="$(getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}")" eval set -- "${opts}" @@ -29,6 +29,7 @@ main() { number=0 mode=0 prepare_only=0 + fail_on_skip=0 toolchains_csv="${TOOLCHAINS_CSV}" while [ ${#} -gt 0 ]; do case "${1}" in @@ -65,6 +66,9 @@ main() { (-e|--externals) BR2_EXTERNALS="${2}"; shift 2 ;; + (-f|--fail-on-skip) + fail_on_skip=1; shift 1 + ;; (--) shift; break ;; @@ -147,7 +151,12 @@ main() { printf "%d builds, %d skipped, %d build failed, %d legal-info failed\n" \ ${nb} ${nb_skip} ${nb_fail} ${nb_legal} - return $((nb_fail + nb_legal)) + nb_result=$((nb_fail + nb_legal)) + if [ ${fail_on_skip} ]; then + nb_result=$((nb_result + nb_skip)) + fi + + return $nb_result } build_one() { @@ -279,6 +288,9 @@ Options: Externals to be used as part of the build process. Packages from within these externals may be tested. + -f, --fail-on-skip + If any builds are skipped, return a negative exit value. + Example: Testing libcec would require a config snippet that contains:
Add flag to allow the return value to be incremented when a skip happens, treating the overall run as a failure. This flag could be used when using test-pkg for local validation of downstream packages, for example in a user's br2-external. An example of this could be a user choosing to use test-pkg as part of a CI infrastructure where the fragment and toolchains are tightly controlled to test a package against multiple toolchains easily. In this case, the user would want to lower the rate of false-positives of that fragment/toolchain configuration if a build is instead skipped (which may happen in the case of buildroot version bumps where dependencies could change). Signed-off-by: Matthew Weber <matthew.weber@collins.com> --- utils/test-pkg | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)