sbt-native-packager
sbt-native-packager copied to clipboard
AshScriptPlugin generates unnecessary warnings for undefined environment variables
Expected behaviour
When building packages using the AshScriptPlugin and the DockerPlugin
Running docker-compose up
Should generate no warnings or errors from the shell
Actual behaviour
Running `docker-compose up' Actually generates
fooservice_1 | bin/fooservice: line 45: is_cygwin: not found
fooservice_1 | bin/fooservice: line 45: addJava: not found
Information
- What sbt-native-packager are you using :
1.1.6
- What sbt version :
0.13.15
- What is your build system (e.g. Ubuntu, MacOS, Windows, Debian ) :
macOS 10.12.4
- What package are you building (e.g. docker, rpm, ...) :
docker
- What version has your build tool (find out with e.g.
rpm --version
) :Docker version 17.03.1-ce, build c6d412e
- What is your target system (e.g. Ubuntu 16.04, CentOS 7) :
Alpine Linux on docker
Thanks for your report @AesaKamar
I wonder where these variables are being referenced. The ash-template doesn't use them.
Can you post the content of bin/fooservice
here?
@AesaKamar I updated the test-project-docker
to reproduce this, but without luck. Can you provide a small example to demonstrate this?
To test with my setup do the following
- checkout the current
v1.1.x
branch withgit checkout v1.1.x
-
cd test-project-docker
-
sbt docker:publishLocal
-
docker-compose up
I don't see any warnings. I'm building the image on Ubuntu 17.04
with Docker version 17.04.0-ce, build 4845c56
I'm on the same project as Aesa but I'm also not seeing these functions referenced in the ash template https://github.com/sbt/sbt-native-packager/blob/master/src/main/resources/com/typesafe/sbt/packager/archetypes/scripts/ash-template
some extra info, we're using these plugins https://github.com/cakesolutions/sbt-cake/tree/v1.0.4/src/main/scala/net/cakesolutions
of most relevance are
- https://github.com/cakesolutions/sbt-cake/blob/v1.0.4/src/main/scala/net/cakesolutions/CakeDockerPlugin.scala
- https://github.com/cakesolutions/sbt-cake/blob/v1.0.4/src/main/scala/net/cakesolutions/CakeJavaAppPlugin.scala
oh, hold on, addJava
is definitely in this script
but not in v1.1.6... hmm https://github.com/sbt/sbt-native-packager/blob/v1.1.6/src/main/resources/com/typesafe/sbt/packager/archetypes/ash-template
upgrading to 1.2.0 we no longer see the addJava
warning, but this still exists
myproject_1 | bin/myproject: line 56: is_cygwin: not found
which is weird, because line 56 is https://github.com/sbt/sbt-native-packager/blob/v1.2.0/src/main/resources/com/typesafe/sbt/packager/archetypes/scripts/ash-template#L56
[ -f "$script_conf_file" ] && opts=$(loadConfigFile "$script_conf_file")
aaah, but the generated file, line 56 looks like this
addJava "-Duser.dir=$(realpath "$(cd "${app_home}/.."; pwd -P)" $(is_cygwin && echo "fix"))"
which looks like it is coming from ${{template_declares}}
show bashScriptDefines
AHA! There it is. Also in bashScriptExtraDefines
my workaround is to set this in projectSettings
bashScriptExtraDefines := List(
"""addJava "-Duser.dir=$(realpath "$(cd "${app_home}/.."; pwd -P)")""""
)
or defining is_cygwin
to return false. Probably the best way to fix this in core would be to add the is_cygwin
script to this template.
Thanks for the digging @fommil ❤️
Where does this line come from?
addJava "-Duser.dir=$(realpath "$(cd "${app_home}/.."; pwd -P)" $(is_cygwin && echo "fix"))"
I couldn't find this in native-packer nor in the sbt-cake code base yet. I have searched von is_cygwin
and only found the bash-template and bash-forwarder scripts.
Hmm, I had assumed native-packager was adding it. Is it generated somewhere? We're not adding it. Spooky.
Very spooky. I tried to find the echo "fix"
line as well without any luck.
I wonder if another plugin is adding it with reflection magic.
I'm seeing it coming from https://github.com/playframework/playframework/blob/a4d9414e13ba46852e11d1cbf046703921d175bf/framework/src/sbt-plugin/src/main/scala/play/sbt/PlaySettings.scala#L223
well I'm not sure but bashScriptExtraDefines
should probably only add something for bash scripts - I mean it should not add anything to the ash script at all?
@schmitch it actually does. We reused this setting. The distinction is currently between "Unix" and "windows" start scripts not between various shell implementations.
Hi,
As @fommil said:
Probably the best way to fix this in core would be to add the is_cygwin script to this template.
Would be a solution for the issue or the is_cygwin
has bash-specific code that is not compatible with ash?
Thanks for taking an initiative again on this one @glammers1 :hugs: . I have no experience with ash
whatsoever, but we do have some docker integration tests, so feel free to create a pull request and we see what happens :smile:
Hi, I have some questions about the possible solutions. The problem is not just adding the is_cygwin
function to ash-template, the question is whether we should add all the functions and uses related to cygwin to ash-template as in bash-template are. For example, the function realpath
is aware of cygwin in bash-template. In fact, its use in playframework is to invoke realpath
where the second parameter indicates if we are using cygwin (result of the function is_cygwin
). It would make no sense to use realpath
with its second parameter "CHECK_CYGWIN != empty" and not change cygwin style paths to windows style paths as it would happen in ash-template.
The two possible solutions I have thought of are:
1 - ash-template is not designed to use with cygwin (docs should be updated). The solution would be add the following:
# ash-template is not designed to use with cygwin
is_cygwin() {
return 1
}
2 - ash-template is adapted to use with cygwin. It involves adding everything related to cygwin from bash-template to ash-template.
TBH, I don't know which solution would be the right one or if there are others that I have not thought.
I'll heavily favor solution 1. Even I have neither used cygwin nor ash from my understanding
-
cygwin
provides linux tools on windows. If you are building a docker image and want to use ash, you explicitly don't want to usecygwin
- is there a working windows docker image so that cygwin would actually be working in a docker image?
- windows 10 including bash and finally shipping an SSH client will probably shift cygwin out
@muuki88 5 years later... I agree probably just adding a is_cygwin
function to the ash template returning 1 would probably be the sanest thing to do. If you are still OK with that I would provide a pull request.
Go for it ❤️❤️
@muuki88 See
- #1590
if you could take a look this would be nice.
Thanks for merging @muuki88, this issue is now resolved and can be closed.