It appears I wanted “git fetch upstream pull/61/head:Try61”.
And to the extent that command looks right to you (it seemed to “do stuff”), I’m sad to say that I still fail the tailer test.
[ERROR] Failures:
[ERROR] TailerTest.testTailer:95 expected:<…ailer by TailerTest.[
It has several lines.]> but was:<…ailer by TailerTest.[
It has several lines.
]>
Test set: VASSAL.tools.io.TailerTest
Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.011 s <<< FAILURE! - in VASSAL.tools.io.TailerTest
VASSAL.tools.io.TailerTest.testTailer Time elapsed: 1.006 s <<< FAILURE!
org.junit.ComparisonFailure:
expected:<…ailer by TailerTest.[
It has several lines.]> but was:<…ailer by TailerTest.[
It has several lines.
]>
at VASSAL.tools.io.TailerTest.testTailer(TailerTest.java:95)
If I want to try out PR#61 on my system, what’s the git command line
stuff for that?
I was trying things like “git fetch origin pull/61/head:Try61” but no
luck
origin for you is going to be your repo on github (unless you set it to
something else). You need a remote for the upstream repo in order to do
this, because the remote where the PR exists is that one.
We could make an appender that sends the log messages over the network socket to the parent process, the ModuleManager, the ModuleManager would combine them and send to all Player instances, and they would show the combined logs in their dialog panels.
I think I’ll rather work on getting rid of the JVM fork
What was the use case again for having several Player instances run at the same time, that can’t be solved by the user starting several Vassal instances at the same time?
About the bin/ in .gitignore, it’s commonly added for Eclipse, e.g. the “gitignore generator” under toptal.com/developers/gitignore when asked for Eclipse spits out bin/ as part of the generated .gitignore.
We can avoid adding it for now but the bin/ directory might keep coming back for Eclipse users, if it does we should probably add it sometime.
About the bin/ in .gitignore, it’s commonly added for Eclipse, e.g. the
“gitignore generator” under toptal.com/developers/gitignore[1] when asked for Eclipse
spits out bin/ as part of the generated .gitignore.
We can avoid adding it for now but the bin/ directory might keep coming
back for Eclipse users, if it does we should probably add it sometime.
INFO] Running VASSAL.tools.io.TailerTest
[INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.004 s - in VASSAL.tools.io.TailerTest
This is using Eclipse on Windows 10. Thanks Brian for the heads up on resolving the pom.xml error. Apart from the expected loads of Checkstyle errors, I also get the following errors highlighted in red at the top of the maven console output:
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/C:/Users/beast/.p2/pool/plugins/org.eclipse.m2e.maven.runtime.slf4j.simple_1.16.0.20200610-1735/jars/slf4j-simple-1.7.5.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [file:/J:/Data/Eclipse/eclipse/configuration/org.eclipse.osgi/599/0/.cp/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [org.slf4j.impl.SimpleLoggerFactory]
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/C:/Users/beast/.p2/pool/plugins/org.eclipse.m2e.maven.runtime.slf4j.simple_1.16.0.20200610-1735/jars/slf4j-simple-1.7.5.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [file:/J:/Data/Eclipse/eclipse/configuration/org.eclipse.osgi/599/0/.cp/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [org.slf4j.impl.SimpleLoggerFactory]
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/C:/Users/beast/.p2/pool/plugins/org.eclipse.m2e.maven.runtime.slf4j.simple_1.16.0.20200610-1735/jars/slf4j-simple-1.7.5.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [file:/J:/Data/Eclipse/eclipse/configuration/org.eclipse.osgi/599/0/.cp/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [org.slf4j.impl.SimpleLoggerFactory]
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/C:/Users/beast/.p2/pool/plugins/org.eclipse.m2e.maven.runtime.slf4j.simple_1.16.0.20200610-1735/jars/slf4j-simple-1.7.5.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [file:/J:/Data/Eclipse/eclipse/configuration/org.eclipse.osgi/599/0/.cp/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [org.slf4j.impl.SimpleLoggerFactory]
Eclipse classes clashing with m2e plugin classes. Ignore it, it’s not our fight. Eclipse and maven never were good friends.
I do not find any place in Tailer or TailerTest where newline translation should be happening. RandomAccessFile.readLine() should strip LF or CRLF, but the input file has no CR in it anyway. We append “\n” to the lines after stripping them. I don’t see in the code for IOUtils.toString where any CR would be introduced.
Brian, would you mind adjusting that test so that it prints out the two values being compared in their entirety?
Found it. Here you go. TL;DR: expected has CR/LF and actual is just LF
Btw I’m running Windows 10 if it matters.
Expected: This is a file for testing Tailer by TailerTest.
It has several lines.
Actual: This is a file for testing Tailer by TailerTest.
It has several lines.
What is very weird is that it works for Brent on Windows but doesn’t work for Brian on Windows.
Is the source of the error and a way to solve this possibly not our Java code at all but the way git handles line endings, i.e. “git config core.autocrlf”?
Brent, Brian, what is your git’s local setting for “core.autocrlf”, and what line endings does the text file for the TailerTest have on your machines? I suspect that your git setting and the line endings in the file are different from each other, one of you probably has CRLF and the other just LF.
Found it. Here you go. TL;DR: expected has CR/LF and actual is just LF
That is surprising. There’s nothing in the documentation of for
IOUtils.toString() suggesting that newline translation will occur, nor
did I spot it in the code I read for that.
As of Java 11, we can use Files.readString() for reading an entire file
to a string; the documentation for that says “The resulting string will
contain line separators as they appear in the file.” I’ve pushed a change
to use that instead.
What is very weird is that it works for Brent on Windows but doesn’t
work for Brian on Windows.
Is the source of the error and a way to solve this possibly not our Java
code at all but the way git handles line endings, i.e. “git config
core.autocrlf”?
Brent, Brian, what is your git’s local setting for “core.autocrlf”, and
what line endings does the text file for the TailerTest have on your
machines? I suspect that your git setting and the line endings in the
file are different from each other, one of you probably has CRLF and the
other just LF.
This is a setting which is super helpful for messing up files for digital
forensics, but it generally only happens to people once when they’re
getting set up for the first time so then I forget it exists.
As the problem isn’t happening for Brent on Windows, it’s a good guess
for what’s wrong.
Try the change I pushed before adjusting the git setting, though; I’d
like to see if it makes a difference against the original state of things.
Yes please don’t adjust anything for now, if it needs adjusted we will need to decide first how to adjust it exactly, in a consistent way. This per-repository settings file they write about in that guide on GitHub sounds good though.
Since you’re working on Windows with others who are not, having it set to
true is almost certainly the right thing.
What I think is happening is that git is converting the LF to CRLF in the
test file when you check it out, because of your core.autocrlf setting
and the fact that the test file is a text file. (This wouldn’t happen for
a binary file.)
So… I think what I’ll do is the simplest thing which could possibly
work, which is to convert the CRLF back to LF in the test case.
