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

cygwin support #167

Merged
merged 5 commits into from
Oct 8, 2016
Merged

cygwin support #167

merged 5 commits into from
Oct 8, 2016

Conversation

pawelpanasewicz
Copy link
Contributor

Anything against this? Now I can run it as well from windows.

@paulp
Copy link
Collaborator

paulp commented Oct 5, 2016

Does travis offer any way to test it?

@xuwei-k
Copy link
Contributor

xuwei-k commented Oct 5, 2016

Maybe appveyor support windows and cygwin.

https://www.appveyor.com/docs/installed-software/#mingw-msys-cygwin

@pawelpanasewicz
Copy link
Contributor Author

Hi!
Can we test it?

Automated testing after each commit in CI requires some work and decisions.

Firstly - bats must be able to run under cygwin in order to run sbt-extras' tests which will prove that it behaves nicely under cygwin.

There is some work done (in LeahCim/bats fork) towards this direction, but it's not merged so far. LeahCim/bats is developed practically on top of master of original sstephenson/bats (it does not contain changes from particular text file). On the other hand LeahCim/bats is not fully compatible but maybe it's enough to test sbt-extras and it's worth to use it for price of having tested cygwin.
See PR128.

Secondly - it has to be run on agent which supports cygwin. As @xuwei-k notices approyer is one possible choice.

Let me do some research on "firstly" thing.
What is your opinion?

@pawelpanasewicz
Copy link
Contributor Author

I am almost there.

This is build log from appveyor which runs test under cygwin on windows.
https://ci.appveyor.com/project/pawelpanasewicz/sbt-extras/build/1.0.13

Tests are mainly failing with assertion messages very similar to this one:

# expected:
# Downloading sbt launcher for 0.12.2:
#   From  http://repo.typesafe.com/typesafe/ivy-releases/org.scala-sbt/sbt-launch/0.12.2/sbt-launch.jar
#     To  $ROOT/.sbt/launchers/0.12.2/sbt-launch.jar
# 
# actual:
# Downloading sbt launcher for 0.12.2:
#   From  http://repo.typesafe.com/typesafe/ivy-releases/org.scala-sbt/sbt-launch/0.12.2/sbt-launch.jar
#     To  C:\cygwin64\tmp\download.bats.744\.sbt\launchers\0.12.2\sbt-launch.jar

As one can see only prefix of To some_path are different.

This is because normalize_paths doesn't normalize windows like paths. This is code of normalize_paths:

normalize_paths () {
  stdin_or_args "$@" | \
    sed "s:$TEST_ROOT:\$ROOT:g" | \
    sed "s:$HOME:\$ROOT:g"
}

Windows like path in this example is C:\cygwin64\tmp\download.bats.744\.sbt\launchers\0.12.2\sbt-launch.jar. We need to normalize it into $ROOT/.sbt/launchers/0.12.2/sbt-launch.jar.
How to do this?
The command cygpath -w $TEST_ROOT called from scope of mentioned function can help. It returns wanted prefix of windows style path, in this example it is C:\cygwin64\tmp\download.bats.744.

Now it is sufice to make some replacement in normalize_paths and it should work.

The tricky part for me is that I don't know how to use sed in order to replace this from input string.

Can someone help?

@paulp
Copy link
Collaborator

paulp commented Oct 6, 2016

@pawelpanasewicz If I understand correctly having barely skimmed and running out the door, this might do it. Replace sed "s:$TEST_ROOT:\$ROOT:g" with

sed "s:$(cygpath -w "$TEST_ROOT"):\$ROOT:g"

@pawelpanasewicz
Copy link
Contributor Author

pawelpanasewicz commented Oct 6, 2016

@paulp Thanks! I've manage to run it.

Unfortunately tests are not passing. At first glance outputs are the same. I did some CRLF => LF convertion hoping it will solve the problem but it didn't helped. Not sure what can cause problems. I'll think on it.

https://ci.appveyor.com/project/pawelpanasewicz/sbt-extras/build/1.0.22

Briefly some test failures look like this:

not ok 2 tolerates blank lines and comments in jvm_opts file
# (in test file test/config-file.bats, line 33)
#   `expectedOutput | assert_output' failed
# /cygdrive/c/sbt-extras/test/test_helper.bash: line 131: warning: add_process: pid  2080 (stdin_or_args "$@") marked as still alive
# 
# expected:
# -Xmx1g
# -Dsome.prop="pass a # in a string"
# -Xss4m
# -Xms1g
# -jar
# $ROOT/.sbt/launchers/0.13.12/sbt-launch.jar
# about
# 
# actual:
# -Xmx1g
# -Dsome.prop="pass a # in a string"
# -Xss4m
# -Xms1g
# -jar
# $ROOT/.sbt/launchers/0.13.12/sbt-launch.jar
# about
# 

@pawelpanasewicz
Copy link
Contributor Author

It looks like we have sbt-extras working under both linux and cygwin.
Travis and Appveyor managed to test it successfully.

https://ci.appveyor.com/project/pawelpanasewicz/sbt-extras/build/1.0.38
https://travis-ci.org/jawp/sbt-extras/builds/165897944

@paulp
Copy link
Collaborator

paulp commented Oct 8, 2016

@pawelpanasewicz Great work! It restores a little of my faith in humanity to see a contribution of this kind. Probably I have to flip a switch somewhere to enable appveyor tests?

@paulp
Copy link
Collaborator

paulp commented Oct 8, 2016

Okay, I think I have appveyor set up, but the easiest way to test that is to merge this so that's what I'm doing.

@paulp paulp merged commit bb9fb6d into dwijnand:master Oct 8, 2016
@dwijnand
Copy link
Owner

dwijnand commented Oct 8, 2016

Nice one @pawelpanasewicz!

@pawelpanasewicz
Copy link
Contributor Author

Thanks guys! Nice to read such words!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants