Update on Windows shortcut features for ITW

Jiri Vanek jvanek at redhat.com
Mon Jan 7 16:59:53 UTC 2019


I had reviewed your changes, your java code, and provided integration to ITW, including  optional
mslinks[1] dependence, which I will include in Official ITW builds.
[1] https://github.com/DmitriiShamrikov/mslinks Is this right usptream?  Is it ok to use freshest
version each ITW release?

I had to do some changes to your code (especially to make it optional) and had dared to do few
improvements (including additional functionality), tested linux parts. Still, I have few questions
and recommendations. Also I had fixed few bugs in your new code.

Small changes:
 - moved all windows-specific stuff from ApplicationInstance to separate, new file WindowsDesktopEntry
  -- It is mandatory step, to make mslin library optional.
  -- Added an interface "shared" by XDesktopEntry and WindowsDesktopEntry. Necessary to have
WindowsDesktopEntry optional.
 - applied ITW codestyle
 - removed lambda and method reference. Itw will move to lambdas with 1.9. Do you mind to review
sanity of this change?
 - mentioned in docs, that xclearcache can take arguments
 - rewritten "A" for enum. What is it. To Add? to Append? Actually.. Only "A" was ever used... Maybe
drop it completely?
 - made jnlp-path value visible in cache-viwer
 - Used KEY_JNLP_PATH instead of hardcoded "jnlp-path"
 - mentioned new feature in NEWS
 - added you to authors
 - used XDesktopEntry.getJavaWsBin instead of calls to "icedtea-web.bin.location"
 - reused extracetd getFavIcon.
 - unified calls to shortcutList.txt instead of hardcoding it
 - I had to (of course) modify windows bat launchers (the change from make file affected also linux
sh launchers directly via bootclassapth).

Fixed bugs:
 - the  `String jnlpPath = JNLPRuntime.getInitialArguments().get(1); //get jnlp from args passed`
was VERY WRONG (fix it in your impl if you will not use future ITW upstream sources/binaries). If
you put javaws.sh some.jnlp it dies, because some.jnlp is on iindex 0. If you use javaws.sh -verbose
some.jnlp, it works, but you get -verbose as identifier.
  -- Also fixed this to work with -html.
 - used  XDesktopEntry.getDesktopIconName whcih sanitize filename, instead of direct
 - fixed few == instead of equals with strings

 - fixed favicon obtaining (was not working fine for nested icons and for windows servers)
 - cmdline switch Xcacheids to list available IDs in cache
 - gui for deleting items via ID in itweb-settings
 - next to jnlp-path key for deletion, I added also domain key. As jnlp-path is definitely good
idea, under some circumstances, there can be two jnlp-paths with same resources. This is what
deleting by domain would fix.
 - made cache-viwer aware about jnlp-path. (you can sort here by any column)

 1) how do you feel about hardoded `Desktop` in your paths? Cant localized desktop bear another
name?  Sill I do not know better way:(
 2) As I mentioned in small changes, I had to adapt bat launchers. The launchers are not in your
archive. How do you get mslinks to classpath?
 3) you use same location of jnlp file in the lnk as I do for XDesktopEntry. That measn *remote*
jnlp is stored. So even if you clear cache, your LNks will most likely work. Dont you really wish to
reconsider removing of links during full clear of cache? Or generally this cache-lnks connection>
 4) enum of "A". What is it. To Add? to Append? Actually.. Only "A" was ever used... Maybe drop it
 5) jnlp-path as key. This is most genial, and most strange part of this patch. It is far frombeing
