RFR: JDK-8027698: Platform specific jars are not being signed by the sign-jars target

[Bradford Wetmore]

On 11/5/2013 1:31 AM, Erik Joelsson wrote:
>
> On 2013-11-04 20:57, Bradford Wetmore wrote:
>> Hi Erik,
>>
>> Thanks for the update!  Looks good, but one minor comment:
>>
>> SignJars.gmk
>> ============
>> 78:100:106:111-116 > 80 chars per line.
>>
> Thanks. We have not enforced 80 chars in any other file in the new build
> system. Has this been a habit before?

Depends on who you ask, and be prepared for the 80 chars Holy Wars[1]. 
According to the old Java style guide, 80 is the rule for Java files, so 
many of us tend to apply it throughout the JDK codebase.  Some 
developers are adamant about not following it. "Get a modern 
terminal..."  Just google "java 80 characters" and you'll see what I mean.

 From my point of view, as someone who routinely has multiple windows 
side by side for easy file change comparisons, I'd *MUCH* prefer having 
3 at 80 chars than 2 at 120 (or 1 at something really wide) (using a 8x12 font). 
  When working on laptops with smaller screens, even 2 at 80 gets tight 
(without squinting).  This also impacts things like webrev's sdiffs & 
frames: if you have two 160 line reviews, that's a wide window that you 
have to vertically scroll (twice for frames).  On a 24" screen at max 
resolution of 1680x1050, the 160 char side-by-side lines *REALLY* suck 
to review.

> I personally find it very
> impractical, especially with make adding a tab indent to each recipe
> line already, and because we tend to use rather long and descriptive
> variable names. I still try to keep lines reasonably short without
> forcing breaks that just make things less readable.

I will defer to the build team, this is primarily your code, but for my 
$0.02, I almost always keep all code <= 80 unless there's a really good 
reason.  If I need to touch long line code, I will generally shorten it, 
and I liberally use "\" (make) and "+" (java strings).  I won't play 
"Format Tennis" unless the code is just unreadable: fortunately I don't 
work in some of the other code within the JDK.

Brad

[1] Holy Wars = Differences of Opinion, with people ready to fight for 
their viewpoint.


> /Erik
>> Brad
>>
>>
>>
>>
>>
>> On 11/4/2013 2:19 AM, Erik Joelsson wrote:
>>> New webrev posted:
>>> http://cr.openjdk.java.net/~erikj/8027698/webrev.jdk.02/
>>>
>>> Addressed all comments below. I must have missed the save button before
>>> generating the webrev when adding ucrypto.jar, but it's there now at
>>> least.
>>>
>>> /Erik
>>>
>>> On 2013-11-02 01:35, Bradford Wetmore wrote:
>>>>
>>>>
>>>> On 11/1/2013 3:34 AM, Erik Joelsson wrote:
>>>>> Please review this simple fix, adding sunmscapi.jar and ucrypto.jar to
>>>>> the list of jars to be signed by the sign-jars target. Since these
>>>>> jars
>>>>> are platform dependent, they aren't always present.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8027698
>>>>> Webrev: http://cr.openjdk.java.net/~erikj/8027698/webrev.jdk.01/
>>>>>
>>>>> /Erik
>>>>
>>>> makefiles/SignJars.gmk
>>>> ======================
>>>>
>>>> Overall:  Some of the existing/new lines are greater than the standard
>>>> 80 chars.
>>>>
>>>> 2:  Some people update the Copyright date instead of waiting for RE to
>>>> do it.  Feel free if you're one of those.
>>>>
>>>> 48 & 108:  The RE process for signing jars for JDK 7/8 is completely
>>>> different than what was originally written/described for JDK 7.  There
>>>> is an additional separate script that RE uses to include the
>>>> source_tips into the jar files, and then do the actual signing.
>>>> SignJars.gmk is here primarily for developer support.
>>>>
>>>> 47:  Suggest changing to:
>>>>
>>>> # SPECIAL NOTE TO JCE/JDK developers: The source files must eventually
>>>> # be built, signed, and then the resulting jar files MUST BE CHECKED
>>>> # INTO THE CLOSED PART OF THE WORKSPACE*. This separate step *MUST NOT
>>>> # BE FORGOTTEN*, otherwise a bug fixed in the source code will not be
>>>> # reflected in the shipped binaries.
>>>> #
>>>> # Please consult with Release Engineering, which is responsible for
>>>> # creating the final JCE builds suitable for checkin.
>>>> #
>>>>
>>>> 94:  I don't see ucrypto.jar listed here.
>>>>
>>>> 108:  Can you please update the code here to output:
>>>>
>>>>     The jar files built by the 'sign-jars' target are developer
>>>>     builds only and *MUST NOT* be checked into the closed workspace.
>>>>
>>>>     Please consult with Release Engineering: they will generate
>>>>     the proper binaries for the closed workspace.
>>>>
>>>>     @$(PRINTF) $(README-MAKEFILE_WARNING)
>>>>
>>>> Since this bug is a subtask of JDK-8027266, I'll add a few more
>>>> comments to that bug.  I'm guessing you'll probably want to address
>>>> separately.
>>>>
>>>> Thanks, Erik!
>>>>
>>>> Brad
>>>
>