pkgcheck
pkgcheck copied to clipboard
[New Check]: detect `declare` without `-g` in global scope
Is there an existing such new check request for this?
- [X] I have searched the existing issues
Explain
Any use of declare is local scope unless a -g is passed. When in global scope, to behave the same for portage and other PMs.
As first priority, we should catch all cases of declare -A, and then see if any other use exists. I should be careful, because some options to declare print things and they don't need -g. But those might not be used at all in global scope. In short, verify the difference between all declare and declare -A flagged to see for false positives.
IRC logs from #gentoo-dev
<@arthurzam> negril: please explain or give example.
<@arthurzam> You can also request checks using the template: https://github.com/pkgcore/pkgcheck/issues/new?assignees=&labels=check&projects=&template=new-check.yml&title=%5BNew+Check%5D%3A+
<+negril> https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-block/thin-provisioning-tools/thin-provisioning-tools-1.0.6.ebuild#n131 bash is odd in that declare -A is not in global scope. Hence when you source that ebuild in function that get's called in a function it is not in scope
<@arthurzam> negril: can be checked. So please file the issue template, and I can do it.
<+negril> I'm not sure if it's worth having an official check for that. Someone more familiar with bash/PMS would need to chime in
<+negril> If that even is desired behaviour
<@ulm> I think the gist is that in portage, global scope is bash global scope, but that may not be the case for other package managers
<@ulm> i.e. they may source the ebuild inside a shell function
<@arthurzam> ulm: you think this is a bad code we should flag? (this is easy check to impl)
<+negril> eg foo() { bar ) bar() { source pkg.ebuild }
<+negril> it's down to associative arrays being stupid in bash
<+negril> I do this little headstand to fix this https://github.com/negril/paludis/commit/eb1f87da456cf1212d63cbf34da2f014e9630861
<@ulm> in shell global scope, declare -A and declare -gA should do the same
<@ulm> but apparently there is a difference if in a function
<+negril> src_prepare(){ declare -A foo } is not global scope either
<@arthurzam> ulm: do I understand correctly: (function scope --> declare -A) - bad ; (global scope --> declare -A) - fine?
<@ionen> I did get some fun surprises when I used declare -A in linux-mod-r1 until I added in -g ;p not that it matters for e.g. GIT_CRATES which can be discarded
<+ztrawhcse> arthurzam: yes
<@ulm> the other way around?
<@arthurzam> (function scope --> declare -A) - find ; (global scope --> declare -A) - bad
<@ulm> declare -A is fine in ebuild function scope but bad in ebuild global scope
<@arthurzam> s/find/fine
<+ztrawhcse> I read it as "does what is expected"
<+ztrawhcse> probably wrong on my part, you want to know what to lint for
<@arthurzam> I mean "bad" is what I should catch and flag
<+negril> i'm trying to find my test code
<+ztrawhcse> if it's used in function scope it's going to behave the same in portage and elsewhere, so it's fine
<@arthurzam> ok, thanks
<@arthurzam> ulm: in case you missed prior message: https://dpaste.com/EGQM3UGTV
<+ztrawhcse> if it's used in global scope, it's bad because the behavior "depends", so the author probably expected it to behave like global scope and it will be buggy
<@ulm> arthurzam: I had seen it :)
<@ulm> lots of IUSE=""
<+ztrawhcse> arthurzam: (Alfr pointed out that I should probably mention this) but *any* use of declare, not just -A, has the same effect of being implicitly `local ...`
<+ztrawhcse> it's likely rarely used except for the actual effect of -A, because declare foo=bar at global scope is just, well, foo=bar
<@arthurzam> ztrawhcse: Oh, good point. So I should just check that each declare has `-g`
<@arthurzam> (in global scope)
<+negril> I think part of the issue was that associative arrays are two step, one if creating the array and one is the assignment
<+ztrawhcse> yes, but I suspect you will not find much of interest aside for -A
<@arthurzam> ztrawhcse: I'm sure someone managed to find a weird use for that - Gentoo devs are sometimes special :)
<+ztrawhcse> declare -l perhaps...
<+ztrawhcse> declare has "options" (which print data) and "options which set attributes"
<+ztrawhcse> the options that print data don't need -g and -g will be silently ignored
<@ionen> I use -n sometimes fwiw, but pretty rare I'd do this outside a function
<+ztrawhcse> but I very much *highly* doubt there's a valid use case for printing stuff at global scope
Examples
https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-block/thin-provisioning-tools/thin-provisioning-tools-1.0.6.ebuild#n131
Output message
line {lineno}: call to 'declare -A' without '-g' in global scope
Documentation
No response
Result level
error