Opened 7 years ago

Closed 4 years ago

#895 closed enhancement (fixed)

user comments for greenstone 3

Reported by: kjdon Owned by: ak19
Priority: moderate Milestone: 3.09 Release
Component: Greenstone3 Runtime Severity: enhancement
Keywords: Cc:

Description

Has this been done yet? if not, add it to gs3.

Change History (6)

comment:2 by ak19, 5 years ago

Owner: changed from nobody to ak19

Changesets for adding User Comments feature to GS3. So far tested on Linux.

http://trac.greenstone.org/changeset/31537

http://trac.greenstone.org/changeset/31540 to http://trac.greenstone.org/changeset/31547

http://trac.greenstone.org/changeset/31555 to http://trac.greenstone.org/changeset/31559

Still to do: test GS3 User comments feature on Windows, and remove the debug stmts that have now merely been commented out.

comment:3 by ak19, 5 years ago

Windows testing revealed a problem with adding user comments and document editing: there was blocking in the java code launching perl at GS2PerlConstructor.runPerlCommand().

Wrote SafeProcess.java based on the combination of GLI's Input- and OutputStreamGobblers and how FormatConversionDialog used them, and used the new SafeProcess class in GS2PerlConstructor.runPerlCommand().

Commits:

http://trac.greenstone.org/changeset/31568, http://trac.greenstone.org/changeset/31569

http://trac.greenstone.org/changeset/31572 to http://trac.greenstone.org/changeset/31574

http://trac.greenstone.org/changeset/31578,

http://trac.greenstone.org/changeset/31579 which contains a correction (and undoing accidental commit: http://trac.greenstone.org/changeset/31580 )

Windows testing appeared successful: user comments and doc editing worked. No more blocking.

Then brought SafeProcess over to GLI to first replace the extgernal Input- and OutputStreamGobbler classes, and to make GLI's FormatConversionDialog, GS3ServerThread (and Gatherer's use of it) and DownloadPane go through SafeProcess when running processes.

Commits: http://trac.greenstone.org/changeset/31581 to http://trac.greenstone.org/changeset/31587

which also required bringing the GS3 version of SafeProcess up to speed. The differences between the GLI and GS3 version are in package name and usage of logger by GS3. Need to retest the GS3 changes with doc editing and user comments on Windows.

comment:4 by kjdon, 5 years ago

Milestone: Greenstone 3 catch-up3.09 Release

comment:5 by ak19, 4 years ago

Other Java classes of GLI and GS3 src code were shifted from using Java's Process class directly to using our new SafeProcess class.

The changes in GLI were to classes

GLI

wharariki:[265]/Scratch/ak19/gs3-svn-15Nov2016/gli>fgrep -rl "Process" src

+ src/org/greenstone/gatherer/util/SafeProcess.java

+ src/org/greenstone/gatherer/gui/FormatConversionDialog.java

+ src/org/greenstone/gatherer/gui/DownloadPane.java

+ src/org/greenstone/gatherer/util/GS3ServerThread.java

+ 2 src/org/greenstone/gatherer/Gatherer.java [2 MORE but don't want to mess with those]

+ 1 src/org/greenstone/gatherer/download/ServerInfoDialog.java

+ 2 src/org/greenstone/gatherer/greenstone/Classifiers.java

+ 2 src/org/greenstone/gatherer/greenstone/Plugins.java

+ 1 src/org/greenstone/gatherer/collection/ScriptOptions.java

+ !! src/org/greenstone/gatherer/util/ExternalProgram.java && file/FileAssociationManager.java

After updating GS3 src code to using SafeProcess, returned to

VERY HARD:

+ 1 src/org/greenstone/gatherer/shell/GShell.java

VERY, VERY HARD:

+ 2 src/org/greenstone/gatherer/download/DownloadJob.java

GS3 src code:

fgrep -r "Process" . | grep -v ".svn" | grep -v "~" | less

+ 2 ./java/org/greenstone/admin/guiext/Command.java

X ./java/org/greenstone/gsdl3/util/Processing.java

+ MANY OCCURRENCES ./java/org/greenstone/gsdl3/service/MapRetrieve.java

+ Uses Processing 2 times: ./java/org/greenstone/gsdl3/util/GDBMWrapper.java

+ ./java/org/greenstone/util/BrowserLauncher.java

+ ./java/org/greenstone/util/RunTarget.java

In the GLI and GS3 src files modified to use SafeProcess above, only the modifications to GS3's Command.java, MapRetreive.java and GDMBWrapper.java are untested. All others have been tested, at least on Linux. Many have also been tested on Windows.

Tested GLI's collection building (uses GShell.java's modifications) and DownloadJob.java on all 3 OS. Both have a Cancel button that should cancel any processes and subprocesses launched. Before the modifications, subprocesses would still be running on Windows and during (full-)import on the Mac when cancel was pressed. But now have added code to SafeProcess to issue OS specific system calls to terminate the process and its subprocesses successfully on Windows and Mac too.

Also tested GS3 user comments and the online GS3's doc editor's meta editing followed by rebuilding. All worked successfully on Linux, Windows and Mac with the modifications to use SafeProcess.

The modifications to GLI started with GS3ServerThread.java and other *easily* changed classes in http://trac.greenstone.org/changeset/31582 (beginning of GLI modification) until http://trac.greenstone.org/changeset/31665 (GS3 src code modification).

The version of GS3 and GLI Java code before the GS3 src was modified is at http://trac.greenstone.org/browser/main/tags/GS3-src-SafeProcess

Next, GLI's GShell.java and any associated files were modified along with SafeProcess, to support cancelling.

Then, around 22 May, GLI's DownloadJob.java and any associated class files were modified along with SafeProcess to ensure subprocesses were safely terminated on Windows when the download action was cancelled OR when building (GShell.java) was cancelled. The Mac modifications to SafeProcess to ensure subprocesses were terminated was finished on 26 May 2017.

The tagged version of the GS3 and GLI Java code for when all the changes related to using SafeProcess were made to GS3 and GLI code is at http://trac.greenstone.org/browser/main/tags/UsingSafeProcess

comment:6 by ak19, 4 years ago

Resolution: fixed
Status: newclosed

More changesets

http://trac.greenstone.org/changeset/31711 to http://trac.greenstone.org/changeset/31718

These changesets involved:

  • the remaining 2 instances of using Runtime.exec/Java Process in Gatherer have been ported to using SafeProcess. These instances were of the inner classes Gatherer.ExternalApplication and Gatherer.BrowserApplication, both of which needed SafeProcess to have the cancel functionality before they could be ported to use SafeProcess. Tested the modified inner classes on Linux.

When tested on Mac, I found that with or without these new changes, GLI does not wait to exit if external applications are still running. On Linux (and the way GLI is coded), GLI's GUI may disappear, but the overall GLI process still waits for external processes launched via GLI to have exited before GLI exits. This doesn't happen on Mac with either the original or the modified code. Worth investigating. But at least the modified code has not broken something that used to work.

Note: See TracTickets for help on using tickets.