<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi Jiri,<br></div><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I had  harm your patch a bit.<br>
I made the location configurable, as I cannot enforce linux distros to have the args file in bin folder.<br>
<br>
Still I kept the default of yours - bin.<br>
<br>
That naturally went to issue with searching for args file outside the distro world. So I added one<br>
more copy into win/linux static images.<br>
<br>
WDYT?<br></blockquote><div><br></div><div>I had a quick look (not tested) and it looks good.</div><div><br></div><div>I noticed you forgot to replace 1 hard-coded value in windows batch file:</div><div><span style="font-family:monospace,monospace">+  set "RUN_ARGS_LOCATION=%ITW_HOME%/bin/launchers-run.args"</span></div><div></div><div>should be</div><div><span style="font-family:monospace,monospace">+  set "RUN_ARGS_LOCATION=%ITW_HOME%/bin/itw-modularjdk.args"</span></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
IN addition I had renamed the variables to have common root, and properly used variable instead of<br>
hardcoded filename.<br>
<br>
WDYT?<br></blockquote><div><br></div><div>OK, except the typo.</div><div>Will test it soon or after you push the patch.</div><div><br></div><div>Laurent</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanx for patience. Unless you have much more objections, I will push for you tomorrow. (+rust<br>
launchers)<br>
<br>
J.<br>
On 2/8/19 4:26 PM, Laurent Bourgès wrote:<br>
> Hi Jiri,<br>
> I easily merge with your latest changes and it builds properly.<br>
> (I noticed build generated the win-deps folder now)<br>
> <br>
> Here is the updated patch.<br>
> <br>
> Cheers,<br>
> Laurent<br>
> <br>
> Le jeu. 7 févr. 2019 à 18:45, Laurent Bourgès <<a href="mailto:bourges.laurent@gmail.com" target="_blank">bourges.laurent@gmail.com</a><br>
> <mailto:<a href="mailto:bourges.laurent@gmail.com" target="_blank">bourges.laurent@gmail.com</a>>> a écrit :<br>
> <br>
>     Jiri,<br>
> <br>
>     Le jeu. 7 févr. 2019 à 17:12, Jiri Vanek <<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a> <mailto:<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a>>> a écrit :<br>
> <br>
>         So the cross-build-bats is pushed.<br>
> <br>
> <br>
>     Yes I got the notification, thx.<br>
> <br>
> <br>
>         I will elaborate on rest tomorrow.<br>
>         I yu adapt your patch to current head, it will be significant help.But is not necessary.<br>
> <br>
> <br>
>     I will try merging tomorrow as I stay at home (flu).<br>
> <br>
>     Thank you too<br>
>     Laurent<br>
> <br>
> <br>
>         TYVM for cooperation!<br>
>          J.<br>
> <br>
> <br>
>         On 2/7/19 3:47 PM, Laurent Bourgès wrote:<br>
>         > Hi Jiri,<br>
>         > Good to have your feedback.<br>
>         ><br>
>         ><br>
>         > Le jeu. 7 févr. 2019 à 14:27, Jiri Vanek <<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a> <mailto:<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a>><br>
>         <mailto:<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a> <mailto:<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a>>>> a écrit :<br>
>         ><br>
>         >     Hi Laurent!<br>
>         ><br>
>         >     How are we with this?<br>
>         ><br>
>         ><br>
>         > I prefer you to do its integration as my patch has conflicts with your changes (am) and my<br>
>         changes<br>
>         > are too big so splitting patch is needed, I agree.<br>
>         ><br>
>         ><br>
>         >     Can I commit my "crosscompile" bats patch, and will you adapt?<br>
>         >     Maybe it woudl be better if you first adapt your patch to my crosscompile patch before<br>
>         I push  - to<br>
>         >     test it  abit.<br>
>         >     Or do you suggest any other workflow?<br>
>         ><br>
>         ><br>
>         > Ok for me, I will follow and merge. If anything is wrong or missing, I will propose<br>
>         smaller patches,<br>
>         > sorry.<br>
>         ><br>
>         ><br>
>         >     After crosscompile patch is in, I 'm most likely +1  for pushing your new bat+the work<br>
>         on @arg file<br>
>         >     (wiht some tuning on my side before push).<br>
>         ><br>
>         ><br>
>         > Excellent plan.<br>
>         ><br>
>         ><br>
>         >     Until now I have not found better solution for jigsaw and other params, so also rust<br>
>         launchers will<br>
>         >     do as you do - on jdk9+, will add this @file to java cmd. That will be done as<br>
>         separate changeset<br>
>         >     (by me)<br>
>         ><br>
>         ><br>
>         > Sounds good and easier to do in rust.<br>
>         ><br>
>         > Thanks a lot.<br>
>         ><br>
>         > Laurent<br>
>         ><br>
>         ><br>
>         >     Thanx!<br>
>         >      J.<br>
>         >     On 1/30/19 3:46 PM, Laurent Bourgès wrote:<br>
>         >     > Hi Jiri,<br>
>         >     ><br>
>         >     >     > I finally finished rewriting / unify windows batch script with linux<br>
>         >     >     > shell script.<br>
>         >     >     ><br>
>         >     >     Taht is bat scripting masterpiece. Congratualtions for gaining eternal patience.<br>
>         >     ><br>
>         >     ><br>
>         >     > thanks.<br>
>         >     >  <br>
>         >     ><br>
>         >     >     > Changes:<br>
>         >     >     > - Xnofork is no more needed on windows because argument parsing is working now<br>
>         >     >     > - use run-args + fix classpaths (portable itw with deps)<br>
>         >     >     > - fix CRLF in bat files<br>
>         >     ><br>
>         >     >  <br>
>         >     > One remaining difference: windows batch does not parse deployment.properties (to<br>
>         look up JVM)<br>
>         ><br>
>         ><br>
>         >     This is ok, I do not intend to add it. If anybody wish, anybody can.  And yes,<br>
>         JAVA_HOME can<br>
>         >     help a lot.<br>
>         >     ><br>
>         >     >     The crlg hack is necessary only for args file.  That really have to be another<br>
>         patch for that.<br>
>         >     >     (still we can keep the thread, as many was told here already))<br>
>         >     ><br>
>         >     ><br>
>         >     > I think the contrary, only batch files really expect CRLF line endings. if you edit<br>
>         using<br>
>         >     notepad or<br>
>         >     > if you use echo "text" then the line ending is important.<br>
>         >     > I suppose jvm arg file are well handled by jvm concerning line endings, so it is not<br>
>         required to<br>
>         >     > have CRLF in launcher_run.args.<br>
>         >     ><br>
>         >     > I agree some changes to Makefile.am could be postponed to another patch.<br>
>         >     >  <br>
>         >     ><br>
>         >     >     ><br>
>         >     >     > I tested it on windows 7/10 and it works great.<br>
>         >     >     > I noted windows shortcuts work now, except on JDK12 (maybe related to<br>
>         >     >     > jigsaw classloader ?)<br>
>         >     ><br>
>         >     >     No idea. Adding Joel. to CC. Maybe you will be able to look into it?<br>
>         >     >     Alex quite IcedTea recently, so there is no windows guy around. I will try to<br>
>         get help in<br>
>         >     RH with<br>
>         >     >     windows, otherwise windows "support" of itw will depend purely on community.<br>
>         >     ><br>
>         >     ><br>
>         >     >     ><br>
>         >     >     > I have questions about header variables (too many) and why these need<br>
>         >     >     > to be absolute paths ?<br>
>         >     >     Because in distribution, they ecan be anywhere.<br>
>         >     ><br>
>         >     >     > it could be relative paths if we add ITW_HOME on the top.<br>
>         >     >     > Of course, it depends between DIST or BUNDLE modes ... like for<br>
>         >     >     > dependencies (win/linux_deps folders)<br>
>         >     >     Yes. Bundle mode will be more happy with a bit different setup. Maybe ITW will<br>
>         once grow<br>
>         >     to be more<br>
>         >     >     standalone then distributions frindly.<br>
>         >     ><br>
>         >     ><br>
>         >     > OK, but it means bundle support needs to re-compute path relative to ITW_HOME on<br>
>         linux & windows.<br>
>         >     > In this case, it is not obvious to guess the relative path to a resource from its<br>
>         absolute<br>
>         >     path and<br>
>         >     > lots of path are hard-coded in windows batch.<br>
>         >     > For example:<br>
>         >     > set "BINARY_LOCATION=%ITW_HOME%/bin/%PROGRAM_NAME%.bat"<br>
>         >     ><br>
>         >     > set "SPLASH_LOCATION=%ITW_HOME%/share/icedtea-web/javaws_splash.png" <br>
>         >     ><br>
>         >     > It is not good, as ITW resource may be moved in Makefile or by installer, so these<br>
>         paths /<br>
>         >     variables<br>
>         >     > must be maintained if any file is renamed or moved (netx.jar -> javaws.jar recently !)<br>
>         >     ><br>
>         >     > Any idea ? It is possible to provide both absolute path & relative paths but it will<br>
>         be too ugly &<br>
>         >     > complicated.<br>
>         >     >  <br>
>         >     ><br>
>         >     >     ><br>
>         >     >     > Please explain how do you plan to make portable (cross-platform) packages ?<br>
>         >     >     Nothing like that!<br>
>         >     >     I will build (and I already am) linux tarablls on linux, and windows tarballs on<br>
>         windows.<br>
>         >     ><br>
>         >     ><br>
>         >     > In my tests, I build on linux with batch generated (wrong paths inside) then create<br>
>         a zip file<br>
>         >     with<br>
>         >     > linux/win deps.<br>
>         >     > It works on windows using bundle mode (ITW_HOME pointing to the script dir).<br>
>         >     > Here is my do-package.sh script (post-build):<br>
>         >     > ITW_HOME=./install/<br>
>         >     > mkdir -p $ITW_HOME/linux-deps-runtime/<br>
>         >     > mkdir -p $ITW_HOME/win-deps-runtime/<br>
>         >     ><br>
>         >     > # add rhino ?<br>
>         >     ><br>
>         >     > # linux:<br>
>         >     > cp tagsoup-1.2.1.jar $ITW_HOME/linux-deps-runtime/<br>
>         >     ><br>
>         >     > # windows:<br>
>         >     > cp tagsoup-1.2.1.jar $ITW_HOME/win-deps-runtime/<br>
>         >     > cp mslinks.jar $ITW_HOME/win-deps-runtime/<br>
>         >     ><br>
>         >     > zip -r itw-1.8-install.zip $ITW_HOME<br>
>         >     >  <br>
>         >     ><br>
>         >     >     I had attached my version of crosscompiler bats on linux. Compared to yours,<br>
>         thay are true on<br>
>         >     >     windows. Not sure how you were handluing this case. Otherwise they loks same<br>
>         asyours. I<br>
>         >     wrote them<br>
>         >     >     in Monday, but then fell down with flue which keeps persisting.<br>
>         >     ><br>
>         >     ><br>
>         >     > Yes it is really close. As I am not an automake expert, I prefer your approach (true<br>
>         on windows).<br>
>         >     > I will merge your changes and adapt my Makefile patch with less changes (LUNCHERS -><br>
>         LAUNCHERS,<br>
>         >     > run-args, fix CRLF in batch files)<br>
>         >     >  <br>
>         >     ><br>
>         >     ><br>
>         >     >     Also i noticed, that *image* as done on linux, with crosscompile on, is useless on<br>
>         >     windows.  Yes,<br>
>         >     >     you have bats, but windows-deps directory is  corrupoted from linux build.<br>
>         >     >     Whre I wont to keep this state on 1.7, in head, the only possible solution ois<br>
>         to get rid of<br>
>         >     >     linux/windows*deps*dirs and replace them by singledir deps.  WDYT?<br>
>         >     ><br>
>         >     ><br>
>         >     > I can not build on windows, so I am making a cross-platform package on linux (jar<br>
>         files +<br>
>         >     resources<br>
>         >     > + shell launchers).<br>
>         >     > I agree it would be simpler to put dependencies in a single location (jar files are<br>
>         >     cross-platform).<br>
>         >     >  <br>
>         >     ><br>
>         >     >     In addition, yours patch contains also the arg file. I probably agree that it is<br>
>         way to<br>
>         >     go. This<br>
>         >     >     will do its job for shell launchers. For rust ones, I will need to add the<br>
>         cp/bootcp and<br>
>         >     other java<br>
>         >     >     switches customisation properties into deployment.properties. WDYT?<br>
>         >     ><br>
>         >     ><br>
>         >     > The aim consist in having all ITW jigsaw args (add-reads / add-opens) gathered in a<br>
>         single<br>
>         >     place and<br>
>         >     > minimize the jvm args.<br>
>         >     > I wonder if rust launchers could do the same (use @launchers-run.args) and only<br>
>         customize the<br>
>         >     > --patch-moduleargs (netx,plugin,js) and of course, bootclasspath + classpath<br>
>         (absolute paths)<br>
>         >     >  <br>
>         >     ><br>
>         >     >     ><br>
>         >     >     > PS2: you can have a look @github to see complete shell scripts (not diff):<br>
>         >     >     > <a href="https://github.com/bourgesl/icedtea-web/tree/run-args/shell-launcher" rel="noreferrer" target="_blank">https://github.com/bourgesl/icedtea-web/tree/run-args/shell-launcher</a><br>
>         >     ><br>
>         >     >     h<t is really splendid work. I\m unable to have nay comment to new bat file,<br>
>         except that<br>
>         >     it loosk<br>
>         >     >     much better then old one, and that  it do not looks malicious:)<br>
>         >     ><br>
>         >     ><br>
>         >     > Yes I mimic the linux behaviour (functional blocks); only the JVM lookup (from<br>
>         >     deplyoment.properties<br>
>         >     > is not handled).<br>
>         >     >  <br>
>         >     ><br>
>         >     >     I'm for its incorporation to both head and 1.7<br>
>         >     ><br>
>         >     > Great !<br>
>         >     ><br>
>         >     ><br>
>         >     >     Few comments to ther chnegs inline:<br>
>         >     >     ...<br>
>         >     >     ><br>
>         >     >     > patch-run-args-win.2.diff<br>
>         >     ><br>
>         >     >     All refactorings/untypos must go as separate patch. Sorry.  Feel free topush<br>
>         them without<br>
>         >     any other<br>
>         >     >     review. They are making reading of this hard.<br>
>         >     ><br>
>         >     ><br>
>         >     > Please be more precise. You mean fixing typos in Makefile.am (luncher -> launcher) ?<br>
>         >     >  <br>
>         >     ><br>
>         >     >     > +if ENABLE_WIN_SHELL_LAUNCHERS<br>
>         >     >     > +# convert Unix newlines (LF) to DOS format:<br>
>         >     >     > +  line_end_edit_launcher_script=-e "s/\$$/\r/"<br>
>         >     >     > +else<br>
>         >     >     > +  line_end_edit_launcher_script=<br>
>         >     >     > +endif<br>
>         >     ><br>
>         >     >     Only for args file path, right?<br>
>         >     ><br>
>         >     > No, it only fix generated batch files.<br>
>         >     >  <br>
>         >     ><br>
>         >     >     > +if "%MODULAR_JDK%" == "YES" (<br>
>         >     >     > +  rem warning extra escaping<br>
>         >     >     > +  set "MODULAR_ARGS=--patch-module "java.desktop=%NETX_JAR%;%PLUGIN_JAR%""<br>
>         >     ><br>
>         >     >     why NETX.JAR?<br>
>         >     ><br>
>         >     ><br>
>         >     > %NETX_JAR% means $NETX_JAR ie .../javaws.jar<br>
>         >     > It is required to patch jdk.desktop module with ITW code (jigsaw issue as before).<br>
>         >     ><br>
>         >     >  <br>
>         >     ><br>
>         >     >     > +# Support Modular JDK (jigsaw):<br>
>         >     >     >  MODULAR_JDK="NO"<br>
>         >     >     > -version=`${JAVA} -version 2>&1 | head -n 1 | cut -d'-' -f1 | cut -d'"' -f2 |<br>
>         cut -d'.' -f1`<br>
>         >     >     > +fullversion=`${JAVA} -version 2>&1`<br>
>         >     >     > +echo "fullversion: $fullversion"<br>
>         >     ><br>
>         >     >     I would skipp the fullversion: string, and printed $fullversion to stderr as are<br>
>         people<br>
>         >     used from<br>
>         >     >     jdk/ (?)<br>
>         >     ><br>
>         >     ><br>
>         >     > Not understood. I wanted to avoid calling java -version twice and but show JVM<br>
>         version once.<br>
>         >     > What do you propose ?<br>
>         >     >  <br>
>         >     ><br>
>         >     >     > <br>
>         >     >     > +#echo "CMD: ${COMMAND[@]}"<br>
>         >     ><br>
>         >     >     The commented echo is weird. One is usually running this in -x subshell to see<br>
>         details.<br>
>         >     ><br>
>         >     ><br>
>         >     > Ok I will remove such debugging lines.<br>
>         >     ><br>
>         >     > PS: Would it sound possible for you to handle merging & pushing this patch ?<br>
>         >     > Anyway I will wait for your changes to Makefiles before merging.<br>
>         >     ><br>
>         >     > Cheers,<br>
>         >     > Laurent<br>
>         ><br>
>         ><br>
>         >     --<br>
>         >     Jiri Vanek<br>
>         >     Senior QE engineer, OpenJDK QE lead, Mgr.<br>
>         >     Red Hat Czech<br>
>         >     <a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a> <mailto:<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a>> <mailto:<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a><br>
>         <mailto:<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a>>>    M: +420775390109<br>
>         ><br>
> <br>
> <br>
>         -- <br>
>         Jiri Vanek<br>
>         Senior QE engineer, OpenJDK QE lead, Mgr.<br>
>         Red Hat Czech<br>
>         <a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a> <mailto:<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a>>    M: +420775390109<br>
> <br>
> <br>
> <br>
> -- <br>
> -- <br>
> Laurent Bourgès<br>
<br>
<br>
-- <br>
Jiri Vanek<br>
Senior QE engineer, OpenJDK QE lead, Mgr.<br>
Red Hat Czech<br>
<a href="mailto:jvanek@redhat.com" target="_blank">jvanek@redhat.com</a>    M: +420775390109<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature">-- <br>Laurent Bourgès</div></div></div></div>