ARROW-2436: [Rust] Add windows CI #1949
Conversation
|
@xhochy some of the travis builds are failing but looking at other PR's it seems that they are failing because the integration tests are being triggered (i.e. other PR's don't fail because they don't trigger the integration tests). I don't believe I have changed anything that should impact the Node.js and openjdk8 builds. Can you confirm that these integration tests normally pass? |
|
@paddyhoran We're having problems with |
appveyor.yml
Outdated
There was a problem hiding this comment.
Does clcache interfere with the Rust build?
There was a problem hiding this comment.
It's just for line 94 where clcache is called (this fails). I guess I could have used NOT "%Rust_Stable%" but the presence of the USE_CLCACHE variable made me think that there might be a valid use case to turn it off for non Rust builds
There was a problem hiding this comment.
It's fine, I just wanted to know.
appveyor.yml
Outdated
There was a problem hiding this comment.
Why the "%JOB%" to "%BUILD_SCRIPT%" renaming? This looks a bit arbitrary.
There was a problem hiding this comment.
The NOT "%JOB%"=="Cmake_Script_Tests" needs to be changed and from what I have read online the best way to do an AND is to use nested if statements. However, is seems AppVeyor does not play nice with multi-line if statements.
There was a problem hiding this comment.
If the problem is the impossibility of multi-line statements, we should move all this to dedicated scripts (e.g. ci/appveyor-install.bat and ci/appveyor-build.bat).
There was a problem hiding this comment.
Ok, will do. Thanks
appveyor.yml
Outdated
There was a problem hiding this comment.
Do you know if it's possible to have multi-line IF blocks instead?
appveyor.yml
Outdated
There was a problem hiding this comment.
At some point we will want to selectively disable some matrix builds based on the modified files, as we already do for Travis-CI. The ci/travis_detect_changes.py script does that, it can be extended for AppVeyor. The following logic (from CPython) is supposed to be able to detect changed files on AppVeyor PRs:
https://github.com/python/cpython/blob/master/.github/appveyor.yml#L12-L26
Builds already pile up quickly due to concurrency limitations on AppVeyor, ideally we should not make things worse.
There was a problem hiding this comment.
Yea, it's pretty annoying that all cpp builds would re-trigger on changes to the PR. I'll create a JIRA for it. It's probably something I could take a look at.
- Created appveyor-install.bat - Created appveyor-build.bat - Added rust-msvc-build.bat into appveyor-build.bat - Updated appveyor.yml to use the above
|
@pitrou I have re-configured the scripts as you suggested. However, I have some trivial issue that is causing an error. I am polluting the build queue trying to figure out why it is not working. If you have a chance could you take a quick look? You might spot something straight away that I am doing wrong... If it looks like it should work I'll keep researching... Thanks |
493c77c to
c6723d3
Compare
`clcache` is not being found. Checking to see if this is the reason.
|
Congrats for sorting this out :-) Is this PR basically ready now? |
|
Hey @pitrou. Yea, I think this is ready. The only thing I would like to clean up is this (in appveyor-install.bat): For some reason the following is causing an error: Another oddity of windows .bat scripting... For now I'd like to get this merged. I'll circle back to this at some point when I have time to research it more and I'll clean it up. Thanks very much for all your help! |
|
Thanks @paddyhoran ! |
No description provided.