[PATCH] Handle alternative Kerberos credential cache locations

Tue Jan 28 17:55:12 UTC 2014

The technical part of the patch looks good (as previously, I've only checked the build system). 

The overshadowing issue is still legal, though. I can't see how this patch can be accepted without legal clearance. 

/Magnus

27 jan 2014 kl. 22:48 skrev Elliott Baron <ebaron at redhat.com>:

> Hi Magnus,
> 
> On 01/15/2014 06:08 AM, Magnus Ihse Bursie wrote:
>> On 2014-01-15 03:38, Wang Weijun wrote:
>>> Hi Elliott
>>> 
>>> Great to see this again. I’ll come back to this later. There are some urgent issues I have to deal with at this moment. I’ll also need to get those legal advices regarding pkg.m4 etc.
>> 
>> I see some issues and questions about this patch.
>> 
>> First of all, and I believe this was discussed the last time this patch was around, is that there might be legal question marks about including build-aux/krb5.m4. This code is written by someone who has not, to my knowledge, signed the OCA. Unfortunately, legal issues tend to shadow all technical issues, so you might want to start by getting this one solved.
>> 
>> On the technical level, given that the krb5.m4 legality is cleared, I see:
>> * The patch does not seem to be updated to the removed old build system. make/sun/security is no more.
>> * Whitespace and indentation seems to be incorrect in several places in help.m4, libraries.m4 and SecurityLibraries.gmk. Please check surrounding code, or look at the guidelines here: http://mail.openjdk.java.net/pipermail/build-dev/2013-October/010477.html
>> 
>> I have only looked at the build part of the patch.
> 
> How does this revised webrev look [1]? I have fixed indentation and removed remnants of the old build system. I have also fixed the test's Makefile, which no longer worked due to the removal of the old build system. I would like to point out that like the inheritedChannel test which this test is derived from, this test expects pre-built libraries to be checked into version control. I understand this is required due to test systems not necessarily having the required build environment.
> 
> Thanks,
> Elliott
> 
> [1] http://icedtea.classpath.org/~ebaron/webrevs/krb5-default-ccache/02/