Bug 829579 – app-portage/portage-utils-0.92: qlist -U skips useflags (root cause identified and fix proposed)

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Michael Yagliyan 2021-12-18 19:03:30 UTC]

(I have a simple fix for this at the bottom.)

For example when I have a package (in my case app-accessibility/at-spi2-core-2.40.3) with:

use_argv = abi_x86_64 amd64 elibc_glibc introspection kernel_linux userland_GNU X

iuse_argv = abi_mips_n32 abi_mips_n64 abi_mips_o32 abi_s390_32 abi_s390_64 abi_x86_32 abi_x86_64 abi_x86_x32 gtk-doc introspection test X

That is the result of sorting using the cmpstringp comparator, which internally uses strcasecmp -- a case INSENSITIVE comparator.

The main outer loop in qlist.c's umapstr() function gets to a point where it is checking iuse_argv[12] (i.e. "X").  It starts an inner loop by comparing it against use_argv[6] (i.e. "userland_GNU").  However it uses strcmp, which is case-SENSITIVE, and in this case it returns 29, so instead of continuing with the inner loop to compare against the next use_argv entry the code breaks out of the inner loop, and since the outer loop condition is that i<iuse_argc it too terminates, all before we've detected that the IUSE "X" is also in USE, so qlist ends up (erroneously) not printing it.

The fix is simple, in cmpstringp() (which is only being used to compare USE flags, which are inherently case-sensitive) replace strcasecmp with strcmp.  They have the same interface so it's just a 1-word change (well, also remove the comment about it being case-insensitive).

[Comment 1 Michael Yagliyan 2021-12-18 19:04:31 UTC]

There should probably be a test case for this as well.

[Comment 2 Sam James 2021-12-18 19:09:36 UTC]

Thanks for the analysis. Could you provide a patch then?

[Comment 3 Michael Yagliyan 2021-12-18 19:26:33 UTC]

Created attachment 759613 [details, diff]
Patch on top of v0.92's qlist.c (419b78c)

[Comment 4 Michael Yagliyan 2021-12-18 19:27:09 UTC]

I submitted the patch.  I'm not familiar with how the test cases work for this package so I haven't added one.  I just fixed qlist.c.

[Comment 5 Fabian Groffen 2021-12-19 09:50:43 UTC]

to be precise, what happens here is that the use of strcasecmp and strcmp is mixed, which breaks the assumption of the merge-sort strategy:

- strcasecmp would order: [y, X]
- strcmp would order: [X, y] (because uppercase comes before lowercase)

now the merge sort establishes it no longer needs to search if it finds an item that is "greater than" what's left; in this case established by strcmp.
Because X is there deemed to be larger than y, the search is aborted.

Solution is to use strcasecmp or strcmp in both cases.  I don't know the reason for choosing strcasecmp here, and while it sounds kind of like being on the safe side, PMS documents USE-flags as "any of the characters [A-Za-z0-9+_@-]".  In other words, I agree that we best do the sorting case-sensitive (strcmp).  This seems to match with what portage does, except it seems to first list all enabled, then disabled flags, both groups sorted, whereas qlist -Uv shows all mixed but sorted.

[Comment 6 Larry the Git Cow 2021-12-19 09:54:34 UTC]

The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage-utils.git/commit/?id=74b75c453354a10af3c499aac15d3a2ae0e1c7ee

commit 74b75c453354a10af3c499aac15d3a2ae0e1c7ee
Author:     Fabian Groffen <[e-mail]>
AuthorDate: 2021-12-19 09:51:16 +0000
Commit:     Fabian Groffen <[e-mail]>
CommitDate: 2021-12-19 09:51:16 +0000

    qlist: fix matching of USE-flags
    
    Thanks to Michael Yagliyan for finding this bug and proposing the fix.
    
    We cannot perform merge-sort if the comparison in use is different
    between sorting and processing afterwards.  Use case-sensitive sort
    everywhere, for it is cheaper, matches Portage and is safer/more
    correct/inline with PMS.  This does change the default output ordering
    of the flags though.
    
    Added a test that ensures capital USE-flag is now matched correctly.
    
    Bug: https://bugs.gentoo.org/829579
    Signed-off-by: Fabian Groffen <[e-mail]>

 qlist.c                                    | 5 +++--
 tests/qlist/dotest                         | 3 +++
 tests/qlist/root/sys-fs/mtools-4.0.13/IUSE | 1 +
 tests/qlist/root/sys-fs/mtools-4.0.13/USE  | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

[Comment 7 Michael Yagliyan 2021-12-21 06:45:09 UTC]

Thank you for approving and incorporating the change, and for adding a test case!  How long does it typically take before a new version is released?

[Comment 8 Fabian Groffen 2021-12-21 08:31:55 UTC]

looks like it's time for a new release, bit overdue

[Comment 9 Larry the Git Cow 2021-12-21 10:25:37 UTC]

The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=7e74e1d6c4bf1db571db1cec13356ffbe10ba56f

commit 7e74e1d6c4bf1db571db1cec13356ffbe10ba56f
Author:     Fabian Groffen <[e-mail]>
AuthorDate: 2021-12-21 10:23:54 +0000
Commit:     Fabian Groffen <[e-mail]>
CommitDate: 2021-12-21 10:25:33 +0000

    app-portage/portage-utils-0.93: version bump
    
    Closes: https://bugs.gentoo.org/815622
    Closes: https://bugs.gentoo.org/816033
    Closes: https://bugs.gentoo.org/816060
    Closes: https://bugs.gentoo.org/829579
    Package-Manager: Portage-3.0.28, Repoman-3.0.3
    Signed-off-by: Fabian Groffen <[e-mail]>

 app-portage/portage-utils/Manifest                 |  1 +
 .../portage-utils/portage-utils-0.93.ebuild        | 67 ++++++++++++++++++++++
 2 files changed, 68 insertions(+)

[Comment 10 Joakim Tjernlund 2021-12-21 11:23:10 UTC]

(In reply to Fabian Groffen from comment #8)
> looks like it's time for a new release, bit overdue

Seem like you forgot to tag the release in git repo ?

[Comment 11 Fabian Groffen 2021-12-21 11:30:24 UTC]

sort of, I forgot push --tags :)