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

Some instructions missing from the "Compiling Cockatrice" page #2147

Closed
joshuabengal opened this issue Aug 26, 2016 · 21 comments
Closed

Some instructions missing from the "Compiling Cockatrice" page #2147

joshuabengal opened this issue Aug 26, 2016 · 21 comments
Labels
OS - Windows Issue specific to windows operating systems

Comments

@joshuabengal
Copy link

I am setting up a new build environment for Cockatrice, I had previously used the instructions on the "Compiling Cockatrice" page and they were really great. It seems some things got missed when they were updated though.

  • For Visual Studio Express 2015, when installing you need to do a custom install and manually install Visual C++. It is not installed by default and you cannot compile protobuf without it
  • You need to install the Microsoft Windows 8.1 SDK. That should be added with a link. I needed to install this even though my build machine is Windows 7
  • The link to cMake is an outdated version that does not work. The link is for 3.0, you need at least 3.1 to create the installer
  • A note that it takes a little time for the protobuf convert command to generate the correct files for the next step of the process would be helpful. If you simply go in sequence it fails if you run the commands before the convert is finished.
@Daenyth Daenyth added the OS - Windows Issue specific to windows operating systems label Aug 26, 2016
@Daenyth
Copy link
Member

Daenyth commented Aug 26, 2016

@joshuabengal it seems like you have some excellent suggestions there. The wiki is public, would you mind updating that page with your new information? You're right that it's out of date / misleading

@joshuabengal
Copy link
Author

Sure. Let me actually get Cockatrice to build :) and I can then update everything all at once. It looks like you need to retarget two of the projects for it to build in 2015

@Daenyth
Copy link
Member

Daenyth commented Aug 26, 2016

@woogerboy21 might be able to help with VS-specific stuff

@joshuabengal
Copy link
Author

OK, I updated the comment about having to check VC++, updated the cmake link to the latest version, and added a note about the protobuf convert time. I did not add a note about the SDK since I can't currently get Cockatrice to install and it appears to be an SDK issue and I don't want to add instructions that are not yet working for me

@ctrlaltca
Copy link
Contributor

Afaik there's no need to install the Sdk.
If you need some inspiration, you can have a look at the appveyor build script: https://github.com/Cockatrice/Cockatrice/blob/master/appveyor.yml
It's basically a list of cmd.exe commands and powershell commands. It's still using msvc2013 anyway, since Qt only recently added support for msvc2015.
You may need to comment out this line: https://github.com/Cockatrice/Cockatrice/blob/master/CMakeLists.txt#L82 , since iirc msvc 2015 doesn't support windows Xp anymore.

@joshuabengal
Copy link
Author

I commented out that line, still won't compile, throws same error.

I found a site saying you needed to make sure "Common Tools for Visual C++ 2015" was checked on the VS Install, I checked and mine wasn't, so I checked it, installed those, rebooted, and still, no dice. build crashes with the same error

Any other ideas?

@joshuabengal
Copy link
Author

joshuabengal commented Aug 26, 2016

As an aside you may want to update the page to let people know that the current instructions will not work and you will not be able to compile cockatrice. No need for people to go through all the install time and end up with something that doesn't work.

Also, I am curious, why does that page say those instructions have been "successfully tested" when they were incomplete?

@ZeldaZach
Copy link
Member

They were tested at a point and then updated to change things around with new versions, I assume. If you made the changes needed, feel free to close this ticket. Thanks @joshuabengal!

@joshuabengal
Copy link
Author

I still cannot figure out how to update the directions on the GUI instructions page

@tooomm
Copy link
Member

tooomm commented Aug 30, 2016

Ok, now I finally understood your problem... make sure to include screenshots next time, they help a lot!
(you simply need to drag&drop a picture file from your pc to the comment field here to have it included!)

For clarification:

@skwerlman
Copy link
Contributor

A simple solution to this issue would be to move the GUI instructions to a new wiki page, and link there instead.
Linking to a static point in history is typically a bad idea on a wiki.

@joshuabengal
Copy link
Author

I'm not the best Wiki guy in the world, is someone could move that history to a new wiki page and fix the link I'd be grateful, and i will then go and update the documentation

@skwerlman
Copy link
Contributor

Here you go: https://github.com/Cockatrice/Cockatrice/wiki/temp-windows-gui
I'll merge it in with the main windows instructions once you're done updating it

@joshuabengal
Copy link
Author

Ok, the gui page is now updated with the correct instructions that are confirmed working.

@skwerlman
Copy link
Contributor

skwerlman commented Aug 31, 2016

I have a couple questions about the instructions before I merge them:

  1. Why is Powershell 4 needed? I don't see it being used anywhere.
  2. Why install the GitHub GUI when all you need is Git Bash? You can get that here: https://git-scm.com/download/win Actually this is a problem with the new instructions, too. I'll fix that when i merge the two
  3. Can MariaDB be used instead of MySQL for consistency with the newer instructions?

@joshuabengal
Copy link
Author

joshuabengal commented Aug 31, 2016

My answers (prob not the best but it's all I can offer :)

  1. I don't know, I just saw it listed and included it. I have not tried to build without powershell installed, if that works then yes we should remove that line
  2. I never was able to get the non GUI instructions to work. It throws a VC compiler version error. Also, the instructions intimate that if you are not going to run servatrice you don't need MariaDB/MySQL installed, however the project will not compile without at least MySQL installed per the GUI instructions.

If you want to test a few different scenarios and get different results feel free to update what I have written.

@Daenyth
Copy link
Member

Daenyth commented Aug 31, 2016

@skwerlman mariadb should work but I don't think I've personally tested it.

@woogerboy21
Copy link
Contributor

woogerboy21 commented Sep 6, 2016

@skwerlman
Powershell 4 was required in the non gui instructions due to the github application needing it if I recall correctly. I wouldnt suggest github bash, use the command line tools included in the official github application (just my $0.02). And yes mariadb will work as a drop in replacement for mysql.

@skwerlman
Copy link
Contributor

I don't see PowerShell mentioned in the Github Application documentation, so it could be that it's no longer required. I'd test, but Win10 comes with PS4 by default.

Git Bash is the command line tool included with the Github application: https://git-scm.com/

I'll replace the MySQL instructions for consistency, then.

@woogerboy21
Copy link
Contributor

woogerboy21 commented Oct 18, 2016

Update it how you like. Im of the opinion that you shouldnt need multiple tools to open multiple console to accomplish the task. Using PS4 you can run all your git commands and MS build commands from a single command window rather than jump between multiple command shells to complete the builds.

I also dont recall having to do any type of custom / manual install of VC++ in 2015. But its been a while since I installed it.

@woogerboy21
Copy link
Contributor

Is the only thing left on this issue the question on moving the GUI based instructions to some were?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS - Windows Issue specific to windows operating systems
Projects
None yet
Development

No branches or pull requests

7 participants