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

Windows build fixes for 5.6 #8125

Merged
merged 2 commits into from
Sep 20, 2017

Conversation

colinsurprenant
Copy link
Contributor

@colinsurprenant colinsurprenant commented Sep 1, 2017

Backport of #8116 for 5.6 and added 5.6 windows specific fixes.

Included here is the addition of -Dfile.encoding=UTF-8 in the JVM_OPTS in setup.bat as a stop-gap until #8126 is fixed.

@jakelandis
Copy link
Contributor

@colinsurprenant - sorry for the delay. Was able to run to success locally. Great job tracking these down.

LGTM

image

@jakelandis jakelandis self-requested a review September 15, 2017 18:01
@colinsurprenant
Copy link
Contributor Author

colinsurprenant commented Sep 18, 2017

rebased to ignore the UTF-8 commit since this was already patched by the new launch scripts PR #8178

@colinsurprenant
Copy link
Contributor Author

I rebased against 5.6 and hopefully the build will pass - the last build had problems and I am not sure how much was related if any.

@jakelandis
Copy link
Contributor

@colinsurprenant - the new Jenkins Docker builds aren't quite stable yet, and may get an artificial failure.

However, these Windows tests aren't active yet in the Jenkins build. See the checkbox item "Fix false positive success from ci\ci_test.bat" #7767 (The file is acutally ci\unit_tests.bat now). I am pretty certain that only the Java unit tests are actually running. We will need to fix the long file name issue first before we can enable these tests in ci.

@colinsurprenant
Copy link
Contributor Author

@jakelandis yeah I know - but I do want to make sure none of my changes have negatively impacted the Linux builds too so that's why I am hoping for a green build.

@colinsurprenant
Copy link
Contributor Author

waiting to see if rebasing to include #8339 will help this build.

@colinsurprenant
Copy link
Contributor Author

ok, only failure is the integration test failures on LogStash::Instrument::MetricStore::MetricNotFound
Not sure if this is related.

@colinsurprenant
Copy link
Contributor Author

WDYT @jakelandis @ph ? ^^

@ph
Copy link
Contributor

ph commented Sep 20, 2017

I've looked at the job logs and I can see a lot of failures related to main, its like the pipeline was never started. I would rekick it to see if its a transient error.

@jakelandis
Copy link
Contributor

@colinsurprenant - I think that is a transient error do to an overloaded host. I kicked it off again in Travis to be sure. FWIW, the same tests are run in Jenkins are passing.

@colinsurprenant
Copy link
Contributor Author

WOOT thanks @jakelandis @ph - will now merge. 😅

use rm_rf to delete dir and shutdown pipeline after run

avoid the use of rescue nil, bad practice

do not mock close as it prevents closing the file an prevents removing it on Windows

cleanup temporary files and relax file name check for Windows

fix paths handling for Windows

flag the read file string as UTF-8 which is what we expect

fix Windows issues

ignore load_average on windows

windows safe URI

cleanup and fix file handling for windows

wait for pipeline shutdown to complete

revert to original puts

cleanups

use environment for windows platform check

fix hash path

wait for pipeline thread to complete after shutdown
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants