| Summary: | pseudo applies PSEUDO_IGNORE_PATHS variable too greedy | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Yocto Project Subprojects] Pseudo | Reporter: | Krayushkin Konstantin <ip.jango> | ||||||
| Component: | poky integration | Assignee: | Dorinda <dorindabassey> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | Medium | CC: | dorindabassey, randy.macleod, richard.purdie, ross.burton, yp.pseudo.watcher, yp.watcher | ||||||
| Version: | 3.2.1 | ||||||||
| Target Milestone: | 3.3 | ||||||||
| Hardware: | x86 | ||||||||
| OS: | Multiple | ||||||||
| Whiteboard: | |||||||||
| OS type for building Yocto: | --- | Type of Regression: | --- | ||||||
| Verified: | Documentation change: | Yes (doc changes required) | |||||||
| Attachments: |
|
||||||||
|
Description
Krayushkin Konstantin
2021-01-20 18:11:03 UTC
Dorinda, Is this something you'd like to work on.
We need a sanity check to see if ${S} and the paths in pseudo ignore paths overlap and error if they do overlap.
oh cool, i'd love to work on this, let me think of a solution thank you. Created attachment 4764 [details]
pseudo patch for pseudo_client_ignore_path_chroot
Found a bug in my own patch. Attached *PATCH v2* to this task
(In reply to comment #1) > Dorinda, Is this something you'd like to work on. > We need a sanity check to see if ${S} and the paths in pseudo ignore paths > overlap and error if they do overlap. Hm, that was an issue after all, that should be fixed, not hidden behind some Yocto heuristic mechanisms. I mean such sanity checks are obviously useful, but only for protection; the issue will still be presented in the code despite those protections. The issue is that pseudo code works unstable while processing PSEUDO_IGNORE_PATHS environment variable. The best way to implement PSEUDO_IGNORE_PATHS application in my opinion would be by using globs, or at most regular expressions. Richard's Purdie decision to just compare strings is arguable, but he is an architect after all, so i just followed his ideas in my patch. But once again, the code works unstable - what if similar bug will appears during some other variable matching, not ${S}? How many variables are there in every package build? And all of them are potentially buggy. Please consider my approach once again. Maybe we should call Richard Purdie for his opinion? I was fixing exactly his code after all. Hi, you may have a point there i guess but am not very familiar pseudo. Nonetheless i submitted a patch to oe-core mailing list here @ https://lists.openembedded.org/g/openembedded-core/message/147373 although it's still a WIP. (In reply to comment #5) > Hi, you may have a point there i guess but am not very familiar pseudo. > Nonetheless i submitted a patch to oe-core mailing list here @ > https://lists.openembedded.org/g/openembedded-core/message/147373 although > it's still a WIP. Copying ny reply on mailing list. The point was that ${S}, that is ignored (and should be ignored), is part of some other path, that is NOT in PSEUDO_IGNORE_PATHS and should not be ignored. Example: /some/long/path/and/finally/s_directory -- this is ${S}, that should be ignored; also it is part of PSEUDO_IGNORE_PATHS... and that's why it is actually ignored /some/long/path/and/finally/s_directory_with_some_suffix -- this is some another directory, that should not be ignored, but because it is prefixed by ${S}, it incorrectly is ignored My actual case: S == /<build-root>/build/tmp/work/<SoC-tunes>-poky-linux-gnieabi/lpack/<version>/pack -- it is in PSEUDO_IGNORE_PATHS, and it should be ignored; pay attention to the last path segment, pack /<build-root>/build/tmp/work/<SoC-tunes>-poky-linux-gnieabi/lpack/<version>/package -- this should not have be ignored, but incorrectly ${S} was applied to it, because of the algorithm in pseudo So, what you should do in sanity check is actually check overlaps of ${S} with the list of important directories, that should not be ignored, not with PSEUDO_IGNORE_PATHS. Where to get this list of important directories - that's a question... The only thing i can imagine right now - just hardcode it like shown below. Also, consider that ${S} is only one case of overlapping. In mailing list https://lists.openembedded.org/g/openembedded-core/topic/patch_sanity_bbclass/80117826 i saw Chen Qi suggested the opposite strategy for pseudo path filtering. If this new strategy would be applied, this task 14193 may be closed. With PSEUDO_CONSIDER_PATHS the problem in this task would be solved anyways - neither mine, nor Dorinda's patch would be necessary. Rightly or wrongly (and it can be argued either way), PSEUDO_IGNORE_PATHS does simple prefix matching. Whilst we could force that to be more exact path matching, we could lose flexibility and the configuration here wouldn't work since we do use that matching capability to minimise the list of paths needed in PSEUDO_IGNORE_PATHS. I've replied to Dorina's patch on the mailing list detailing the kind of sanity test we need to ensure people at least don't hit problems like reported here and get a better error message explaining the problem. Whether or not we move to the path inclusion method rather than exclusion remains to be seen, it was advised explicitly against by the pseudo authors. Patches for pseudo and discussion can be sent to the mailing list as detailed in the README but its unlikely we'd change the matching mechanism at this point as detailed above. The sanity test is however important to stop people being caught out by this. Hi All, Patch submitted at https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=785b5489aba6aae6919abdd3faf54abf2439bec6 The sanity test is merged in 6e4bd8cabcdedf4b52345ef5eb421f71d0f19b1d. Sanity test has been added, not sure we can change the rest of the implementation without causing efficiency issues or breakage with the way it is being used in OE-Core. Path inclusion rather than exclusion may be the other route to explore too, many people are requesting that. |