[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: Let jvmtiGen exit with a non-zero exit code upon failure
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2015-10-30 22:16:30
Message-ID: 5633EC3E.5020701 () oracle ! com
[Download RAW message or body]

Agreed.
This makes sense.

Carsten,

Does this approach looks better to you?
If so, could you, please, change your webrev accordingly?
I do not want to push a different fix under your name.

Thanks,
Serguei


On 10/30/15 04:33, Staffan Larsen wrote:
> I realize now that the code can be simplified even more. If we remove 
> all the exception-catches and just let any exceptions propagate out of 
> main(), then the java executable will terminate with a non-zero exit 
> value if an exception is thrown. I don't think the code in the 
> exception handlers add any significant functionality. If you do this, 
> then you don't need to keep track of the "cleanRun" variable.
>
> /Staffan
>
>> On 30 okt. 2015, at 10:12, serguei.spitsyn@oracle.com 
>> <mailto:serguei.spitsyn@oracle.com> wrote:
>>
>> Carsten,
>>
>> The fix looks good.
>> I'll push it as soon as I get a time.
>>
>> New bug filed:
>> https://bugs.openjdk.java.net/browse/JDK-8141035
>>
>> Feel free to update it if necessary.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 10/29/15 17:00, Carsten Varming wrote:
>>> Dear Serguei and Steffan,
>>>
>>> Thank you for the reviews. I have updated the webrev at 
>>> http://cr.openjdk.java.net/~cvarming/jvmtiGen . I haven't been able 
>>> to log into JBS yet, so please go ahead and create bug id for this 
>>> change.
>>>
>>> Dan: Thank you for forwarding this email to the proper alias.
>>>
>>> Carsten
>>>
>>> On Thu, Oct 29, 2015 at 6:44 PM, serguei.spitsyn@oracle.com 
>>> <serguei.spitsyn@oracle.com <mailto:serguei.spitsyn@oracle.com>> wrote:
>>>
>>>     Carsten,
>>>
>>>     I forgot to thank you for taking care about this issue!
>>>
>>>     Thanks,
>>>     Serguei
>>>
>>>
>>>     On 10/29/15 15:41, serguei.spitsyn@oracle.com
>>>     <mailto:serguei.spitsyn@oracle.com> wrote:
>>>
>>>         Hi Carsten,
>>>
>>>         The fix looks good.
>>>         I share the Staffan's comments though.
>>>         If understand correctly, you do not have an openjdk author
>>>         status yet.
>>>
>>>         I will sponsor your fix.
>>>         Please, let me know the bug ID after you create one.
>>>
>>>
>>>         Thanks,
>>>         Serguei
>>>
>>>
>>>         Please, let me know
>>>
>>>         On 10/29/15 14:22, Staffan Larsen wrote:
>>>
>>>             Carsten,
>>>
>>>             This looks good with a few comments:
>>>
>>>             1) If you make the "verbose" variable into a static
>>>             field, you can avoid the final-copying.
>>>             2) nit: Line 216: put "System.exit(1);" on it's own line
>>>
>>>             Oh, and create a bug: https://bugs.openjdk.java.net
>>>             <https://bugs.openjdk.java.net/>
>>>
>>>             Thanks,
>>>             /Staffan
>>>
>>>                 On 29 okt. 2015, at 14:54, Daniel D. Daugherty
>>>                 <daniel.daugherty@oracle.com
>>>                 <mailto:daniel.daugherty@oracle.com>> wrote:
>>>
>>>                 JVM/TI belongs to the Serviceability team so adding
>>>                 serviceability-dev@...
>>>
>>>                 Dan
>>>
>>>
>>>
>>>                 On 10/28/15 8:45 PM, Carsten Varming wrote:
>>>
>>>                     webrev:
>>>                     http://cr.openjdk.java.net/~cvarming/jvmtiGen/
>>>                     <http://cr.openjdk.java.net/%7Ecvarming/jvmtiGen/>
>>>                     bug: ?
>>>
>>>                     jvmtiGen is used to process a number of xml and
>>>                     xslt files in OpenJDK.
>>>                     Currently jvmtiGen exits with exit code 0
>>>                     regardless of its success. This
>>>                     causes make to often consider a target finished
>>>                     when in fact the target
>>>                     failed. It also leads to funny error checking
>>>                     after the execution of
>>>                     jvmtiGen. For instance, in many trace.make
>>>                     files[*] a test for the
>>>                     existence of the output file is carried out
>>>                     after the completion of
>>>                     jvmtiGen. In a clean working repository that
>>>                     test is equivalent to jvmtiGen
>>>                     exiting with a proper exit failure code on
>>>                     failure, but in a dirty working
>>>                     repository the target file might just be
>>>                     pre-existing. This causes
>>>                     unnecessary pain when working with files
>>>                     processed by jvmtiGen.
>>>
>>>                     In this change I chose to exit with exit code 1
>>>                     whenever a failure is
>>>                     detected, be it a dtd validation failure, an IO
>>>                     failure, or something else
>>>                     entirely. This halts the building of OpenJDK on
>>>                     failures and ultimately
>>>                     makes development easier. I also added a verbose
>>>                     option such that warnings
>>>                     from the xml parser and dtd checker can be
>>>                     printed on stderr if desired.
>>>                     Finally, I changed all the error message
>>>                     printing to stderr. :-)
>>>
>>>                     Let me know what you think.
>>>
>>>                     BTW. This is the first time I tried the webrev
>>>                     system, so hopefully it all
>>>                     looks good. I havn't figured out how to create a
>>>                     bug yet, whence the
>>>                     question mark.
>>>
>>>                     I wasn't sure if hotspot-runtime-dev is the
>>>                     right email alias. Please let
>>>                     me know if there is a more appropriate alias for
>>>                     this email.
>>>
>>>                     [*] Why are so many of the non-shared makefiles
>>>                     almost identical?
>>>
>>>
>>>
>>>
>>
>


