pkgcore / pkgcheck

pkgcore-based QA utility for ebuild repos
https://pkgcore.github.io/pkgcheck
BSD 3-Clause "New" or "Revised" License
35 stars 29 forks source link

pkgcheck scan doesn't detect misuse of python_gen_cond_dep? #481

Open thesamesam opened 1 year ago

thesamesam commented 1 year ago

Steps to reproduce:

  1. git checkout 72a1fbf3592c8ceee6f797e36fe474c2d251ee47~1
  2. Run caja-dropbox-1.26.0.ebuild clean install
    $ ebuild caja-dropbox-1.26.0.ebuild clean install
    Appending /home/sam/git/gentoo to PORTDIR_OVERLAY...
    * caja-dropbox-1.26.0.tar.xz BLAKE2B SHA512 size ;-) ...                                                                                                                                                [ ok ]
    /home/sam/git/gentoo/mate-extra/caja-dropbox/caja-dropbox-1.26.0.ebuild: line 37: no match: dev-python/pygobject:3[%PYTHON_USEDEP-NEEDS-TO-BE-USED-IN-PYTHON_GEN_COND_DEP%]
    /home/sam/git/gentoo/mate-extra/caja-dropbox/caja-dropbox-1.26.0.ebuild: line 37: no match: dev-python/pygobject:3[%PYTHON_USEDEP-NEEDS-TO-BE-USED-IN-PYTHON_GEN_COND_DEP%]
    * Using python3.10 to build
    * Determining the location of the kernel source code
    * Found kernel source directory:
    [...]

The bad *DEPEND snippet:

[...]
COMMON_DEPEND="${PYTHON_DEPS}
        dev-libs/atk
        >=dev-libs/glib-2.50:2
        $(python_gen_cond_dep \
                dev-python/pygobject:3[${PYTHON_USEDEP}]
        )
        >=mate-base/caja-1.19.1
        mate-extra/caja-extensions
        media-libs/fontconfig:1.0
        media-libs/freetype:2
        x11-libs/cairo
        x11-libs/gdk-pixbuf:2
        >=x11-libs/gtk+-3.22:3
        x11-libs/libXinerama
        x11-libs/pango
"
[...]

I feel like something should be dying fatally here? Apparently egencache shows it too:

* Updating metadata cache for gentoo ...
/srv/repos/gentoo/mate-extra/caja-dropbox/caja-dropbox-1.26.0.ebuild: line 37: no match: dev-python/pygobject:3[%PYTHON_USEDEP-NEEDS-TO-BE-USED-IN-PYTHON_GEN_COND_DEP%]

pkgcheck scan isn't bothered by it.

mgorny commented 1 year ago

I suppose the problem is due to the combination of failglob and the subshell.

Basically, we catch two problems:

  1. Use of unescaped [] in global scope because it causes failglob to exit while sourcing ebuild.
  2. Use of ${PYTHON_USEDEP} directly because it causes junk to land in the variable.

The problem is that unescaped [] inside the subshell causes the subshell to exit immediately with the error, and $() evaluates to an empty string. However, we never catch that error (I'm not even sure if it's possible) and empty string is valid in this context, so the user never sees a problem (except for noise in output).