RFC: Minor build correctness patch

[Lillian Angel]

Andrew John Hughes wrote:
> 2009/7/7 Lillian Angel <langel at redhat.com>:
>   
>> jon.vanalten at redhat.com wrote:
>>     
>>> Hi,
>>>
>>> The attached patch corrects a minor issue noted in
>>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=289
>>>
>>> Include statements for the .c files affected are missing, resulting in
>>> compiler warnings. Â While this wasn't affecting the build or correct
>>> performance, it is good practice to include headers where appropriate. Â This
>>> patch, adapted from that provided by the original bug reporter above, adds
>>> the include statements.
>>>
>>> Comments welcome.
>>>       
>> As per Andrew Hughes comments in the bug, I would like to see this filed
>> upstream to Sun. I have CC'ed Matthew Flaschen.
>>
>> For now, it is fine to commit.
>>
>>
>> Cheers,
>> Lillian
>>
>>     
>
> I'd much prefer we didn't commit this to IcedTea, as it is just
> another patch we have to manage and maintain.  The arguments Jon makes
> for including this (it doesn't affect performance or the build) can
> equally be used as very good reasons for not including it in IcedTea.
> The majority of the developers on this list, who have been hacking on
> IcedTea for a while, will be aware of how much of a pain it is to have
> to test the build with different patches applied and to have to
> recreate them when a new build drop appears upstream.  The current aim
> (at least of those at Red Hat) is to try and reduce this burden by
> getting the majority of patches upstream.
>
> Jon, as you're still new to the project, you're probably unaware of a
> lot of this pain so you'll have to take what we say on trust for now
> :)  Believe me, I think your efforts would be better spent applying
> this to the appropriate JDK7 tree and creating a webrev for it.  I
> can't see an appropriate tree, but presumably those on the net-dev
> mailing list can provide appropriate hints.
>
> Lillian: there already is a bug,
> http://bugs.sun.com/view_bug.do?bug_id=6562614 (referenced in our
> IcedTea bug) so it should be a simple matter of just getting the patch
> approved (the bug has been) and someone pushing it.
>
> BTW, I'm aware I said 'I'd commit this' on the bug, but take the fact
> that I didn't as a reconsideration on my part... ;)


Agreed. But there is a good chance if no one takes responsibility for 
this patch, it will never get committed. IcedTea is a place where we 
keep track of what needs to go upstream, and make sure we follow up on 
this. If Jon hadn't been doing a bug clean up, how long would it have 
been until we came across this again?

While the patch doesn't fix any build/performance issues, it is still a 
bug that should be fixed. Who should be responsible for watching it? 
Should we leave this up to the reporter?


Cheers,
Lillian