[Attachment #3 (text/html)]

<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Agreed.<br>
      This makes sense.<br>
      <br>
      Carsten,<br>
      <br>
      Does this approach looks better to you?<br>
      If so, could you, please, change your webrev accordingly?<br>
      I do not want to push a different fix under your name.<br>
      <br>
      Thanks,<br>
      Serguei<br>
      <br>
      <br>
      On 10/30/15 04:33, Staffan Larsen wrote:<br>
    </div>
    <blockquote
      cite="mid:4C0FA96C-1122-4236-84B2-B4CBAB2F795E@oracle.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
      I realize now that the code can be simplified even more. If we
      remove all the exception-catches and just let any exceptions
      propagate out of main(), then the java executable will terminate
      with a non-zero exit value if an exception is thrown. I don't
      think the code in the exception handlers add any significant
      functionality. If you do this, then you don't need to keep track
      of the "cleanRun" variable.
      <div class=""><br class="">
      </div>
      <div class="">/Staffan<br class="">
        <div class=""><br class="">
          <div>
            <blockquote type="cite" class="">
              <div class="">On 30 okt. 2015, at 10:12, <a
                  moz-do-not-send="true"
                  href="mailto:serguei.spitsyn@oracle.com" class=""><a \
class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a></a>  \
wrote:</div>  <br class="Apple-interchange-newline">
              <div class="">
                <meta content="text/html; charset=utf-8"
                  http-equiv="Content-Type" class="">
                <div bgcolor="#FFFFFF" text="#000000" class="">
                  <div class="moz-cite-prefix">Carsten,<br class="">
                    <br class="">
                    The fix looks good.<br class="">
                    I'll push it as soon as I get a time.<br class="">
                    <br class="">
                    New bug filed:<br class="">
                       <a moz-do-not-send="true"
                      class="moz-txt-link-freetext"
                      \
href="https://bugs.openjdk.java.net/browse/JDK-8141035">https://bugs.openjdk.java.net/browse/JDK-8141035</a><br
  class="">
                    <br class="">
                    Feel free to update it if necessary.<br class="">
                    <br class="">
                    Thanks,<br class="">
                    Serguei<br class="">
                    <br class="">
                    <br class="">
                    On 10/29/15 17:00, Carsten Varming wrote:<br
                      class="">
                  </div>
                  <blockquote
cite="mid:CAP_pwnXOjDLQxdxj03jyACrFcCFae7DhhOtYcKCL3QmGcYV=ww@mail.gmail.com"
                    type="cite" class="">
                    <div dir="ltr" class="">Dear Serguei and Steffan,
                      <div class=""><br class="">
                      </div>
                      <div class="">Thank you for the reviews. I have
                        updated the webrev at  <a moz-do-not-send="true"
                          class="moz-txt-link-freetext"
                          \
href="http://cr.openjdk.java.net/%7Ecvarming/jvmtiGen">http://cr.openjdk.java.net/~cvarming/jvmtiGen</a>
                
                        . I haven't been able to log into JBS yet, so
                        please go ahead and create bug id for this
                        change.</div>
                      <div class=""><br class="">
                      </div>
                      <div class="">Dan: Thank you for forwarding this
                        email to the proper alias.</div>
                      <div class=""><br class="">
                      </div>
                      <div class="">Carsten</div>
                    </div>
                    <div class="gmail_extra"><br class="">
                      <div class="gmail_quote">On Thu, Oct 29, 2015 at
                        6:44 PM, <a moz-do-not-send="true"
                          class="moz-txt-link-abbreviated"
                          \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>  <span \
dir="ltr" class="">&lt;<a  moz-do-not-send="true"
                            href="mailto:serguei.spitsyn@oracle.com"
                            target="_blank" class=""><a \
class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a></a>&gt;</span>
  wrote:<br class="">
                        <blockquote class="gmail_quote" style="margin:0
                          0 0 .8ex;border-left:1px #ccc
                          solid;padding-left:1ex">Carsten,<br class="">
                          <br class="">
                          I forgot to thank you for taking care about
                          this issue!<br class="">
                          <br class="">
                          Thanks,<br class="">
                          Serguei
                          <div class="HOEnZb">
                            <div class="h5"><br class="">
                              <br class="">
                              On 10/29/15 15:41, <a
                                moz-do-not-send="true"
                                href="mailto:serguei.spitsyn@oracle.com"
                                target="_blank" class=""><a \
class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a></a>  \
wrote:<br class="">  <blockquote class="gmail_quote"
                                style="margin:0 0 0 .8ex;border-left:1px
                                #ccc solid;padding-left:1ex"> Hi
                                Carsten,<br class="">
                                <br class="">
                                The fix looks good.<br class="">
                                I share the Staffan's comments though.<br
                                  class="">
                                If understand correctly, you do not have
                                an openjdk author status yet.<br
                                  class="">
                                <br class="">
                                I will sponsor your fix.<br class="">
                                Please, let me know the bug ID after you
                                create one.<br class="">
                                <br class="">
                                <br class="">
                                Thanks,<br class="">
                                Serguei<br class="">
                                <br class="">
                                <br class="">
                                Please, let me know<br class="">
                                <br class="">
                                On 10/29/15 14:22, Staffan Larsen wrote:<br
                                  class="">
                                <blockquote class="gmail_quote"
                                  style="margin:0 0 0
                                  .8ex;border-left:1px #ccc
                                  solid;padding-left:1ex"> Carsten,<br
                                    class="">
                                  <br class="">
                                  This looks good with a few comments:<br
                                    class="">
                                  <br class="">
                                  1) If you make the "verbose" variable
                                  into a static field, you can avoid the
                                  final-copying.<br class="">
                                  2) nit: Line 216: put
                                  "System.exit(1);" on it's own line<br
                                    class="">
                                  <br class="">
                                  Oh, and create a bug: <a
                                    moz-do-not-send="true"
                                    href="https://bugs.openjdk.java.net/"
                                    rel="noreferrer" target="_blank"
                                    class=""><a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net">https://bugs.openjdk.java.net</a></a><br  \
class="">  <br class="">
                                  Thanks,<br class="">
                                  /Staffan<br class="">
                                  <br class="">
                                  <blockquote class="gmail_quote"
                                    style="margin:0 0 0
                                    .8ex;border-left:1px #ccc
                                    solid;padding-left:1ex"> On 29 okt.
                                    2015, at 14:54, Daniel D. Daugherty
                                    &lt;<a moz-do-not-send="true"
                                      href="mailto:daniel.daugherty@oracle.com"
                                      target="_blank" \
