[OpenJDK 2D-Dev] RFR(S): JDK-8031134 : PPC64: implement printing on AIX

[Phil Race]

Hi,

On 1/17/14 10:56 AM, Volker Simonis wrote:
> Hi Phil,
>
> first of all, thanks a lot for your review. Please find my answers and 
> comments inline:
>
> On Thu, Jan 16, 2014 at 7:53 PM, Phil Race <philip.race at oracle.com 
> <mailto:philip.race at oracle.com>> wrote:
>
>     Hello Volker,
>
>     Interesting that all this is needed. How has AIX got by before ?
>
>
> Well, there hasn't been AIX support in OpenJDK until now so it 
> obviously didn't work:)

I meant the IBM JDK.
>
>     Is this taken from an existing IBM port or did you write this
>     yourself ?
>
>
> This is from our certified, long running (Java 4,5,6,7), commercial 
> SAP JVM (proudly written by ourselves).
Got it. Still if IBM have something else in this area they should speak up !
>
>     I'd hope you are getting help directly from IBM in this area.
>
>
> Sometimes:) We're doing the OpenJDK port together with IBM. (That is, 
> we're actually merging our existing, commercial ports in.)
>
>     I suppose that if CUPS is configured and running it'll take
>     precedence over these
>     as it does for the other cases. Someone with AIX should test that
>     at some point as
>     its the only way to know for sure that that works.
>
>
> There is no such things like CUPS or fontconfig on AIX. Of course 
> somebody may compile, configure and run it by himself. But that's 
> neither the default setting on AIX nor is it supported by IBM. I have 
> no such box and I don't know any customer that would use it.

Hmm google 'cups aix' suggests it does exist and get used but it may not 
be supported ..
>
> But believe me, our  customers are using our implementation since long 
> time - and it is working (even printing).
>
>     I'd hoped that AIX would be able to fit into either the BSD or
>     SysV printing mold
>     but it seems like its got its own special commands
>     - lsallq is a new one on me and looks like an AIX special
>
>
> Yes, it's AIX special (from the man page " The lsallq command lists 
> the names of all configured queues contained in the /etc/qconfig 
> file."). However, sometimes it can really save your life, especially 
> if you have costumers which have configured thousands of printers and 
> 'lpstat -p' takes hours to come back (we've seen that!).
>
>     - the "-W" arg to lpstat is completely different than what it
>     means on OS X or Linux !
>     and there's the oddity that it doesn't expect spaces between the
>     flag and the value ..
>     so I think your approach is the best one.
>
>     A few specific comments :-
>
>      } else if (aixPrinterEnumerator.equalsIgnoreCase("lsallq")) {
>      144                     aix_defaultPrinterEnumeration = aix_lsallq;
>      145                 }
>
>     this code seems redundant since its just reasserting the default
>     unless
>     you intend that this be something that can change multiple times but
>     I'd advise against that and anyway its in static block so that
>     won't happen ..
>
>
> We've documented both values for "sun.java2d.print.aix.lpstat" to be 
> able to change the default. So the user should be able to set both 
> values. And no, the value won't be changed anywhere else.

I wasn't saying they can't set both, just that the code to re-assert the 
hard coded default
isn't adding anything so I personally would not add the byte code but 
its up to you.
You can push as is if you like.
Your hard coded default is 20 or so lines further up : -

  private static int aix_defaultPrinterEnumeration = aix_lsallq;


>
>     I see you defined
>
>     167     static boolean isAIX( ) {
>      168         return osname.equals("AIX");
>      169     }
>      170
>
>     so why are you using this here :-
>      136         if (osname.equals("AIX")) {
>
>     instead of calling isAIX() as you do elsewhere ?
>
>
> Fixed.
>
>     ...
>
>     } else if (names.length != 1) {
>              // No default printer found
>
>     In the other cases we chose to nominate the 1st printer as the
>     default.
>     This seemed a better choice than making apps deal with a list of
>     installed printers but no default. Not sure what problems you might
>     encounter with this (if any). If the situation never occurs its
>     not an issue.
>
>
> You're right. This was probably a copy-and-past left over.
> I think there's no meaningful default printer name on AIX and if 
> 'lpstat -d' doesn't return anything useful then there's probably a 
> problem and we better return 'null'.
>
> Here's the new webrev:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8031134_2/ 
> <http://cr.openjdk.java.net/%7Esimonis/webrevs/8031134_2/>
>
> Is this now ready for pushing into ppc-aix-port/stage-9?

Yes

-phil.

>
> Thank you and best regards,
> Volker
>
>     -phil.
>
>
>     On 1/16/14 12:08 AM, Volker Simonis wrote:
>
>         Resending one more time to 2d-dev upon request:
>
>         Hi,
>
>         could somebody please review the following small change:
>
>         http://cr.openjdk.java.net/~simonis/webrevs/8031134/
>         <http://cr.openjdk.java.net/%7Esimonis/webrevs/8031134/>
>
>         It's the straight forward implementation of the basic printing
>         infrastructure on AIX and shouldn't have any impact on the
>         existing
>         platforms. As always, this change is intended for the
>         http://hg.openjdk.java.net/ppc-aix-port/stage/jdk repository.
>
>         Thank you and best regards,
>         Volker
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20140117/611ddec0/attachment-0001.html