perfect, but in early satges of jnlp launch it is the only avaialble information.  Thus I think it
is ok (except the nit in improvemnts). I was thinking about many other keys - codebase, doc base.
final jnlp url... But all those have different sidekicks. Dont forget, that the jnlp-path can be tmp
file:(( And I must not forget that final url can be some
http://some.wb/some/sevlet/getJnlp?id=terrible_hash :(. So next to your jnlp_from_argument, I added
possibility to clear also by *domain*. Well domain is removing most of the issues mentioned above,
but have different - one app can come from more domains :) This is probably most discussion
nedeeding part of this patch. But I have not found golden solution. then jnlp-path + domain seems to
be good combo.

The patch, as is now, seems to be feasible for 1.7.
For 1.8, it interfere with those two patches:
 * http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2018-December/040674.html
 * http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2018-December/040675.html
(as all three are touching the places where dependencies jars are flowing through makefile to lunchers)
(well, 1.7 is partially depnding onthem to, as portable shell launchers can go to 1.7)

Generally I'm very happy with the patch. It is much smaller then I though, especially considering
the troubles I had with some parts of XDesktopEntry.
Where I feel unsure is removal of links during clear of cache.  XDesktopEntry is designed, whenever
possible (even if you lunch local file, which can be somehow tracked to remote), it saves *remote*
url to teh shortcut. So even if you clear cache,  the application will work for you if you are
online (similarly, this approach works even if you do not alert cache, and are offline).
You seems to be using same mechanism in LNKs.  Don't you wish to reconsider the clearcache+removal
of windows shortcuts?  Eg remove shortcuts only if  argument is passed to xclearcache (so onl single
app is removed)?

Current changelog entry (please read, you may find something I forget above):

2018-01-07  Joel Tesdall <jtesdall at mapcon.com>
            Jiri Vanek <jvanek at redhat.com>

	Added optional windows desktop integration
	* AUTHORS: added Joel
	* Makefile: Excluded (WindowsDesktopEntry.java) if mslinks are not included, added mslinks to
	included mslinks to windows and linux runtime libs, added MSLINKS_JAR to other composeclasspath calls
	* NEWS: mentioned windows desktop support, mentioned listing of cache and operations via id.
	* acinclude.m4: added check (IT_CHECK_FOR_MSLINKS) for optional mslinks.jar, strong warning printed
if build is on windows
	* configure.ac: call (IT_CHECK_FOR_MSLINKS)
	* netx/net/sourceforge/jnlp/Launcher.java: new variable of (KEY_JAVAWS_LOCATION) to replace
hardcoded icedtea-web.bin.location over netx.
	* netx/net/sourceforge/jnlp/OptionsDefinitions.java:  re-declared clear cache to take none or one
	Added Xcacheids switch for listing the cache (works with verbose)
	* netx/net/sourceforge/jnlp/cache/CacheDirectory.java: refactored hardcoded ".info" to constant.
	* netx/net/sourceforge/jnlp/cache/CacheEntry.java: introduced KEY_JNLP_PATH and used to set
jnlp-path attribute
	* netx/net/sourceforge/jnlp/cache/CacheLRUWrapper.java: hide private constructor, declared and
provided (windowsShortcutList)
	* netx/net/sourceforge/jnlp/cache/CacheUtil.java: extracted and used (checkToClearCache). Added
second method clearCache
	with arg to clear only specific part of cache. Clear cache also alerts windows desktop files now
via new removeWindowsShortcuts.
	Added methods to lists ids and details from cache listCacheIds and getCacheIds. Included new inner
class CacheId to encapsualte
	various types of id - CacheJnlpId and CacheDomainId now.
	* netx/net/sourceforge/jnlp/cache/DirectoryNode.java: only adapted to .info refactoring
	* netx/net/sourceforge/jnlp/cache/ResourceDownloader.java: Save main argument, or jnlp argument or
html argument to
	jnlp-path .info entry if found.
	* netx/net/sourceforge/jnlp/controlpanel/CacheAppViewer.java: gui to itweb-settings cache pane to
allow comfortable
	listing of ids and deleting via those grouping.	New file.
	* netx/net/sourceforge/jnlp/controlpanel/CachePane.java: added logic to show .info details for each
file shown by cache viewer.
	(generateData) made jnlp-path aware, made public and reused several times
	* netx/net/sourceforge/jnlp/controlpanel/TemporaryInternetFilesPanel.java: added button to show
dilog which is deleting by id
	* netx/net/sourceforge/jnlp/resources/Messages.properties: added BXclearcache BXSingleCacheCleared
	BXSingleCacheMoreThenOneId BXSingleCacheFileCount BXcacheids NOAnonorone WinDesktopError. Modified
BXclearcache. Improved
	EXAWdesktopWants EXAWdesktopDontWants EXAWsubmenu EXAWmenuWants EXAWmenuDontWants EXAWrememberByApp
	EXAWrememberByAppTooltip EXAWbrowsersTolltip SDesktopShortcut
	* netx/net/sourceforge/jnlp/runtime/ApplicationInstance.java: added if isWindows reflective calls
to  WindowsDesktopEntry
	and original XdesktopEntry work moved to else part.
	* netx/net/sourceforge/jnlp/runtime/Boot.java: now offer getter for optionParser (so it van be
reused in ResourceDownloader)
	Added understanding to Xcacheids. Understanding to Xclearcache adapted to its new optional argument
	* netx/net/sourceforge/jnlp/util/GenericDesktopEntry.java: interface for (future)
WindowsDesktopEntry and XDesktopEntry unification
	* netx/net/sourceforge/jnlp/util/WindowsDesktopEntry.java: new file, implementation fo windos
desktop integration
	via lnk files generated by mslinks.jar. Unlike XDesktop integration, it swarms also uninstall
	* netx/net/sourceforge/jnlp/util/XDesktopEntry.java: Mostly adapted to refactorings. Extracted
extraction of favicon to method,
	reused, and improved to try more locations on server.
	* netx/net/sourceforge/jnlp/util/optionparser/OptionParser.java: adapted to refactorings
	* shell-launcher/launchers.bat.in: mslinks included in bootclasspath
	* tests/netx/unit/net/sourceforge/jnlp/cache/CacheUtilTest.java: addd tests for CacheId
	* tests/netx/unit/net/sourceforge/jnlp/util/XDesktopEntryTest.java: Added tests for favicon extraction

Most important last: Can you please test this patch on latest ITW, if it still works on windows?

Happy hacking!

On 12/26/18 7:15 PM, jtesdall wrote:
> I have my changes complete for now. I have attached a zip file with my
> changed .java classes and the new mslinks.jar needded to create windows
> shortcuts. I have also included the original unchanged source code in the
> original directory.
> MAPCONIcedTeaSRCChanges.zip
> <http://openjdk.5641.n7.nabble.com/file/t3943/MAPCONIcedTeaSRCChanges.zip>  
> Jiri, you may want to change the way I have done some things as it pertains
> to ITW. What I have seems to work well though. I have done quite a bit of
> testing and will be doing substantially more in the coming weeks in my
> office. I am hoping to have this in production by Jan 15th. Would you please
> do whatever is needed to submit these as changesets and test on linux to
> make sure I didn't break anything there?
> Note: The reason my netx.jar would not work after copying in boot.class is
> that the jar struccturre had changed at some point and added a whole
> directory of classes net\sourceforge\swing. Once I copied in those classes I
> could create a my own jar by replacing classes once again.
> Here is what I have done as it pertains to each class:
> *changed in runtime\ApplicationInstance.java*
> many changes to add windows shortcut to desktop and add Start menu item
> taken from JNLP. This menu has the app shortcut and an uninstall shortcut.
> added new library mslinks to create windows shortcuts since they are in a
> binary format.
> favicon.ico from apps root is downloaded and cached and used for shortcut
> ico, if none is present it will not have a pretty icon
> *changed in cache\CacheUtil.java*
> added code to delete shortcut and menu subdir in clearcache
> added overload function for clearcache to delete one app at a time from
> cache instead of clearing the whole thing
> *changed in runtime\Boot.class*
> added code to allow passing a parameter to -xclearcache with parameter of
> jnlpPath to remove one app from cache. This was needed to allow a Windows
> App uninstall shortcut. If no parameter is passed all of the chache is
> cleared as was normal before my change. This was done in init function.
> *changed in cache\ResourceDownloader.class* 
> add new parameter jnlpPath to .info file so files can be found by jnlp. This
> allows clearcache to mark one applications files for deletion so a later
> call to cleancache works and actually deletes the files correctly.
> added these lines String jnlpPath =
> JNLPRuntime.getInitialArguments().get(1).toString();
> entry.setJnlpPath(jnlpPath);
> *changed in cache\CacheEntry.class*
> added setJnlpPath(jnlpPath) function to add jnlpPath to .info files as
> needed by ResourceDownloader above.
> *changed in resources\Messages.properties*
> improved english for some messages
> --
> Sent from: http://openjdk.5641.n7.nabble.com/OpenJDK-Distribution-specific-Packaging-f25548.html

Jiri Vanek
Senior QE engineer, OpenJDK QE lead, Mgr.
Red Hat Czech
jvanek at redhat.com    M: +420775390109
-------------- next part --------------
A non-text attachment was scrubbed...
Name: windowsDesktopIntegration.patch
Type: text/x-patch
Size: 95572 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20190107/5a03bea0/windowsDesktopIntegration-0001.patch>

More information about the distro-pkg-dev mailing list