class="">daniel.daugherty@oracle.com</a>&gt;

                                    wrote:<br class="">
                                    <br class="">
                                    JVM/TI belongs to the Serviceability
                                    team so adding
                                    serviceability-dev@...<br class="">
                                    <br class="">
                                    Dan<br class="">
                                    <br class="">
                                    <br class="">
                                    <br class="">
                                    On 10/28/15 8:45 PM, Carsten Varming
                                    wrote:<br class="">
                                    <blockquote class="gmail_quote"
                                      style="margin:0 0 0
                                      .8ex;border-left:1px #ccc
                                      solid;padding-left:1ex"> webrev: <a
                                        moz-do-not-send="true"
                                        \
href="http://cr.openjdk.java.net/%7Ecvarming/jvmtiGen/"  rel="noreferrer" \
target="_blank"  class=""><a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~cvarming/jvmtiGen/">http://cr.openjdk.java.net/~cvarming/jvmtiGen/</a></a><br
  class="">
                                      bug: ?<br class="">
                                      <br class="">
                                      jvmtiGen is used to process a
                                      number of xml and xslt files in
                                      OpenJDK.<br class="">
                                      Currently jvmtiGen exits with exit
                                      code 0 regardless of its success.
                                      This<br class="">
                                      causes make to often consider a
                                      target finished when in fact the
                                      target<br class="">
                                      failed. It also leads to funny
                                      error checking after the execution
                                      of<br class="">
                                      jvmtiGen. For instance, in many
                                      trace.make files[*] a test for the<br
                                        class="">
                                      existence of the output file is
                                      carried out after the completion
                                      of<br class="">
                                      jvmtiGen. In a clean working
                                      repository that test is equivalent
                                      to jvmtiGen<br class="">
                                      exiting with a proper exit failure
                                      code on failure, but in a dirty
                                      working<br class="">
                                      repository the target file might
                                      just be pre-existing. This causes<br
                                        class="">
                                      unnecessary pain when working with
                                      files processed by jvmtiGen.<br
                                        class="">
                                      <br class="">
                                      In this change I chose to exit
                                      with exit code 1 whenever a
                                      failure is<br class="">
                                      detected, be it a dtd validation
                                      failure, an IO failure, or
                                      something else<br class="">
                                      entirely. This halts the building
                                      of OpenJDK on failures and
                                      ultimately<br class="">
                                      makes development easier. I also
                                      added a verbose option such that
                                      warnings<br class="">
                                      from the xml parser and dtd
                                      checker can be printed on stderr
                                      if desired.<br class="">
                                      Finally, I changed all the error
                                      message printing to stderr. :-)<br
                                        class="">
                                      <br class="">
                                      Let me know what you think.<br
                                        class="">
                                      <br class="">
                                      BTW. This is the first time I
                                      tried the webrev system, so
                                      hopefully it all<br class="">
                                      looks good. I havn't figured out
                                      how to create a bug yet, whence
                                      the<br class="">
                                      question mark.<br class="">
                                      <br class="">
                                      I wasn't sure if
                                      hotspot-runtime-dev is the right
                                      email alias. Please let<br
                                        class="">
                                      me know if there is a more
                                      appropriate alias for this email.<br
                                        class="">
                                      <br class="">
                                      [*] Why are so many of the
                                      non-shared makefiles almost
                                      identical?<br class="">
                                    </blockquote>
                                  </blockquote>
                                </blockquote>
                                <br class="">
                              </blockquote>
                              <br class="">
                            </div>
                          </div>
                        </blockquote>
                      </div>
                      <br class="">
                    </div>
                  </blockquote>
                  <br class="">
                </div>
              </div>
            </blockquote>
          </div>
          <br class="">
        </div>
      </div>
    </blockquote>
    <br>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic