Vs2008 appveyor cmake #617
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| echo %CD% | ||
| mkdir %PREFIX%\Scripts | ||
| MOVE %SRC_DIR%\setup_x64.bat %PREFIX%\Scripts\ | ||
| MOVE %SRC_DIR%\x64 %PREFIX%\Scripts\ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package: | ||
| name: vs2008_express_vc_python_patch | ||
| version: 1.0 | ||
|
|
||
| source: | ||
| url: https://github.com/menpo/condaci/raw/master/vs2008_patch.zip | ||
| sha256: c54143587f74a120f77987ea9a2d011e1c090709f07e3678f25aa1ffb93a6777 | ||
|
|
||
| build: | ||
| number: 0 | ||
| skip: true # [not win] | ||
|
|
||
| test: | ||
| commands: | ||
| - IF NOT EXIST %SCRIPTS%\setup_x64.bat exit 1 | ||
| - IF NOT EXIST %SCRIPTS%\x64 exit 1 | ||
|
|
||
| about: | ||
| summary: Workaround to help CMake find the appropriate x64 compiler on VS 2008 Express with VS compiler for python on top. | ||
|
There was a problem hiding this comment. I guess There was a problem hiding this comment. Please pin to v0.4.8 as I am deleting that file in condaci. As a note, you don't really need it for MSVC - it only matters with all the Intel architectures (Itanium etc). You just need to copy the one file. |
||
|
|
||
| extra: | ||
| recipe-maintainers: | ||
| - msarahan | ||
| - patricksnape | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| call %PREFIX%\Scripts\setup_x64.bat | ||
| copy "C:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\bin\vcvars64.bat" "C:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\bin\amd64\vcvarsamd64.bat" | ||
|
There was a problem hiding this comment. I'd much prefer explicit to implicit given what this is doing. Could we please make this a proper script that one calls explicitly? |
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we pick a tag or commit maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v0.4.8 is the latest tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add that personally I would also be ok with just including those files with the recipe. This seems like a special case recipe where that is totally appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of the reason I say that, @pelson, is because @patricksnape plans to remove these from his repo. So, it might be best for both of us if they live somewhere new. Here seems like as good as any place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add there are other cases where we are making the same decision (e.g. the
toolchain,conda-forge-build-setup, etc.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't ideal but if necessary I'm not against it.
Does this whole package need to be put in conda-forge-build-setup? At least, I guess that should be the package which depends on this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO it would be nice to keep this separate if this is an option. That way someone has the option of running this locally if needed. Though I'm perfectly fine with making it a dependency of
conda-forge-build-setupand running it in the scripts there.What do you think @msarahan and @patricksnape?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned below, the copying into
Program Fileswould be problematic.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been persuaded. Moving this to conda-forge-build-setup seems like the best way forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Liking the separate package still for local use though.