Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ebegin/eend: accept properly nested calls in different functions #824

Conversation

laumann
Copy link
Contributor

@laumann laumann commented Apr 21, 2022

Turn the EBEGIN_EEND variable into a stack that tracks which function
called ebegin. Calls to ebegin may be stacked and do not generate a QA
warning if the callers are different functions. Calls to eend then check
that the function name at the top of the stack matches the caller's
function name.

Bug: https://bugs.gentoo.org/839585
Bug: https://bugs.gentoo.org/839588
Signed-off-by: Thomas Bracht Laumann Jespersen t@laumann.xyz

Turn the EBEGIN_EEND variable into a stack that tracks which function
called ebegin. Calls to ebegin may be stacked and do not generate a QA
warning if the callers are different functions. Calls to eend then check
that the function name at the top of the stack matches the caller's
function name.

Bug: https://bugs.gentoo.org/839585
Bug: https://bugs.gentoo.org/839588
Signed-off-by: Thomas Bracht Laumann Jespersen <t@laumann.xyz>
@laumann
Copy link
Contributor Author

laumann commented Apr 21, 2022

Small demo:

EAPI=8

DESCRIPTION="Testing package"
HOMEPAGE="http://nowhere.com"

LICENSE="GPL-2"
SLOT="0"

somefunc() {
	ebegin "i'm somefunc"
	#eend 0
}

pkg_pretend() {
	ebegin "well hello there"
	somefunc
	eend 0
}

and we get:

 * well hello there ...
 * i'm somefunc ...
 * QA Notice: eend (in pkg_pretend) improperly matched with ebegin (called in somefunc)
 [ ok ]
 * QA Notice: ebegin called in pkg_pretend but missing call to eend

Uncommenting the eend 0 line, produces no warnings (but the output does get messed up a bit):

Appending /home/tj/sources/gentoo/portage-tester to PORTDIR_OVERLAY...
 * well hello there ...
 * i'm somefunc ...
 [ ok ]
 [ ok ]

# Already a call to ebegin
local prev="${EBEGIN_EEND[-1]}"
if [[ "${prev}" == "${FUNCNAME[1]}" ]] ; then
eqawarn "QA Notice: ebegin called in ${prev}, but missing call to eend (${FUNCNAME[1]})"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err, maybe not the best message, as ${prev} and ${FUNCNAME[1]} will be the same string :-)

How about:

QA Notice: ebegin already called in ${prev}, but missing call to eend

?

eqawarn "QA Notice: ebegin called, but missing call to eend (phase: ${EBUILD_PHASE})"
# Already a call to ebegin
local prev="${EBEGIN_EEND[-1]}"
if [[ "${prev}" == "${FUNCNAME[1]}" ]] ; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was pointed out to me that properly recursive functions with properly nested ebegin-eend will still generate a QA notice here.

@floppym
Copy link
Contributor

floppym commented Apr 21, 2022

Maybe it would be better to ban nested ebegin calls and adjust the affected eclasses to not do that.

@laumann
Copy link
Contributor Author

laumann commented Apr 21, 2022

Maybe it would be better to ban nested ebegin calls and adjust the affected eclasses to not do that.

The more I've thought about it, the more I would agree.

floppym pushed a commit to floppym/portage that referenced this pull request Jul 11, 2022
Turn the EBEGIN_EEND variable into a stack that tracks which function
called ebegin. Calls to ebegin may be stacked and do not generate a QA
warning if the callers are different functions. Calls to eend then check
that the function name at the top of the stack matches the caller's
function name.

Bug: https://bugs.gentoo.org/839585
Bug: https://bugs.gentoo.org/839588
Signed-off-by: Thomas Bracht Laumann Jespersen <t@laumann.xyz>
Closes: gentoo#824
Signed-off-by: Sam James <sam@gentoo.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants