Separate workspace for files generated from plugins #3152

jelovirt · 2018-11-15T15:54:52Z

Add support for a workspace directory outside the DITA-OT installation that contains the files generated during the plugin integration.

Note: The discussion and examples below use OS X as the platform for simplicity's sake. Windows and Linux support will be part of the final implementation.

The goal is to allow DITA-OT installation to be read-only, and all generated files can be written to and read from e.g. ~/Library/Application Support/DITA-OT on OS X or %APPDATA%\DITA-OT on Windows. In order to support a distribution package that doesn't require a installation step, the code should fall back to files located in the DITA-OT installation directory.

The old generated file would be that inside DITA-OT installation we'd have

~/dita-ot/plugins/foo/build_template.xml
~/dita-ot/plugins/foo/build.xml

with the separate workspace, we'd have

~/dita-ot/plugins/foo/build_template.xml
~/Library/Application Support/DITA-OT/plugins/foo/build.xml

The workspace directory should be overridable with workspace key in the configuration properties to support multiple DITA-OT installations; value workspace = . will use the DITA-OT installation directory as the workspace.

The default distribution package should use DITA-OT installation as the workspace directory. DITA-OT installed with e.g. Brew could use macOS's ~/Library/Application Support/DITA-OT as the workspace with an integration step as part of the installation process.

jelovirt · 2018-11-15T16:00:25Z

Ping @raducoravu @eerohele, you might be interested in this feature.

raducoravu · 2018-11-17T07:20:59Z

@jelovirt that's great, I will try to compile a DITA OT from your branch and work with it.

jelovirt · 2018-11-17T07:40:11Z

@raducoravu This is still just a POC and doesn't work yet. I wanted to work in public to get comments in early, so that this doesn't end up being something that's not really needed or wanted.

eerohele · 2018-11-17T19:46:50Z

src/main/java/org/dita/dost/ant/ExtensibleAntInvoker.java

