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

Add Windows support #122

Merged
merged 3 commits into from
May 9, 2019
Merged

Add Windows support #122

merged 3 commits into from
May 9, 2019

Conversation

iSoron
Copy link
Contributor

@iSoron iSoron commented May 8, 2019

The package currently fails to build on Windows with the message SCIP is currently not supported on "NT". This pull request fixes this by modifying deps/build.jl to search for the correct DLL. I have verified not only the package builds successfully with these changes, but also that all unit tests pass.

The function write_depsfile needed to be modified because, on Windows, the path variable contains backslashes which need to be quoted.

@codecov
Copy link

codecov bot commented May 8, 2019

Codecov Report

Merging #122 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #122   +/-   ##
=======================================
  Coverage   14.11%   14.11%           
=======================================
  Files         134      134           
  Lines        4251     4251           
=======================================
  Hits          600      600           
  Misses       3651     3651

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a5095f...e166c02. Read the comment docs.

@rschwarz
Copy link
Collaborator

rschwarz commented May 9, 2019

Cool, thanks!

It would be great if we could also set up appveyor to run automated tests on Windows.
I have not done that before, but could look into that later.

@matbesancon
Copy link
Member

Maybe we can add it during that PR right away (never done it either)

@matbesancon
Copy link
Member

The PR needs to add an appveyor file as in:
https://github.com/JuliaLang/Example.jl/blob/master/.appveyor.yml

@matbesancon
Copy link
Member

@rschwarz I think there are some custom steps because of the way SCIP is imported, you know them better than me

@rschwarz
Copy link
Collaborator

rschwarz commented May 9, 2019

Maybe we can add it during that PR right away.

Yes, that's what I meant.

I think there are some custom steps because of the way SCIP is imported

Yes, SCIP needs to be downloaded and installed. We also need to set the SCIPOPTDIR variable.

 - start with generic Julia file from Example.jl
 - add some SCIP-specific things from PySCIPOpt
@rschwarz
Copy link
Collaborator

rschwarz commented May 9, 2019

@fserra, @mattmilten I've tried to setup AppVeyor integration for SCIP.jl, by installing it to the SCIP-Interfaces group, but only to "selected projects", namely SCIP.jl. If I have disabled the integration for PySCIPOpt through this action (it does not show up in the settings), please let me know or change it back.

@mattmilten
Copy link
Collaborator

mattmilten commented May 9, 2019

I guess back when I added appveyor, I only added it for PySCIPOpt.

Edit: It seems I set it up using my personal account on appveyor/GitHub. I don't think that there will be a conflict

@rschwarz
Copy link
Collaborator

rschwarz commented May 9, 2019

I guess back when I added appveyor, I only added it for PySCIPOpt.

That's alright. My concern is that I might disabled it for PySCIPOpt inadvertantly.

@rschwarz rschwarz merged commit 61d3527 into scipopt:master May 9, 2019
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