kavedaa / sbt-javafx

JavaFX plugin for SBT
36 stars 6 forks source link

Package preparation hook #16

Closed metasim closed 9 years ago

metasim commented 10 years ago

I have some application specific customization I need to apply to the JavaFX packaging configuration that's outside the scope of the plugin (one such tweak is to work around a JavaFX packager bug in the WiX-based installer).

I'd like to be able to apply my customizations within the processing of the sbt-javafx workflow so I don't have to build things twice (which can take a while on Windows).

This pull request proposes one way of tweaking the plugin to expose the "pre-package" vs. "package" stages. It does this by separating the deployment artifact copying and build.xml generation into a separate task, upon which the Ant invocation task (package-javafx) depends upon.

Because the approach implemented exposes a new task, thereby exposing a new API, I suggest it be given more critical thought than normal. While I've read the "Plugin Best Practices" section of the SBT docs, I've not developed a plugin before and don't know if what I'm proposing is ultimately a good idea. It suffices for my immediate needs (I can now redefine the updated package-javafx task in my build.sbt file without reimplementing hard stuff), and I can use my fork until a final solution is deployed.

Generally speaking I think breaking the process up is a good idea, giving users additional flexibility they will likely need; it's the selection of an approach for accomplishing it that I want to flag for particular critique.

kavedaa commented 10 years ago

If I understand it correctly you want to hand-edit build.xml before it is processed with Ant.

One important thing here is to make sure that this feature is completely invisible to the casual user, they should not know that it even exists without digging into the "advanced" section of the documentation. (And I think that will be covered with your proposed solution.)

In general I think breaking it up into several tasks is the right way to go. But as you say you would need to redefine package-javafx for it to work. It would be nice be able to avoid that, so that you would have three tasks:

One that creates build.xml, one that runs Ant on it, and one (package-javafx) that runs these two tasks in the right order. Then you would be able to use the two first tasks independently. I think this would (or should) be possible to achieve.

Additionally, with this approach you would probably need to hand-edit build.xml every time you package the application? It would be nice to be able to automate this. A different solution might thus be to provide a setting that is a function from scala.xml.Elem to scala.xml.Elem, which the generated xml can be passed through before being written to file. In this function you could then have the necessesary rewriting logic for tweaking the xml.

Perhaps we could implement both solutions.

metasim commented 10 years ago

If I understand it correctly you want to hand-edit build.xml before it is processed with Ant.

Actually my plan (such that it is) is to override the preparePackageJavaFx task to do some additional modification on build.xml; something like this:

build.sbt:


JFX.preparePackageJavaFx := {
    val distFiles = JFX.preparePackageJavaFx.value

    ... // Apply additional transforms on `build.xml`, which is  `distFiles._2`.

    distFiles
}

One important thing here is to make sure that this feature is completely invisible to the casual user, they should not know that it even exists without digging into the "advanced" section of the documentation. (And I think that will be covered with your proposed solution.)

Agreed, and yes, current approach aims for that.

In general I think breaking it up into several tasks is the right way to go. But as you say you would need to redefine package-javafx for it to work. It would be nice be able to avoid that, so that you would have three tasks:

One that creates build.xml, one that runs Ant on it, and one (package-javafx) that runs these two tasks in the right order. Then you would be able to use the two first tasks independently. I think this would (or should) be possible to achieve.

I hadn't thought of that, but think it's a better approach.

Additionally, with this approach you would probably need to hand-edit build.xml every time you package the application?

Again, I'd assumed I'd use task redefinition to get what I wanted, but admit that may be a hack-ish approach. Not sure what the "idiomatic SBT way" is.

It would be nice to be able to automate this. A different solution might thus be to provide a setting that is a function from scala.xml.Elem to scala.xml.Elem, which the generated xml can be passed through before being written to file. In this function you could then have the necessary rewriting logic for tweaking the xml.

Would that (Elem) => (Elem) function be a setting that defaults to identity? Or a separate task?

I went back and forth on whether preparePackageJavaFx should return the build.xml as a File or scala.xml.Elem. Not sure why I went with the File, but now think Elem would be much more flexible.

Perhaps we could implement both solutions.

I think we can, and do so in a way that doesn't expose the mechanization to users who want the out-of-the-box experience.

Generally speaking, we kinda have three stages can can be chained/aggregated in two different ways. If the steps are:

  1. Copy distribution resources
  2. Generate default Ant build definition as DOM, optionally pass through transform function.
  3. Write build.xml and run Ant.

The two options are:

  1. Have three tasks, each dependent on the output of the previous, with the last task having the name package-javafx
  2. Have four tasks, with each of the three steps above defined as sub tasks to the main package-javafx being aggregating task, calling each in turn(?).

The latter seems less functional to me in some way, and I'm not sure how you deal with task dependencies. The former seems more natural, but again, my SBT experience is weak here. Thoughts?

I'm happy to give a stab at an update it if you're OK with that.

Simeon

kavedaa commented 10 years ago

Simeon H.K. Fitch wrote:

Actually my plan (such that it is) is to override the |preparePackageJavaFx| task to do some additional modification on |build.xml|; something like this:

Ok, in that scenario I think using a transformation function setting as described below would be the better solution, provided it solves your use case adequately.

Would that |(Elem) => (Elem)| function be a setting that defaults to |identity|? Or a separate task?

Yes, the former, something like:

val customizeXml = SettingKeyElem => Elem

And you'd use it like:

JFX.customizeXml := { xml => // rewrite it here xml }

And then in the plugin run the generated xml through that function before writing to file.

So conceptually it's quite similar to your task-based solution, but I think a bit simpler and intuitive. Given other users might want to use this, I think "provide a transformation function" (nothing SBT-specific) sounds less scary than "redefine a task" (WTF is a task???). User-friendliness above all. :)

Perhaps we could implement both solutions.

I think we can, and do so in a way that doesn't expose the mechanization to users who want the out-of-the-box experience.

The task-based approach would certainly be useful in cases where one might want to quickly edit build.xml before it is run.

  1. Have three tasks, each dependent on the output of the previous, with the last task having the name |package-javafx|
  2. Have four tasks, with each of the three steps above defined as sub tasks to the main |package-javafx| being aggregating task, calling each in turn(?).

The latter seems less functional to me in some way, and I'm not sure how you deal with task dependencies. The former seems more natural, but again, my SBT experience is weak here. Thoughts?

If we want to be able to run the latter tasks without also running any of the former, which I think is essential for the (edit build.xml) use case, we'd have to go with option 2. The open question is if we have a task D that depends on (A, B, C) whether we can be sure that it will execute them in order or not.

metasim commented 10 years ago

If we want to be able to run the latter tasks without also running any of the former, which I think is essential for the (edit build.xml) use case, we'd have to go with option 2. The open question is if we have a task D that depends on (A, B, C) whether we can be sure that it will execute them in order or not.

Actually I don't think that's the case: if a task is defined with an explicit dependency on another task (see this line in my pull request), then the transitive execution (or whatever you want to call it) is automatically handled by SBT. IOW, using the map et al. with other tasks, the dependency chaining comes for free.

But my point doesn't really matter. I think the transformation function approach is more elegant and easier to grok and is probably the better approach at this point. Working on a product release ATM, but will revisit this when the dust settles. I'll resubmit using that approach (or a hybrid) and see what you think.