Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overridable bash template #635

Closed
dvic opened this issue Aug 1, 2015 · 8 comments
Closed

Overridable bash template #635

dvic opened this issue Aug 1, 2015 · 8 comments

Comments

@dvic
Copy link

dvic commented Aug 1, 2015

I'm using the Play framework and I want to override the bash template. Is there currently a way to override the bash template without placing it under src/templates/bash-template?

It seems from this that it uses sourceDirectory to determine the base. I tried setting sourceDirectory in makeBashScript := baseDirectory.value / "app" and that doesn't work (as I expected). Play defines the following: sourceDirectory in Compile := baseDirectory.value / "app",. Setting sourceDirectory := baseDirectory.value / "app" works but then I get also other folders in the package.

@muuki88
Copy link
Contributor

muuki88 commented Aug 2, 2015

I'm afraid this is not possible without overriding the task that generates the bash script.
We could introduce a special setting, but I think this more cosmetic than actual functionality.

@dvic
Copy link
Author

dvic commented Aug 5, 2015

With overriding the task, you mean something like this?

@dvic
Copy link
Author

dvic commented Aug 5, 2015

I have this now (added to build.sbt) and it works (bash-template is located in the conf folder of Play). But if we would have defaultTemplateLocation as a setting, then it would be easier to override (because that's basically what I'm overriding in this function). Do you think this is useful?

{
  object Override extends JavaAppStartScript {
    val bashTemplate = "bash-template"

    def settings: Seq[Setting[_]] = Seq(
      makeBashScript <<= (baseDirectory, bashScriptDefines, target in Universal, executableScriptName, sourceDirectory) map makeCustomUniversalBinScript
    )

    import JavaAppPackaging.autoImport._

    def makeCustomUniversalBinScript(baseDir: File, defines: Seq[String], tmpDir: File, name: String, sourceDir: File): Option[File] =
      if (defines.isEmpty) None
      else {
        val defaultTemplateLocation = baseDir / "conf" / bashTemplate
        val defaultTemplateSource = getClass getResource bashTemplate

        val template = if (defaultTemplateLocation.exists)
          defaultTemplateLocation.toURI.toURL
        else defaultTemplateSource

        val scriptBits = JavaAppBashScript.generateScript(defines, template)
        val script = tmpDir / "tmp" / "bin" / name
        IO.write(script, scriptBits)
        // TODO - Better control over this!
        script.setExecutable(true)
        Some(script)
      }
  }
  Override.settings
}

@muuki88
Copy link
Contributor

muuki88 commented Aug 5, 2015

Hm. I'm not sure if this setting really makes sense as it's more a cosmetic thing. Would creating a src folder have cause any issues?

On the other hand, separating is not that of a big deal.

@muuki88
Copy link
Contributor

muuki88 commented Aug 5, 2015

And thanks for sharing your code snippet. A few notes/questions

  • why check if defines isEmpty?
  • getClass getResource won't work I think

@dvic
Copy link
Author

dvic commented Aug 6, 2015

If it's not a lot of hassle, I'd would rather add it, as it's nice to stick to the directory convention of the project you're using (in my case, Play, which doesn't have a src folder).

I am not sure why the check for defines.isEmpty exists, I copied that method straight from the source code (except for the line val defaultTemplateLocation = baseDir / "conf" / bashTemplate). Same goes for getClass getResource bashTemplate. The original method can be found here.

@muuki88
Copy link
Contributor

muuki88 commented Aug 6, 2015

You can change the play layout structure since 2.4 if that helps. I keep this issue then as a feature request. Happy to merge a pr from you ;)

I am not sure why the check for defines.isEmpty exists, I copied that method straight from the source code

That's weird. I have to look closer.

@muuki88
Copy link
Contributor

muuki88 commented Aug 10, 2015

Implemented in #646

@muuki88 muuki88 closed this as completed Aug 10, 2015
@muuki88 muuki88 mentioned this issue Aug 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants