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

Incorrect MissingInherits if conditional use #352

Open thesamesam opened 2 years ago

thesamesam commented 2 years ago

I suspect this is because we're not just forcing OPERA_FORCE_RPM=no, but allowing it to be overridden in the environment, which would violate metadata invariance.

$ pkgcheck scan -k MissingInherits
www-client/opera-developer
  MissingInherits: version 80.0.4162.0: rpm: missing inherit usage: 'rpm_src_unpack "${A[0]}"', line 106
  MissingInherits: version 80.0.4170.0: unpacker: missing inherit usage: 'unpacker', line 108
  MissingInherits: version 81.0.4175.0: unpacker: missing inherit usage: 'unpacker', line 108

opera-developer-80.0.4162.0.ebuild:


# These are intended for ebuild maintainer use to force RPM if DEB is not available.
: ${OPERA_FORCE_RPM=no}

if [[ ${OPERA_FORCE_RPM} == yes ]]; then
        OPERA_UNPACKER="rpm"
        OPERA_ARCHIVE_EXT="rpm"
else
        OPERA_UNPACKER="unpacker"
        OPERA_ARCHIVE_EXT="deb"
fi

inherit chromium-2 multilib pax-utils ${OPERA_UNPACKER} xdg

...
src_install() {
        dodir /
        cd "${ED}" || die
        if [[ ${OPERA_FORCE_RPM} == yes ]]; then
                rpm_src_unpack "${A[0]}"
        else
                unpacker
        fi
radhermit commented 2 years ago

Personally, I'd suggest using multiple inherit lines instead of extra variables for stylistic reasons. In terms of the check, I forget what exactly affects the check although I do know it doesn't handle conditionals and especially doesn't work with metadata invariance.

thesamesam commented 2 years ago

Personally, I'd suggest using multiple inherit lines instead of extra variables for stylistic reasons. In terms of the check, I forget what exactly affects the check although I do know it doesn't handle conditionals and especially doesn't work with metadata invariance.

Yeah, I agree. Reported in Gentoo here.

thesamesam commented 1 month ago

Thinking about this a bit more after @eli-schwartz pointed out an issue with meson-9999 and:

if [[ ${PV} == 9999 ]] ; then
    inherit ninja-utils
else
   ...
fi

Couldn't we substitute things like ${PV} with their value before passing to treesitter for parsing? ts doesn't know they're constant in the ebuild environment. (I don't think this would help the original example though.)