Attempt to resolve issue 161 - Add human readable methods to map directories

[ivanfrain]

I added two helper methods to simplify directory mappings.

One can add a directory by writing

mappings in Universal ++= directory("MyDir")

One can add a directory content by writing

mappings in Universal ++= contentOf("MyDir")

(Hope this one is better than my first pull request)

[lightbend-cla-validator]

Hi @ivanfrain,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://typesafe.com/contribute/cla

[jsuereth]

Nice. these are definitely the most frequently used.

[jsuereth]

Look cool to me. @muuki88 or @aparkinson ?

[muuki88]

This only works for static files, which are already present. I think an example which depends on a task is neccessary, like

mappings in Universal <++= (packageBin in Compile, target) map { (_, target) =>
   contentOf(target / "somefolder")
}

[ivanfrain]

Ok. This explains why I was not able to use the targetfrom sbt Keys like this:

mapping in Universal ++= directory(target / "scala-2.10" / "api")

The above snippet does not compile due to bad type java.io.File. Using target... requires a kind of task scope.

I'll add it to the doc !

[ivanfrain]

Before adding it to the documentation, I did test this and unfortunately, it does not compile. might be a type problem I do not understand yet. I have put the following in my build.sbt :

mappings in Universal <++= (packageBin in Compile, target) map { (_, target) =>
  directory(target / "streams")
}

And loading the project, I have obtained the following error:

> reload
[info] Loading project definition from /Users/ivan/****
/Users/ivan/****/build.sbt:49: error: value / is not a member of java.io.File
  directory(target / "streams")
                   ^
[error] Type error in expression

Do you have any idea about this problem ?

[jsuereth]

sbt 0.13:

mappings in Universal ++= {
  val ensureRun = (packageBin in Compile).value
  directory(target.value / "streams")
}

What I think you're running into above is some shadowing flipping out the implicits. Try to use a different name for target:

mappings in Universal <++= (packageBin in Compile, target) map { (_, t) =>
  directory(t / "streams")
}

Also +1 on an example which depends on a task.

[muuki88]

Ah damn, sorry for the confusion. Didn't test it either, just quickly wrote it.

[muuki88]

Thanks very much! This looks pretty good. Small tweaks on the docs and we can merge this

[ivanfrain]

You are welcome and thank you for the review.

[muuki88]

Would you like to add the additional docs in this pr? If not, we merge this and add the in a follow up pr (even though it would be nice every to have everything in one place and you get the credits ;) )

[lightbend-cla-validator]

Hi @ivanfrain,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://typesafe.com/contribute/cla

[ivanfrain]

@muuki88 I added the documentation about the mapping which depends on task. Tell me if you are ok with that.

I tested the different mappings mentionned in the Universal documentation and I was not able to make them work because of the "target" problem I mentionned above. I think opening an issue on that to understand what's happening could be good. On that point, I also tested wwhat you have proposed @jsuereth, due to implicit problem ...Thus I have tried with s/target/t and it did not work either.

[jsuereth]

Hmm, I'll have to look into this target problem Was thing in sbt 0.12.4?

[ivanfrain]

It was with sbt 0.13.1 on MacOSX. Let me know if you need other information to reproduce the problem.

[muuki88]

Awesome. @jsuereth , I think we are fine with merging and raising another issue with "target" problem referencing this one?

[jsuereth]

sounds good. the target issue seems like an issue with the 0.13 macros. I bet if you try it in 0.12.4 it's fine. If it's still an issue, we should open it up against sbt 0.13

[ivanfrain]

Very Good. I am ok too, I mean I have finished for this pull request.

I would like two propose two new features/enhancements (sorry if this is not the right place for discussing this). I'll be able to implement them as well. Is it ok to open issues for :

1. add the ability to choose the destination directory in helper method for directory mappings.

   def directory(source:File, targetName: String): Seq[(File, String)]
   def contentOf(source:File, targetName: String): Seq[(File, String)]

2. Add a way to add to scriptClasspath directories outside of lib directory. I was not able to do that using following snippet

   scriptClasspath += "conf"

   This adds lib/confto script classapth and not conf at the package root directory

[jsuereth]

You should open tickets for 1 and 2. ALso, for 2 - You probably want "../conf", although I don't recommend adding user-writable directories to your executable classpath [security].... (even though that's how typesafe config works cough @havocp cough). You should open a ticket in either case. I hoped help scriptClasspath explains how it's relative to the lib dir, but perhaps it doesn't....

[havocp]

I don't understand enough background here to know what the security issue is or what Typesafe config change you are proposing. You can parse any file you want with the config lib. Presumably you could also put your config files in a non writable directory - if you don't want it to be writable then don't make it writable? Is the point that you want a writable config but don't allow adding jars to classpath? It's not secure to have a writable default config either though; both play and Akka for example let you set all kinds of sensitive stuff in there. So I don't understand the scenario where you'd want the config writable but not the classpath (for the default classpath-based config anyway - if you wanted a user writable config you'd want it to be a special sandboxed one and then you could read it from where you want instead of from classpath). Classpath config is just the default global one that affects all libs in the jar.

[muuki88]

I like 1. A few days ago I would have liked 2 as well, however I gave it a thought.

Adding a conf folder to the classpath is an easy way, as most of the libraries scan the classpath for a specific file. However, the good libraries also provide to specify a system property where to find their configuration files. E.g. Typesafe Config and Log4j.

I put together an example here to demonstrate how you can easily and safely use config files from your application.

@jsuereth I will give this project a little love. Maybe we can transfer it to the sbt organization at some point.

[ivanfrain]

@muuki88 Thanks I'll have a look at your project.

[lightbend-cla-validator]

Hi @ivanfrain,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://typesafe.com/contribute/cla

[ivanfrain]

I finally add another update since the mapping with a directory at the root of a project was not working. In such case getParentFile returns null and thus prevent from running the relativeTo method correctly.

From my point of view, ready for merging.

[ivanfrain]

By the way I have signed the Typesafe contributor CLA 5 minutes ago.