| Message ID | 20231211164553.12425-1-simone.p.weiss@posteo.com |
|---|---|
| State | Accepted, archived |
| Commit | 5c264063f6363e5ff88146125217b6089eb22f12 |
| Headers | show |
| Series | patchtest: Add test for deprecated CVE_CHECK_IGNORE | expand |
On 11 Dec 2023, at 16:45, simone.p.weiss via lists.openembedded.org <simone.p.weiss=posteo.com@lists.openembedded.org> wrote: > > From: Simone Weiß <simone.p.weiss@posteo.com> > > If a recipes was modified recommand the use of `CVE_STATUS` instead if > `CVE_CHECK_IGNORE` is used. This is a depreacted variable and will > result in a warning from the cve-check.class and should hence not be > used anymore. [YOCTO #15311] It feels like this shouldn’t be CVE_CHECK_IGNORE-specific but a general deprecated variable check. Can you rename so we can easily add more variables later? Relatedly, BB_RENAMED_VARIABLES is a map of variables which got deprecated but the solution is a simple rename, the same check should look for those too. Ross
On 2023-12-13 06:27, Ross Burton wrote: > On 11 Dec 2023, at 16:45, simone.p.weiss via lists.openembedded.org <simone.p.weiss=posteo.com@lists.openembedded.org> wrote: >> From: Simone Weiß <simone.p.weiss@posteo.com> >> >> If a recipes was modified recommand the use of `CVE_STATUS` instead if >> `CVE_CHECK_IGNORE` is used. This is a depreacted variable and will >> result in a warning from the cve-check.class and should hence not be >> used anymore. [YOCTO #15311] > It feels like this shouldn’t be CVE_CHECK_IGNORE-specific but a general deprecated variable check. Can you rename so we can easily add more variables later? > > Relatedly, BB_RENAMED_VARIABLES is a map of variables which got deprecated but the solution is a simple rename, the same check should look for those too. I agree, this would be a good change to make for long-term maintainability. LGTM otherwise. - Trevor > > Ross > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#192287): https://lists.openembedded.org/g/openembedded-core/message/192287 > Mute This Topic: https://lists.openembedded.org/mt/103112424/7611679 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [tgamblin@baylibre.com] > -=-=-=-=-=-=-=-=-=-=-=- >
> > I agree, this would be a good change to make for long-term > maintainability. LGTM otherwise. I have tried to implement such a check as well, and while this check is fine and works, the test of a patch with patchtest containing a variable from BB_RENAMED_VARIABLES that is not a pretest, i.e. the patch is already merged, the tinfoil setup and the subsequent parsing of the recipes while raise an issue for this test. e.g. ERROR: Variable SDK_LOCAL_CONF_BLACKLIST has been renamed to ESDK_LOCALCONF_REMOVE ... So on the one hand the issues is already cached by other parts of the buildsystem and on the other hand this will cause tests not related to renamed variables to fail. It was suggested on bugzilla by Trevor to merge this original test and open an issue in bugzilla to address the question of BB_RENAMED_VARIABLES separately. See [1] If you do agree, please tell me if a rebased version of the patch would be needed or not. - Cheers, Simone [1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=15311#c5
On Mon, 2023-12-18 at 12:42 -0800, simone.p.weiss@posteo.com wrote: > > I agree, this would be a good change to make for long-term > > maintainability. LGTM otherwise. > I have tried to implement such a check as well, and while this check > is fine > and works, the test of a patch with patchtest containing a variable > from > BB_RENAMED_VARIABLES that is not a pretest, i.e. the patch is already > merged, > the tinfoil setup and the subsequent parsing of the recipes while > raise an issue for this test. > > e.g. > > ERROR: Variable SDK_LOCAL_CONF_BLACKLIST has been renamed to > ESDK_LOCALCONF_REMOVE > ... > > So on the one hand the issues is already cached by other parts of the > buildsystem and on the other hand this will cause tests not related > to > renamed variables to fail. > > It was suggested on bugzilla by Trevor to merge this original test > and open > an issue in bugzilla to address the question of > BB_RENAMED_VARIABLES separately. See [1] > > If you do agree, please tell me if a rebased version of the patch > would be > needed or not. > > - Cheers, > Simone > > [1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=15311#c5 > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#192677): > https://lists.openembedded.org/g/openembedded-core/message/192677 > Mute This Topic: https://lists.openembedded.org/mt/103112424/8052774 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded- > core/unsub [simone.p.weiss@posteo.com] > -=-=-=-=-=-=-=-=-=-=-=- > Hi, any thoughts on this? Cheers, Simone
diff --git a/meta/lib/patchtest/tests/test_metadata.py b/meta/lib/patchtest/tests/test_metadata.py index b6f4456ad2..174dfc31c6 100644 --- a/meta/lib/patchtest/tests/test_metadata.py +++ b/meta/lib/patchtest/tests/test_metadata.py @@ -25,6 +25,8 @@ class TestMetadata(base.Metadata): sha256sum = 'sha256sum' git_regex = pyparsing.Regex('^git\:\/\/.*') metadata_summary = 'SUMMARY' + cve_check_ignore_var = 'CVE_CHECK_IGNORE' + cve_status_var = 'CVE_STATUS' def test_license_presence(self): if not self.added: @@ -178,3 +180,16 @@ class TestMetadata(base.Metadata): # "${PN} version ${PN}-${PR}" is the default, so fail if default if summary.startswith('%s version' % pn): self.fail('%s is missing in newly added recipe' % self.metadata_summary) + + def test_cve_check_ignore(self): + if not self.modified: + self.skip('No modified recipes, skipping test') + for pn in self.modified: + # we are not interested in images + if 'core-image' in pn: + continue + rd = self.tinfoil.parse_recipe(pn) + cve_check_ignore = rd.getVar(self.cve_check_ignore_var) + + if cve_check_ignore is not None: + self.fail('%s is deprecated and should be replaced by %s' % (self.cve_check_ignore_var, self.cve_status_var))