Bugzilla Bug 190871

Blame jakub ;)

"nicely in emerge" just means failing silently for no @eclass thingy.

Reproducible: Always

Created an attachment (id=129681) [details]
eclass-to-manpage.awk.patch

This is for eclass-to-manpage.awk in files.

the variables are mixed in on purpose ... the idea is that variables usually
arent global eclass things, but they're tied to a specific function

a way to differentiate should be put together

(In reply to comment #2)
> the variables are mixed in on purpose ... the idea is that variables usually
> arent global eclass things, but they're tied to a specific function
> 
> a way to differentiate should be put together

Yeah there's no way to bind certain vars to specific functions as it is, you
get a list of variables under FUNCTIONS section, then a list of functions.
Confusing and looks much better with this patch, for php stuff at least ;) Lots
of variables are global and used across many functions in PHP eclasses, not
really tied to a specific one.

(In reply to comment #2)
> the variables are mixed in on purpose ... the idea is that variables usually
> arent global eclass things, but they're tied to a specific function
>
Er OK, only it seemed to be (environment-type) vars which are set by the
calling ebuilds. As such the sample I saw had all the vars at the top.

> a way to differentiate should be put together
> 
Yeah, that's why I didn't just swap var and function handling around (which
would have sorted the php eclass i looked at.)

I guess we could do "global" vars before any function, and then do other vars
in function scope. But we'd have to clean the syntax up wrt functions at least:
it's just not needed when we can look for `function()[[:space:]]*{' and losing
the need for it would make it much easier to document existing eclasses.

i'm not interested in merging a patch which makes one thing look nice while
breaking another

use @ECLASS-VARIABLES for global scope vars

(In reply to comment #5)
Your fix completely broke the whole thing that was previously working on ported
eclasses (unknown content between @DESCRIPTION and variable), also producing
zillions of 'Unexpected tag in "header" state' errors while parsing eclasses
that are not ported at all and should be just skipped.

Reopen.

The eclasses are here ATM, if you need to test:

http://overlays.gentoo.org/proj/php/browser/portage/eclass

the warnings are appropriate and will remain

added support for ignoring default value extraction

Nice, so...

<snip>
/usr/portage/eclass/php-ext-pecl-r1.eclass:27: PHP_EXT_PECL_PKG: unable to
extract default variable content: 
/usr/portage/eclass/php-ext-pecl-r1.eclass:36: PHP_EXT_PECL_FILENAME: unable to
extract default variable content: 
/usr/portage/eclass/php-ext-pecl-r1.eclass:40: DOCS: unable to extract default
variable content: 
</snip>

What's this trying to say? Sorry but I don't grok it. The eclasses are in CVS
now, could you tell me what's it moaning about?

----

</snip>
/usr/portage/eclass/java-ant-2.eclass:26: Unexpected tag in "header" state: #
@variable-preinherit WANT_ANT_TASKS
/usr/portage/eclass/java-ant-2.eclass:27: Unexpected tag in "header" state: #
@variable-default ""
/usr/portage/eclass/java-ant-2.eclass:33: Unexpected tag in "header" state: #
@variable-preinherit WANT_SPLIT_ANT
/usr/portage/eclass/java-ant-2.eclass:34: Unexpected tag in "header" state: #
@variable-default ""
/usr/portage/eclass/java-ant-2.eclass:43: Unexpected tag in "header" state: #
@variable-preinherit JAVA_ANT_DISABLE_ANT_CORE_DEP
/usr/portage/eclass/java-ant-2.eclass:44: Unexpected tag in "header" state: #
@variable-default unset for java-pkg-2, true for java-pkg-opt-2
/usr/portage/eclass/java-ant-2.eclass:83: Unexpected tag in "header" state: #
@global JAVA_PKG_BSFIX
/usr/portage/eclass/java-ant-2.eclass:93: Unexpected tag in "header" state: #
@global JAVA_PKG_BSFIX_ALL
/usr/portage/eclass/java-ant-2.eclass:102: Unexpected tag in "header" state: #
@global JAVA_PKG_BSFIX_NAME
/usr/portage/eclass/java-ant-2.eclass:111: Unexpected tag in "header" state: #
@global JAVA_PKG_BSFIX_TARGETS_TAGS
/usr/portage/eclass/java-ant-2.eclass:120: Unexpected tag in "header" state: #
@global JAVA_PKG_BSFIX_SOURCE_TAGS
/usr/portage/eclass/java-ant-2.eclass:129: Unexpected tag in "header" state: #
@global JAVA_ANT_IGNORE_SYSTEM_CLASSES
/usr/portage/eclass/java-ant-2.eclass:137: Unexpected tag in "header" state: #
@private java-ant_bsfix

...
</snip>

$ grep @ECLASS /usr/portage/eclass/java-ant-2.eclass
$

Why's it even trying to parse this, flooding people with bogus warnings?

Ditto: java-utils-2.eclass, java-vm-2.eclass, webapp.eclass

those are warnings, not errors

Yeah, warnings about what? What's wrong with the PHP eclasses?

And why is this thing flooding people with gazillions of warnings about stuff
that's it shouldn't parse at all in the first place?

$ grep Unexpected emerge.log | wc -l
291

Sorry but I don't think this is a sane way to handle things.

warnings are not bugs and the issue this bug is about is fixed

Good that you failed to address any of the questions actually asked above,
thanks very much. 

So, once again - what's wrong with the PHP eclasses? And why on earth are you
flooding people with hundreds lines of bogus output? And no, the issue is not
fixed, all these loads of false positives are direct result of your fix.

the only thing that needs to be addressed is the original and it has been

Fine, Bug 190981 now.

Created an attachment (id=129795) [details]
man depend.php.eclass output

So, this is what the script produced from depend.php.eclass; the PHPCHECKNODIE
variable is mangled with function name, the @CODE tag gets misparsed as well,
resulting in complete mess, apparently as a result of trying to extract
'default' value (which is just bad idea, as already noted above).

If you want to tell me that it's eclass fault again, then:

1/ It worked perfectly fine before your 'fix'
2/ It works perfectly fine with the patch attached to this bug.

See above.

Created an attachment (id=129797) [details]
depend.php.eclass.5

if you actually read the awk code you'd see that the variable changes had
nothing to do with that

fixed in cvs

(In reply to comment #5)
> i'm not interested in merging a patch which makes one thing look nice while
> breaking another
>
Fair enough; is this stuff documented anywhere?

> use @ECLASS-VARIABLES for global scope vars
> 
Ah i see, didn't know about that. Although I didn't see any reference to such a
thing in the awk, only header, function and variable 'states'. Personally I'm
much more interested in vars that are set by the calling ebuild than vars used
within eclass functions.

I have to say I found the layout of the php blah eclass quite intuitive,
although as I said I think the syntax is a bit facile wrt functions at least.
I'm getting my thoughts in here as this is the public discussion and the two of
you have commented loads already, as my inbox will testify ;) but really I
think it would be more efficient if we could discuss this on IRC (if you don't
mind my involvement, vapier?)

WRT to the argument you guys are having, I have to say firstly it's quite funny
to see Jakub asking for more clarification, in the same way I have seen users
ask him for the same, and secondly that I have to agree with him. The output I
got from emerge eclass-manpages was totally useless, which was why I got it to
fail silently if there was no @ECLASS.

A reasonable compromise to my mind is to include all the warnings only if a
flag is set. In the general user case, debug warnings are not helpful. A dev
testing an eclass for this new stuff can add "-v debug=1" to the command line
or just use an alias when using the awk script. (Assuming you don't have a
simpler way to tell when it's being run within an ebuild.)

At the end of the day, it's down to the bugmaster to call stuff on user
interaction IMO. I'm aware my opinions are not very mainstream within this
community, however. ;-)