Skip to content

Windows build fixes#8116

Merged
colinsurprenant merged 2 commits intoelastic:masterfrom
colinsurprenant:fix/windows_build
Sep 1, 2017
Merged

Windows build fixes#8116
colinsurprenant merged 2 commits intoelastic:masterfrom
colinsurprenant:fix/windows_build

Conversation

@colinsurprenant
Copy link
Copy Markdown
Contributor

@colinsurprenant colinsurprenant commented Aug 31, 2017

Modifications to have a green build on Windows.

Tested for successful rake test:core locally on Windows Server 2012 and on OSX.

These are mostly spec fixes.

The only non-spec significant change which should prevent potential problems on all platforms is in logstash-core/lib/logstash/pipeline_action/stop.rb where after issuing a pipeline shutdown, we were not waiting on the pipeline thread to complete.

@jordansissel
Copy link
Copy Markdown
Contributor

I will test this shortly.

@jordansissel jordansissel self-requested a review August 31, 2017 18:11
@jordansissel
Copy link
Copy Markdown
Contributor

Finished in 11 minutes 36 seconds (files took 8.31 seconds to load)
2328 examples, 0 failures, 3 pending

Copy link
Copy Markdown
Contributor

@jordansissel jordansissel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested on my home workstation and tests pass.

Code LGTM.

"total_in_millis"=>Numeric,
"percent"=>Numeric,
"load_average" => { "1m" => Numeric }
# load_average is not supported on Windows, set it below
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed manually that on Elasticsearch (I used ES 5.5.0) only has load_average on Linux. On Windows, the cpu object only has percent.

@jordansissel
Copy link
Copy Markdown
Contributor

@colinsurprenant Thanks for solving these issues!

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
@colinsurprenant
Copy link
Copy Markdown
Contributor Author

I decided to rebase into 2 separate commits, one for the specific tests specs fixes and the other for the pipeline stop action fix which for some reason only surfaced in windows tests but is definitely a potential problem everywhere.

@colinsurprenant colinsurprenant merged commit 21dbde2 into elastic:master Sep 1, 2017
@colinsurprenant
Copy link
Copy Markdown
Contributor Author

merged in master, 6.x and 6.0

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.

2 participants