@@ -611,6 +639,8 @@ public void addConfiguredDitaFileset(final FileInfoFilterElem fileInfoFilter) {

 private String name;
 private String value;
+ // XXX This should be List<ResourceCollection>
+ private List<FileSet> filesets = new ArrayList<>();;


Double semicolon.

eerohele · 2018-11-17T19:50:01Z

src/main/config/configuration.properties

@@ -15,6 +15,7 @@ plugindirs = plugins;demo
 plugin.ignores =
 plugin.order = org.dita.base org.oasis-open.dita.v1_3 org.oasis-open.dita.v1_2
 registry = http://plugins.dita-ot.org/
+workspace = .


Refresh my memory: can configuration.properties live outside the DITA-OT directory? If not, you'd still have to "mutate" the installation to change the workspace, I think. Still, just mutating one file is much better than mutating a whole lot.

Would it be possible for DITA-OT to look for something like ~/.dita-ot.properties and merge the properties in that file with the contents of src/main/config/configuration.properties or something along those lines?

configuration.properties needs to be in the classpath, so the exact location is not important. See discussion on main comments.

eerohele · 2018-11-17T19:53:12Z

src/main/java/org/dita/dost/platform/FileGenerator.java

@@ -88,7 +108,7 @@ public void generate(final File fileName) {
 } catch (final RuntimeException e) {
 throw e;
 } catch (final Exception e) {
- logger.error("Failed to transform " + fileName + ": " + e.getMessage(), e);
+ throw new IOException("Failed to transform " + fileName + ": " + e.getMessage(), e);


Not sure why an exception instead of logging the error…?

If generating a file from a template fails, it's likely that the actual DITA-OT process will not work. So it's safer to fail to notify that this doesn't work, instead of logging and continuing.

src/main/java/org/dita/dost/platform/Integrator.java

eerohele · 2018-11-17T20:05:18Z

Left a couple of minor comments, but 👍 👍 & 🥇 for putting generated files outside the installation directory.

jelovirt · 2018-11-18T09:37:01Z

@eerohele OT has for a long time had support for local.properties file in the installation directory, but it's only read by Ant. The normal configuration which is available through the Configuration object's static fields, does not have access to Ant projects properties. My preference on OS X would be to add support for ~/.dita-ot.properties, because that would allow us to keep things outside the OT installation directory. However, it's not absolutely wrong to keep $DITA_HOME/local.properties, so maybe something like:

Read ~/dita-ot.properties
Read $DITA_HOME/local.properties
Read configuration.properties from classpath

where the first declaration is effective. That would give us

user specific configuration
installation specific configuration
built-in configuration.

eerohele · 2018-11-19T08:50:51Z

@jelovirt that sounds good to me. Alternatively (or additionally), you could maybe pass a property file as a command-line argument to dita.

I guess even with the way things currently stand, you could deliver a configuration.properties file in your plugin and add it to the DITA-OT classpath in your plugin.xml…?

One concern is that all of those property files have different names, but it's probably something we just have to live with because it's not worth breaking backwards compatibility.

jelovirt · 2018-11-19T15:17:23Z

Thinking about this aloud, we need to keep build.xml in the root of the installation directory, because we need the start point to exist somewhere. With dita command we could search for it in the workspace and then installation directory, but for traditional Ant use, it's easiest to keep it where it's been before.

In order to make the distribution directory read-only, maybe start build file should be just a proxy that imports or includes the generated build file that contains all the plugin imports. So right now the root build.xml has

<import file="plugins/org.dita.base/build_init.xml"/>
<import file="plugins/org.dita.base/preproess.xml"/>
...

What if the root build.xml only had

<import file="plugins/org.dita.base/start.xml"/>

and plugins/org.dita.base/start.xml had all those

<import file="plugins/org.dita.base/build_init.xml"/>
<import file="plugins/org.dita.base/preproess.xml"/>
...

This way start.xml could be generated and stored in the workspace, and build.xml would be read-only.

infotexture · 2018-11-19T15:28:58Z

👍 for the idea of adding support for something like ~/.dita-ot.properties that can be used to override DITA-OT default settings without modifying anything in the installation directory.

It would be good if the DITA-OT Java code would also read this file, so that it can be used to set things like custom registry entries that cannot currently be overridden with the local.properties file.

raducoravu · 2018-11-22T10:52:45Z

Maybe running something like “dita install ws=some\path” could copy all the resources in the current dita ot to the workspace folder and then run the integrator there. So the separate workspace would end up having an entire dita ot installation there. Is this already the idea or do you plan the custom workspace to contain a minimal set of resources, no jar libraries for example?

raducoravu · 2018-11-22T10:58:54Z

Without thinking on the implementation details my wish for this would be to have a separate plugins folder somewhere in the users home and running the integrator would not modify the existing dita ot folder resources. But publishing would be able to use the plugins.

stefan-jung · 2018-11-22T12:37:25Z

@raducoravu, this would be nice and would make it possible to provide Linux repositories. It would be probably very difficult to implement, especially if the machine is used in a multi-user environment and users use different plugins.

SMillerDev · 2018-12-17T13:12:10Z

Not really a comment on the code but OS X has been called macOS for a couple releases now 😄

jelovirt · 2018-12-30T15:21:48Z

This feature proves to be challenging to implement. Modules can be made to work nicely with external workspace, but Ant imports are a challenge.

jelovirt · 2020-06-06T07:04:47Z

Worked around the Ant import issue by using multirootfileset instead of fileset.

Signed-off-by: Jarno Elovirta <[email protected]>

jelovirt added feature New feature or request plugin-integration Issue related to plug-in installation / integration labels Nov 15, 2018

jelovirt self-assigned this Nov 15, 2018

jelovirt force-pushed the feature/plugin_fileset branch from 66c4ba3 to 259db8b Compare November 17, 2018 07:34

jelovirt force-pushed the feature/plugin_fileset branch 2 times, most recently from 2fca1e7 to cb00a21 Compare November 17, 2018 09:01

eerohele reviewed Nov 17, 2018

View reviewed changes

jelovirt added this to In progress in 3.3 via automation Dec 1, 2018

jelovirt force-pushed the feature/plugin_fileset branch from 35adc27 to 609cbbd Compare December 6, 2018 10:54

jelovirt changed the base branch from develop to feature/base_plugin December 6, 2018 10:54

jelovirt force-pushed the feature/base_plugin branch from 66efc69 to 401d275 Compare December 10, 2018 17:35

jelovirt force-pushed the feature/plugin_fileset branch from 3bc40a7 to f10af43 Compare December 12, 2018 15:34

jelovirt changed the base branch from feature/base_plugin to develop December 12, 2018 16:20

jelovirt force-pushed the feature/plugin_fileset branch from 110f1d6 to 9b13a65 Compare January 26, 2019 18:21

jelovirt force-pushed the feature/plugin_fileset branch from 9b13a65 to 4641872 Compare February 4, 2019 17:20

jelovirt removed this from In progress in 3.3 Feb 7, 2019

jelovirt marked this pull request as draft April 9, 2020 15:05

jelovirt added this to In progress in JAR distribution via automation May 16, 2020

jelovirt force-pushed the feature/plugin_fileset branch from 4641872 to 09328ad Compare June 4, 2020 18:25

jelovirt force-pushed the feature/plugin_fileset branch from fe557bc to a127c28 Compare June 17, 2020 18:34

jelovirt dismissed a stale review via 81fca3b August 2, 2021 09:37

jelovirt force-pushed the feature/plugin_fileset branch 2 times, most recently from 81fca3b to 02dcbf8 Compare August 5, 2021 12:15

Add initial support for configurable workspace

589b20b

Signed-off-by: Jarno Elovirta <[email protected]>

jelovirt force-pushed the feature/plugin_fileset branch from 02dcbf8 to 589b20b Compare November 12, 2021 20:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate workspace for files generated from plugins #3152

Separate workspace for files generated from plugins #3152

jelovirt commented Nov 15, 2018 •

edited

Loading

jelovirt commented Nov 15, 2018

raducoravu commented Nov 17, 2018

jelovirt commented Nov 17, 2018

eerohele Nov 17, 2018

eerohele Nov 17, 2018

jelovirt Nov 18, 2018

eerohele Nov 17, 2018

jelovirt Dec 13, 2018

eerohele commented Nov 17, 2018

jelovirt commented Nov 18, 2018

eerohele commented Nov 19, 2018

jelovirt commented Nov 19, 2018

infotexture commented Nov 19, 2018

raducoravu commented Nov 22, 2018

raducoravu commented Nov 22, 2018

stefan-jung commented Nov 22, 2018

SMillerDev commented Dec 17, 2018

jelovirt commented Dec 30, 2018

jelovirt commented Jun 6, 2020

Separate workspace for files generated from plugins #3152

Are you sure you want to change the base?

Separate workspace for files generated from plugins #3152

Conversation

jelovirt commented Nov 15, 2018 • edited Loading

jelovirt commented Nov 15, 2018

raducoravu commented Nov 17, 2018

jelovirt commented Nov 17, 2018

eerohele Nov 17, 2018

Choose a reason for hiding this comment

eerohele Nov 17, 2018

Choose a reason for hiding this comment

jelovirt Nov 18, 2018

Choose a reason for hiding this comment

eerohele Nov 17, 2018

Choose a reason for hiding this comment

jelovirt Dec 13, 2018

Choose a reason for hiding this comment

eerohele commented Nov 17, 2018

jelovirt commented Nov 18, 2018

eerohele commented Nov 19, 2018

jelovirt commented Nov 19, 2018

infotexture commented Nov 19, 2018

raducoravu commented Nov 22, 2018

raducoravu commented Nov 22, 2018

stefan-jung commented Nov 22, 2018

SMillerDev commented Dec 17, 2018

jelovirt commented Dec 30, 2018

jelovirt commented Jun 6, 2020

jelovirt commented Nov 15, 2018 •

edited

Loading