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

bootstrap.py - script to get Windows dependencies #616

Open
wants to merge 41 commits into
base: master
from

Conversation

Projects
None yet
10 participants
@techtonik
Contributor

techtonik commented Mar 3, 2016

This downloads all dependencies required to build Wesnoth
with GCC on Windows (including SCons). Run, wait for Boost
build to complete, then execute complile.bat to build.

@Elvish-Hunter

This comment has been minimized.

Show comment
Hide comment
@Elvish-Hunter

Elvish-Hunter Mar 4, 2016

Contributor

The script looks nice. I have to test it yet, but from a quick glance I noticed a few things:

  • The filespec list contain a series of dict objects. Since all of them have the same fields, wouldn't it be better to replace the dictionaries with instances of a custom class?
  • I think that you can just support Python 3.2 and later versions, instead of keeping compatibility with the 2.7 series as well. After all, we plan to move all the scripts in data/tools to Python 3.
  • When you generate the SHA-1 hash, you read the file in 64k blocks, instead of all at once. That's usually done when a file is too big to fit in memory, but since it might not be obvious you should note the reason in a comment.
  • You can replace the % string interpolation with the more powerful .format() method. For new code, that's the preferred option. You don't have to, though.
  • open() at line 277 doesn't use the with statement.
  • Several open() calls aren't guarded with a try ... except OSError block, which is needed in case that the user doesn't have the required permissions to perform the read/write operations.
  • The urllib.urlretrieve() call should be guarded with a try ... except block as well, in case of connection problems, or that the files are moved or deleted on the server.
  • Why does the script assume, at line 355, that the codepage used is 437? The comment on the line above seems incomplete.

OK, that's all for now.

Contributor

Elvish-Hunter commented Mar 4, 2016

The script looks nice. I have to test it yet, but from a quick glance I noticed a few things:

  • The filespec list contain a series of dict objects. Since all of them have the same fields, wouldn't it be better to replace the dictionaries with instances of a custom class?
  • I think that you can just support Python 3.2 and later versions, instead of keeping compatibility with the 2.7 series as well. After all, we plan to move all the scripts in data/tools to Python 3.
  • When you generate the SHA-1 hash, you read the file in 64k blocks, instead of all at once. That's usually done when a file is too big to fit in memory, but since it might not be obvious you should note the reason in a comment.
  • You can replace the % string interpolation with the more powerful .format() method. For new code, that's the preferred option. You don't have to, though.
  • open() at line 277 doesn't use the with statement.
  • Several open() calls aren't guarded with a try ... except OSError block, which is needed in case that the user doesn't have the required permissions to perform the read/write operations.
  • The urllib.urlretrieve() call should be guarded with a try ... except block as well, in case of connection problems, or that the files are moved or deleted on the server.
  • Why does the script assume, at line 355, that the codepage used is 437? The comment on the line above seems incomplete.

OK, that's all for now.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 4, 2016

Contributor

Just a few quick answers before I get to code to update the PR.

The filespec list contain a series of dict objects. Since all of them have the same fields, wouldn't it be better to replace the dictionaries with instances of a custom class?

Wrapping entries into some Record construction could make code better documented. I guess I like when data section is isolated from the rest of the code and be simple enough to look at, so that it can be easily edited by non-Python folks. Otherwise they will have to shift through the Python code to get there. It is not a problem to change it though and introduce this Record class just for documeted property access.

I think that you can just support Python 3.2 and later versions

scons doesn't work with Python 3 (yet?), so it would be strange to require this version just for the bootstrap script. PY3K reference is because this code is made as universal as possible to be used for other projects.

When you generate the SHA-1 ... since it might not be obvious you should note the reason in a comment.

Yes. That's an easy one.

open() at line 277 doesn't use the with statement.

It uses oldschool-C-style-explicit-is-better-than-implicit file closing. Actually, there is some magic order of opening and closing files with output file being opened first (for testing writes?) and closed first (not sure why).

Several open() calls aren't guarded with a try ... except OSError block, which is needed in case that the user doesn't have the required permissions to perform the read/write operations.

That's a very unlikely problem. Script downloads everything into .locally subdir in code directory, so if person doesn't have permissions to write there, then that user can't modify the code, build it, and the question is how did that user managed to clone the repository at all?

Also I am not sure about what error handed should do ? Prevent printing stack trace and that's it? Because translating each possible I/O error into user-friendly format seems to be an overkill for the bootstrap script. The script is made to be run by non-programmers - that's right, and I think there is more important concern for Explorer users that cmd window doesn't stay after script exited (with error or not).

The urllib.urlretrieve() call should be guarded with a try ... except block as well, in case of connection problems, or that the files are moved or deleted on the server.

I can work to make specific error messages better, but I need to catch them first and identify in Python 2/3 compatible way, so it is better to do this in case by case basis.

Why does the script assume, at line 355, that the codepage used is 437? The comment on the line above seems incomplete.

The comment misses a link, indeed, because the answer is long - https://stackoverflow.com/questions/606191/convert-bytes-to-a-python-string/27527728#27527728

Contributor

techtonik commented Mar 4, 2016

Just a few quick answers before I get to code to update the PR.

The filespec list contain a series of dict objects. Since all of them have the same fields, wouldn't it be better to replace the dictionaries with instances of a custom class?

Wrapping entries into some Record construction could make code better documented. I guess I like when data section is isolated from the rest of the code and be simple enough to look at, so that it can be easily edited by non-Python folks. Otherwise they will have to shift through the Python code to get there. It is not a problem to change it though and introduce this Record class just for documeted property access.

I think that you can just support Python 3.2 and later versions

scons doesn't work with Python 3 (yet?), so it would be strange to require this version just for the bootstrap script. PY3K reference is because this code is made as universal as possible to be used for other projects.

When you generate the SHA-1 ... since it might not be obvious you should note the reason in a comment.

Yes. That's an easy one.

open() at line 277 doesn't use the with statement.

It uses oldschool-C-style-explicit-is-better-than-implicit file closing. Actually, there is some magic order of opening and closing files with output file being opened first (for testing writes?) and closed first (not sure why).

Several open() calls aren't guarded with a try ... except OSError block, which is needed in case that the user doesn't have the required permissions to perform the read/write operations.

That's a very unlikely problem. Script downloads everything into .locally subdir in code directory, so if person doesn't have permissions to write there, then that user can't modify the code, build it, and the question is how did that user managed to clone the repository at all?

Also I am not sure about what error handed should do ? Prevent printing stack trace and that's it? Because translating each possible I/O error into user-friendly format seems to be an overkill for the bootstrap script. The script is made to be run by non-programmers - that's right, and I think there is more important concern for Explorer users that cmd window doesn't stay after script exited (with error or not).

The urllib.urlretrieve() call should be guarded with a try ... except block as well, in case of connection problems, or that the files are moved or deleted on the server.

I can work to make specific error messages better, but I need to catch them first and identify in Python 2/3 compatible way, so it is better to do this in case by case basis.

Why does the script assume, at line 355, that the codepage used is 437? The comment on the line above seems incomplete.

The comment misses a link, indeed, because the answer is long - https://stackoverflow.com/questions/606191/convert-bytes-to-a-python-string/27527728#27527728

@CelticMinstrel

This comment has been minimized.

Show comment
Hide comment
@CelticMinstrel

CelticMinstrel Mar 5, 2016

Member

@techtonik If you click that little "share" link at the bottom left of the answer (above the comments), you get a much shorter link - http://stackoverflow.com/a/27527728/1502810

Member

CelticMinstrel commented Mar 5, 2016

@techtonik If you click that little "share" link at the bottom left of the answer (above the comments), you get a much shorter link - http://stackoverflow.com/a/27527728/1502810

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 5, 2016

Contributor

Thanks for the pointers.

1. I tried to transform list of dictionaries into list of Records. It took too much time and code looked ugly, because of Python super calls to so I just inserted a comment that explains the data structure.

2. Python 2/3 - if you're interested, the universal code is developed here - https://bitbucket.org/techtonik/locally - Wesnoth changes are more uptodate, but will be synced back eventually

3. SHA1 64k blocks - added comment

4. .format() - not familiar with it https://pyformat.info/ - seems less readable for me, but I agree that new code could should probably use that, because its docs are easier to find. Maybe later.

5. open() is properly closed below. No need for action.

6-7. try Handlers are not touched until a specific problem arises. Script is idempotent, so if something fails badly due to unforeseen circumstances - just kill .locally subdir and run again.

8. cp437 - added comment with a short link.

So everything is addressed I guess.

Contributor

techtonik commented Mar 5, 2016

Thanks for the pointers.

1. I tried to transform list of dictionaries into list of Records. It took too much time and code looked ugly, because of Python super calls to so I just inserted a comment that explains the data structure.

2. Python 2/3 - if you're interested, the universal code is developed here - https://bitbucket.org/techtonik/locally - Wesnoth changes are more uptodate, but will be synced back eventually

3. SHA1 64k blocks - added comment

4. .format() - not familiar with it https://pyformat.info/ - seems less readable for me, but I agree that new code could should probably use that, because its docs are easier to find. Maybe later.

5. open() is properly closed below. No need for action.

6-7. try Handlers are not touched until a specific problem arises. Script is idempotent, so if something fails badly due to unforeseen circumstances - just kill .locally subdir and run again.

8. cp437 - added comment with a short link.

So everything is addressed I guess.

@Elvish-Hunter

This comment has been minimized.

Show comment
Hide comment
@Elvish-Hunter

Elvish-Hunter Mar 12, 2016

Contributor

I finally found the time to run the script - and I was unable to compile Wesnoth on my system.
First of all, I'm on Windows 7/64 bit; from the script's output, everything seemed fine, until I tried running the compile.bat file. The batch file ran, but the compilation failed.
The first problem was that I didn't have the Python path in my environment variables, so I had to add that - a normal Python setup doesn't do it, unless explicitly requested, so you should add a line in the script that warns the user to add the path to Python 2.7 to their environment variables.
After that, Scons was finally able to run, but the compilation failed again. This is my output: http://pastebin.com/BV0CX9nH . As you can see, there was a problem with all the Boost libraries; after noticing that, I found that building the Boost libraries failed, and this is the relative log: http://pastebin.com/U94AkTjc .
What might have went wrong?

Contributor

Elvish-Hunter commented Mar 12, 2016

I finally found the time to run the script - and I was unable to compile Wesnoth on my system.
First of all, I'm on Windows 7/64 bit; from the script's output, everything seemed fine, until I tried running the compile.bat file. The batch file ran, but the compilation failed.
The first problem was that I didn't have the Python path in my environment variables, so I had to add that - a normal Python setup doesn't do it, unless explicitly requested, so you should add a line in the script that warns the user to add the path to Python 2.7 to their environment variables.
After that, Scons was finally able to run, but the compilation failed again. This is my output: http://pastebin.com/BV0CX9nH . As you can see, there was a problem with all the Boost libraries; after noticing that, I found that building the Boost libraries failed, and this is the relative log: http://pastebin.com/U94AkTjc .
What might have went wrong?

@Elvish-Hunter Elvish-Hunter self-assigned this Mar 12, 2016

@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Mar 12, 2016

Contributor

@Elvish-Hunter - From my recent experiences with https://forums.wesnoth.org/viewtopic.php?f=5&t=40694&start=15 , my shoot from the hip guess is that since you are on a Win x64 OS, the script might have built x64 boost libraries for you, when x32 boosts are needed.

Also, I've been wondering that if this pr gets approved, would it be better to use MinGW- 5.1.0 or higher instead of starting it's use with 4.9.2?, based on discussion from the above thread.

Contributor

sigurdfdragon commented Mar 12, 2016

@Elvish-Hunter - From my recent experiences with https://forums.wesnoth.org/viewtopic.php?f=5&t=40694&start=15 , my shoot from the hip guess is that since you are on a Win x64 OS, the script might have built x64 boost libraries for you, when x32 boosts are needed.

Also, I've been wondering that if this pr gets approved, would it be better to use MinGW- 5.1.0 or higher instead of starting it's use with 4.9.2?, based on discussion from the above thread.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 17, 2016

Contributor

Fixed Python path - now a full name to it is written.

Now Boost.. I am on 32-bit OS, so I can't debug bitness problems sufficiently. Looks like Boost libs are built ok - I see iostreams in output - but for some reason compiler can not find them. On my system they are located in .locally/boost_1_57_0/stage/lib/libboost_iostreams-mgw49-mt-1_57.a The usual troubleshooting scheme is to look in build/config.log and see any errors. This is my log for iostreams test:

...
scons: Configure: Checking for Boost iostreams library version >= 1.34.1... 
build\sconf_temp\conftest_5.cpp <-
  |
  |        #include <boost/iostreams/constants.hpp>
  |        
  |#include <boost/version.hpp>
  |#if BOOST_VERSION < 103401
  |#error Boost version is too old!
  |#endif
  |
  |        int main()
  |        {
  |        }
  |        
  |
g++ -o build\sconf_temp\conftest_5.o -c -D_GNU_SOURCE -isystem .locally\SDL2-2.0.4\i686-w64-mingw32\include\SDL2 -isystem .locally\boost_1_57_0 build\sconf_temp\conftest_5.cpp
g++ -o build\sconf_temp\conftest_5.exe -mwindows build\sconf_temp\conftest_5.o -L.locally\SDL2-2.0.4\i686-w64-mingw32\lib -L.locally\boost_1_57_0\stage\lib -lm -lmingw32 -lSDL2main -lSDL2 -lSDL2_net -lboost_iostreams-mgw49-mt-1_57
scons: Configure: yes

scons: Configure: Checking for gzip support in Boost Iostreams... 

@sigurdfdragon MinGW 5.1.0 is certainly possible, but why not 5.3.0? Let me check if it works.

Contributor

techtonik commented Mar 17, 2016

Fixed Python path - now a full name to it is written.

Now Boost.. I am on 32-bit OS, so I can't debug bitness problems sufficiently. Looks like Boost libs are built ok - I see iostreams in output - but for some reason compiler can not find them. On my system they are located in .locally/boost_1_57_0/stage/lib/libboost_iostreams-mgw49-mt-1_57.a The usual troubleshooting scheme is to look in build/config.log and see any errors. This is my log for iostreams test:

...
scons: Configure: Checking for Boost iostreams library version >= 1.34.1... 
build\sconf_temp\conftest_5.cpp <-
  |
  |        #include <boost/iostreams/constants.hpp>
  |        
  |#include <boost/version.hpp>
  |#if BOOST_VERSION < 103401
  |#error Boost version is too old!
  |#endif
  |
  |        int main()
  |        {
  |        }
  |        
  |
g++ -o build\sconf_temp\conftest_5.o -c -D_GNU_SOURCE -isystem .locally\SDL2-2.0.4\i686-w64-mingw32\include\SDL2 -isystem .locally\boost_1_57_0 build\sconf_temp\conftest_5.cpp
g++ -o build\sconf_temp\conftest_5.exe -mwindows build\sconf_temp\conftest_5.o -L.locally\SDL2-2.0.4\i686-w64-mingw32\lib -L.locally\boost_1_57_0\stage\lib -lm -lmingw32 -lSDL2main -lSDL2 -lSDL2_net -lboost_iostreams-mgw49-mt-1_57
scons: Configure: yes

scons: Configure: Checking for gzip support in Boost Iostreams... 

@sigurdfdragon MinGW 5.1.0 is certainly possible, but why not 5.3.0? Let me check if it works.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 17, 2016

Contributor

Updated to MinGW 5.3.0. Still need somebody with 64-bit system to debug what happens with Boost libraries there.

Contributor

techtonik commented Mar 17, 2016

Updated to MinGW 5.3.0. Still need somebody with 64-bit system to debug what happens with Boost libraries there.

@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Mar 18, 2016

Contributor

@techtonik Didn't know there was a MinGW 5.3.0, my suggestion was based on what I was using in the thread.

@Elvish-Hunter does MinGW 5.3.0 make it work? I know I had some trouble getting 4.x.x working when win scons was being fixed, can't remember if the boost thing was a part of it.

Contributor

sigurdfdragon commented Mar 18, 2016

@techtonik Didn't know there was a MinGW 5.3.0, my suggestion was based on what I was using in the thread.

@Elvish-Hunter does MinGW 5.3.0 make it work? I know I had some trouble getting 4.x.x working when win scons was being fixed, can't remember if the boost thing was a part of it.

@Elvish-Hunter

This comment has been minimized.

Show comment
Hide comment
@Elvish-Hunter

Elvish-Hunter Mar 18, 2016

Contributor

I tested again with MinGW 5.3.0. Now, almost everything works - the almost being the gzip support, which still prevents compilation.
When Scons checks the prerequisites, everything is marked with a yes, except for this line:

Checking for gzip support in Boost Iostreams... no

This is the relevant part from the build log: http://pastebin.com/Um1C39WN .

Contributor

Elvish-Hunter commented Mar 18, 2016

I tested again with MinGW 5.3.0. Now, almost everything works - the almost being the gzip support, which still prevents compilation.
When Scons checks the prerequisites, everything is marked with a yes, except for this line:

Checking for gzip support in Boost Iostreams... no

This is the relevant part from the build log: http://pastebin.com/Um1C39WN .

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 18, 2016

Contributor

@Elvish-Hunter line -lboost_iostreams-mgw47-mt-1_57 should not be there for MinGW 5.3.0. Try to drop .locally and build directories and restart the script.

Contributor

techtonik commented Mar 18, 2016

@Elvish-Hunter line -lboost_iostreams-mgw47-mt-1_57 should not be there for MinGW 5.3.0. Try to drop .locally and build directories and restart the script.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 18, 2016

Contributor

Also, it may be that you have a different g++ in the PATH. where command helps to see which version is used.

Contributor

techtonik commented Mar 18, 2016

Also, it may be that you have a different g++ in the PATH. where command helps to see which version is used.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 18, 2016

Contributor

Yes, you do have c:/program files (x86)/codeblocks/mingw/bin/../lib/gcc/mingw32/4.7.1/../../../../mingw32/bin/ld.exe standing in a way. Let me fix that..

Contributor

techtonik commented Mar 18, 2016

Yes, you do have c:/program files (x86)/codeblocks/mingw/bin/../lib/gcc/mingw32/4.7.1/../../../../mingw32/bin/ld.exe standing in a way. Let me fix that..

@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Mar 25, 2016

Contributor

@techtonik I should get the time to test this on my 64-bit os sometime next week. Is the script completely self-contained other than python? By that, I mean I can make a fresh clone of master with this pr, run bootstrap.py, it'll download components such as mingw & scons, and those downloaded components won't affect any other installations I have?

Contributor

sigurdfdragon commented Mar 25, 2016

@techtonik I should get the time to test this on my 64-bit os sometime next week. Is the script completely self-contained other than python? By that, I mean I can make a fresh clone of master with this pr, run bootstrap.py, it'll download components such as mingw & scons, and those downloaded components won't affect any other installations I have?

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 25, 2016

Contributor

@sigurdfdragon yes, it is completely self-contained and doesn't require anything, expect Python. One of my goals was to avoid doing modifications to my operating system or any files outside of source directory.

Contributor

techtonik commented Mar 25, 2016

@sigurdfdragon yes, it is completely self-contained and doesn't require anything, expect Python. One of my goals was to avoid doing modifications to my operating system or any files outside of source directory.

@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Mar 28, 2016

Contributor

@techtonik @Elvish-Hunter @otherdevs
I did a fresh clone of master with the latest updates from this pr, ran it on my win 7 x64 os, and it worked.
Thoughts I had:

  1. The win-scons https://forums.wesnoth.org/viewtopic.php?f=5&t=40694 method (as well as the win 1.13.4 release) has the GNU history optional dependency included as well as being built with experimental open mp support. Should they be added here?
  2. Should the script make a '.gitignore *' file in .locally?
  3. Should cleanup of the readme txt files to fall on the script or the user?
  4. My compile time was over 50 minutes. I think this can be improved by adding multicore support to the compile.bat scons instructions. (-j #cores) Should this be done?

Also, typo in bfw version number in comments (line 8).

Contributor

sigurdfdragon commented Mar 28, 2016

@techtonik @Elvish-Hunter @otherdevs
I did a fresh clone of master with the latest updates from this pr, ran it on my win 7 x64 os, and it worked.
Thoughts I had:

  1. The win-scons https://forums.wesnoth.org/viewtopic.php?f=5&t=40694 method (as well as the win 1.13.4 release) has the GNU history optional dependency included as well as being built with experimental open mp support. Should they be added here?
  2. Should the script make a '.gitignore *' file in .locally?
  3. Should cleanup of the readme txt files to fall on the script or the user?
  4. My compile time was over 50 minutes. I think this can be improved by adding multicore support to the compile.bat scons instructions. (-j #cores) Should this be done?

Also, typo in bfw version number in comments (line 8).

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 28, 2016

Contributor

@sigurdfdragon added .locally to .gitignore. Everything else can be done at any time from my side.

Contributor

techtonik commented Mar 28, 2016

@sigurdfdragon added .locally to .gitignore. Everything else can be done at any time from my side.

@AI0867

This comment has been minimized.

Show comment
Hide comment
@AI0867

AI0867 Mar 29, 2016

Member

Compiling wesnoth can take a lot of memory. Compiling multiple components at the same time can make that memory usage quite a bit larger. This can cause lesser machines to grind to a halt as every operation becomes a page fault.

So, unless you can guarantee that the machine has at least a few gigabytes of RAM free, I do not recommend setting -j4 or the like by default.

Member

AI0867 commented Mar 29, 2016

Compiling wesnoth can take a lot of memory. Compiling multiple components at the same time can make that memory usage quite a bit larger. This can cause lesser machines to grind to a halt as every operation becomes a page fault.

So, unless you can guarantee that the machine has at least a few gigabytes of RAM free, I do not recommend setting -j4 or the like by default.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Mar 29, 2016

Contributor

jobs parameter can actually be specified for compile.bat and it will be passed to SCons.

Contributor

techtonik commented Mar 29, 2016

jobs parameter can actually be specified for compile.bat and it will be passed to SCons.

@Elvish-Hunter

This comment has been minimized.

Show comment
Hide comment
@Elvish-Hunter

Elvish-Hunter Mar 31, 2016

Contributor

Unfortunately, I hit yet another problem.
After the last modifications, the script runs correctly and I can start compiling (I'm still using b347d2b, as we're currently moving to C++11). In fact, Scons confirms that all the base requisites are met:

Saved options: boostdir = 'C:/Users/<username>/Desktop/wesnoth-git/code/.locally/boost_1_57_0',  boostlibdir = 'C:/Users/<username>/Desktop/wesnoth-git/code/.locally/boost_1_57_0/stage/lib', boost_suffix = '-mgw53-mt-1_57', gtkdir = 'C:/Users/<username>/Desktop/wesnoth-git/code/.locally/gtk', sdldir = 'C:/Users/<username>/Desktop/wesnoth-git/code/.locally/SDL2-2.0.4/i686-w64-mingw32'
Building Wesnoth version 1.13.4+dev
Mkdir("build")
---[checking prerequisites]---
Checking for C library m... yes
Checking for C function round()... yes
Checking whether C++ compiler works (g++ version >= 3.3 required)... yes
Checking for Simple DirectMedia Layer library version >= 2.0.2... yes
Checking for SDL2_net library... yes
Checking for Boost iostreams library version >= 1.34.1... yes
Checking for gzip support in Boost Iostreams... yes
Checking for bzip2 support in Boost Iostreams... yes
Checking for Boost random library version >= 1.40.0... yes
Checking for Boost smart_ptr library... yes
Checking for Boost system library... yes
Checking for Boost filesystem library version >= 1.44.0... yes
Checking for Boost locale library... yes
GOOD: Base prerequisites are met
Checking for Simple DirectMedia Layer library version >= 2.0.2... yes
Checking for SDL2_ttf library... yes
Checking for SDL2_mixer library... yes
Checking for SDL2_image library... yes
Checking for C library vorbisfile... yes
Checking for Ogg Vorbis support in SDL... yes
Checking for PNG support in SDL... yes
Checking for JPG support in SDL... yes
Checking for Boost system library... yes
Checking for Boost asio library... yes
Checking for Pango with cairo backend... (cached) yes
Checking for fontconfig... (cached) yes
Checking for Boost program_options library version >= 1.35.0... yes
Checking for Boost regex library version >= 1.35.0... yes
Checking for dbus-1... (cached) no
Checking for fribidi >= 0.10.9... (cached) no
Can't find FriBiDi, disabling FriBiDi support.
Checking for C library png... yes
Checking for C library history... no
Can't find GNU history, disabling history support.
Checking for Boost unit_test_framework library... no
WARN: Unit tests are disabled because their prerequisites are not met

However, while compiling Lua, I get an error and the compilation stops:

g++ -o build\release\lua\liolib.o -c -std=c++98 -D_GNU_SOURCE -W -Wall -mthreads -O2 -Ibuild -Isrc -isystem .locally\SDL2-2.0.4\i686-w64-mingw32\include\SDL2 -isystem .locally\boost_1_57_0 -isystem src\lua src\lua\liolib.cpp
src\lua\liolib.cpp: In function 'int io_pclose(lua_State*)':
src\lua\liolib.cpp:241:29: error: '_pclose' was not declared in this scope
src\lua\liolib.cpp: In function 'int io_popen(lua_State*)':
src\lua\liolib.cpp:249:10: error: '_popen' was not declared in this scope
src\lua\liolib.cpp: In function 'int io_pclose(lua_State*)':
src\lua\liolib.cpp:242:1: warning: control reaches end of non-void function [-Wreturn-type]
scons: *** [build\release\lua\liolib.o] Error 1
scons: building terminated because of errors.

I'm not sure if this problem is related to the script, though.

Contributor

Elvish-Hunter commented Mar 31, 2016

Unfortunately, I hit yet another problem.
After the last modifications, the script runs correctly and I can start compiling (I'm still using b347d2b, as we're currently moving to C++11). In fact, Scons confirms that all the base requisites are met:

Saved options: boostdir = 'C:/Users/<username>/Desktop/wesnoth-git/code/.locally/boost_1_57_0',  boostlibdir = 'C:/Users/<username>/Desktop/wesnoth-git/code/.locally/boost_1_57_0/stage/lib', boost_suffix = '-mgw53-mt-1_57', gtkdir = 'C:/Users/<username>/Desktop/wesnoth-git/code/.locally/gtk', sdldir = 'C:/Users/<username>/Desktop/wesnoth-git/code/.locally/SDL2-2.0.4/i686-w64-mingw32'
Building Wesnoth version 1.13.4+dev
Mkdir("build")
---[checking prerequisites]---
Checking for C library m... yes
Checking for C function round()... yes
Checking whether C++ compiler works (g++ version >= 3.3 required)... yes
Checking for Simple DirectMedia Layer library version >= 2.0.2... yes
Checking for SDL2_net library... yes
Checking for Boost iostreams library version >= 1.34.1... yes
Checking for gzip support in Boost Iostreams... yes
Checking for bzip2 support in Boost Iostreams... yes
Checking for Boost random library version >= 1.40.0... yes
Checking for Boost smart_ptr library... yes
Checking for Boost system library... yes
Checking for Boost filesystem library version >= 1.44.0... yes
Checking for Boost locale library... yes
GOOD: Base prerequisites are met
Checking for Simple DirectMedia Layer library version >= 2.0.2... yes
Checking for SDL2_ttf library... yes
Checking for SDL2_mixer library... yes
Checking for SDL2_image library... yes
Checking for C library vorbisfile... yes
Checking for Ogg Vorbis support in SDL... yes
Checking for PNG support in SDL... yes
Checking for JPG support in SDL... yes
Checking for Boost system library... yes
Checking for Boost asio library... yes
Checking for Pango with cairo backend... (cached) yes
Checking for fontconfig... (cached) yes
Checking for Boost program_options library version >= 1.35.0... yes
Checking for Boost regex library version >= 1.35.0... yes
Checking for dbus-1... (cached) no
Checking for fribidi >= 0.10.9... (cached) no
Can't find FriBiDi, disabling FriBiDi support.
Checking for C library png... yes
Checking for C library history... no
Can't find GNU history, disabling history support.
Checking for Boost unit_test_framework library... no
WARN: Unit tests are disabled because their prerequisites are not met

However, while compiling Lua, I get an error and the compilation stops:

g++ -o build\release\lua\liolib.o -c -std=c++98 -D_GNU_SOURCE -W -Wall -mthreads -O2 -Ibuild -Isrc -isystem .locally\SDL2-2.0.4\i686-w64-mingw32\include\SDL2 -isystem .locally\boost_1_57_0 -isystem src\lua src\lua\liolib.cpp
src\lua\liolib.cpp: In function 'int io_pclose(lua_State*)':
src\lua\liolib.cpp:241:29: error: '_pclose' was not declared in this scope
src\lua\liolib.cpp: In function 'int io_popen(lua_State*)':
src\lua\liolib.cpp:249:10: error: '_popen' was not declared in this scope
src\lua\liolib.cpp: In function 'int io_pclose(lua_State*)':
src\lua\liolib.cpp:242:1: warning: control reaches end of non-void function [-Wreturn-type]
scons: *** [build\release\lua\liolib.o] Error 1
scons: building terminated because of errors.

I'm not sure if this problem is related to the script, though.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Apr 1, 2016

Contributor

@Elvish-Hunter compiler errors are not related to bootstrap script. The may relate to missing dependencies, which SCons checks separately. I added support for boost_thread, which is now a dependency in master af7286c. And master builds ok here.

Contributor

techtonik commented Apr 1, 2016

@Elvish-Hunter compiler errors are not related to bootstrap script. The may relate to missing dependencies, which SCons checks separately. I added support for boost_thread, which is now a dependency in master af7286c. And master builds ok here.

@Elvish-Hunter

This comment has been minimized.

Show comment
Hide comment
@Elvish-Hunter

Elvish-Hunter Apr 3, 2016

Contributor

At long last, I managed to find out what the issue with my system was. Long short story, CodeBlocks' GCC kept conflicting with the one used by this script, so I had to temporarily uninstall CodeBlocks.
After that, Wesnoth compiled succesfully, but didn't start. Instead it warned me about two missing libraries: libiconv-2.dll and libffi-6.dll, which are both located in .locally\gtk\bin. I had to copy them manually to start Wesnoth.
Once you fix the "missing libraries" issue, I'll merge this PR. BTW, don't forget to add yourself to the credits (data/core/about.cfg) and to let us know your forum username, so you can be added to the Code & WML Contributors group.

Contributor

Elvish-Hunter commented Apr 3, 2016

At long last, I managed to find out what the issue with my system was. Long short story, CodeBlocks' GCC kept conflicting with the one used by this script, so I had to temporarily uninstall CodeBlocks.
After that, Wesnoth compiled succesfully, but didn't start. Instead it warned me about two missing libraries: libiconv-2.dll and libffi-6.dll, which are both located in .locally\gtk\bin. I had to copy them manually to start Wesnoth.
Once you fix the "missing libraries" issue, I'll merge this PR. BTW, don't forget to add yourself to the credits (data/core/about.cfg) and to let us know your forum username, so you can be added to the Code & WML Contributors group.

@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Apr 4, 2016

Contributor

@Elvish-Hunter I'm not sure what's different for you, as I got it to compile and run rebased against master at 0a7190c on April 3rd, without getting any warnings about missing dll's. The mentioned dll's are still in my .locally\gtk\bin.

Contributor

sigurdfdragon commented Apr 4, 2016

@Elvish-Hunter I'm not sure what's different for you, as I got it to compile and run rebased against master at 0a7190c on April 3rd, without getting any warnings about missing dll's. The mentioned dll's are still in my .locally\gtk\bin.

@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Apr 4, 2016

Contributor

@Elvish-Hunter @techtonik
It appears my latest 'success' in compiling was due to having "C:\Wesnoth\Master\scons_dependencies\bin; in my PATH". It probably got there when I attempted various things to fix win-scons when it became broke in October 2015 (https://forums.wesnoth.org/viewtopic.php?f=5&t=40694). This is why it worked for me without the dll's in the directory. Having removed that line from my path, I find the same as Elvish-Hunter about the missing dll's.

Contributor

sigurdfdragon commented Apr 4, 2016

@Elvish-Hunter @techtonik
It appears my latest 'success' in compiling was due to having "C:\Wesnoth\Master\scons_dependencies\bin; in my PATH". It probably got there when I attempted various things to fix win-scons when it became broke in October 2015 (https://forums.wesnoth.org/viewtopic.php?f=5&t=40694). This is why it worked for me without the dll's in the directory. Having removed that line from my path, I find the same as Elvish-Hunter about the missing dll's.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Apr 5, 2016

Contributor

Ok. Let me check this myself.

Contributor

techtonik commented Apr 5, 2016

Ok. Let me check this myself.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Apr 5, 2016

Contributor

I see. depends shows that libiconv-2.dll comes from the Ruby installation on my system, which is at C:\Users\user\local settings\bin\ruby200\bin.

Contributor

techtonik commented Apr 5, 2016

I see. depends shows that libiconv-2.dll comes from the Ruby installation on my system, which is at C:\Users\user\local settings\bin\ruby200\bin.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Apr 5, 2016

Contributor

My forum name is the same - techtonik.

Contributor

techtonik commented Apr 5, 2016

My forum name is the same - techtonik.

@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Apr 5, 2016

Contributor

Doing some further tests, the script doesn't add libjpeg-9.dll, which is located in .locally\gtk\bin. The lack of this library causes a black screen behind the map at the game titlescreen, and other graphical issues. There are other dll differences between this build and the official win 1.13.4 build that seem to be more than just the optional dependencies & sdl1 remnants. I haven't looked into these yet.

Contributor

sigurdfdragon commented Apr 5, 2016

Doing some further tests, the script doesn't add libjpeg-9.dll, which is located in .locally\gtk\bin. The lack of this library causes a black screen behind the map at the game titlescreen, and other graphical issues. There are other dll differences between this build and the official win 1.13.4 build that seem to be more than just the optional dependencies & sdl1 remnants. I haven't looked into these yet.

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

'''extract entries from `subdir` of `zipfile` into `target/` directory'''
import os
from os.path import join, exists, dirname
import shutil

This comment has been minimized.

[x] b2 --prefix=..\..\..\_b2 install
[x] cd ..\..\..
[+] set PATH=%PATH%;_b2\bin
"""

This comment has been minimized.

@codacy-bot
@codacy-bot

This comment has been minimized.

@CelticMinstrel

CelticMinstrel Jan 20, 2018

Member

Pretty sure this is a somewhat standard way to make multiline comments in Python?

@CelticMinstrel

CelticMinstrel Jan 20, 2018

Member

Pretty sure this is a somewhat standard way to make multiline comments in Python?

This comment has been minimized.

@techtonik

techtonik Sep 22, 2018

Contributor

Yes. Well, it is also possible to start each line with hashes if that will make the bot happy. =)

@techtonik

techtonik Sep 22, 2018

Contributor

Yes. Well, it is also possible to start each line with hashes if that will make the bot happy. =)

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

#--[inline shellrun 2.0 import run]
import subprocess

This comment has been minimized.

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

@wesnoth wesnoth deleted a comment from codacy-bot Jan 14, 2018

import shutil
import zipfile
zf = zipfile.ZipFile(zippath)

This comment has been minimized.

@codacy-bot
@codacy-bot
# ---[ utilities ]---
import os

This comment has been minimized.

@codacy-bot

This comment has been minimized.

@techtonik

techtonik Sep 22, 2018

Contributor

This is needed to make the snippet copy/pasteable and detectable by static analyzer.

@techtonik

techtonik Sep 22, 2018

Contributor

This is needed to make the snippet copy/pasteable and detectable by static analyzer.

outpipe = subprocess.PIPE
errpipe = subprocess.STDOUT
process = subprocess.Popen(command, shell=True, stdout=outpipe,

This comment has been minimized.

self.success = True
def run(command):
process = subprocess.Popen(command, shell=True)

This comment has been minimized.

# [x] copy varions .dll from .locally into dir that will be packed for
# distribution
#

This comment has been minimized.

@codacy-bot
@codacy-bot
"""
import collections
import subprocess

This comment has been minimized.

def unzip(zippath, target, subdir=None, verbose=0):
'''extract entries from `subdir` of `zipfile` into `target/` directory'''
import os
from os.path import join, exists, dirname

This comment has been minimized.

@codacy-bot
os.makedirs(outdir)
dirs.add(outdir)
if verbose:

This comment has been minimized.

@codacy-bot
@codacy-bot
# file does not exists
if not quiet:
print("Downloading %s into %s" % (entry['filename'], targetdir))
urllib.urlretrieve(entry['url'], filepath)
@soliton-

This comment has been minimized.

Show comment
Hide comment
@soliton-

soliton- Jan 17, 2018

Member

All the run*() calls look like unsanitized strings get injected into shell commands. Either the strings need to get sanitized or handed to the shell as parameters.

That‘s likely the reason for the whitespace issues.

Member

soliton- commented Jan 17, 2018

All the run*() calls look like unsanitized strings get injected into shell commands. Either the strings need to get sanitized or handed to the shell as parameters.

That‘s likely the reason for the whitespace issues.

@GregoryLundberg

This comment has been minimized.

Show comment
Hide comment
@GregoryLundberg

GregoryLundberg Jan 20, 2018

Contributor

Thoughts:

  • Skip Travis/CI runs by adding [ci skip] to the commit message.
  • The recent build failures for Travis and AppVeyor can be fixed by a rebase
  • If you would, please read through the Codacy comments and tell me what you think: does this aid your work?
Contributor

GregoryLundberg commented Jan 20, 2018

Thoughts:

  • Skip Travis/CI runs by adding [ci skip] to the commit message.
  • The recent build failures for Travis and AppVeyor can be fixed by a rebase
  • If you would, please read through the Codacy comments and tell me what you think: does this aid your work?
@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Jan 20, 2018

Contributor

Not sure what @techtonik wants to do with this, but I don't see myself doing anything further with this before 1.14 is released.

I think a script to help people build master on windows would be useful for development, though, for 1.15, it seems likely that there's gonna be some changes to the official windows build, and I would at least want to see what those are first before doing anything further.

The codacy comments do look helpful though

Contributor

sigurdfdragon commented Jan 20, 2018

Not sure what @techtonik wants to do with this, but I don't see myself doing anything further with this before 1.14 is released.

I think a script to help people build master on windows would be useful for development, though, for 1.15, it seems likely that there's gonna be some changes to the official windows build, and I would at least want to see what those are first before doing anything further.

The codacy comments do look helpful though

@loonycyborg

This comment has been minimized.

Show comment
Hide comment
@loonycyborg

loonycyborg Sep 21, 2018

Member

At this point I'm unlikely to use this custom solution for 1.15, I'll be probably using msys2 as in https://forums.wesnoth.org/viewtopic.php?f=5&t=48296 , since such things are better shared between projects, as well as work maintaining them. Unless someone else has a use for this, this PR can be closed.

Member

loonycyborg commented Sep 21, 2018

At this point I'm unlikely to use this custom solution for 1.15, I'll be probably using msys2 as in https://forums.wesnoth.org/viewtopic.php?f=5&t=48296 , since such things are better shared between projects, as well as work maintaining them. Unless someone else has a use for this, this PR can be closed.

@techtonik

This comment has been minimized.

Show comment
Hide comment
@techtonik

techtonik Sep 22, 2018

Contributor

I may think about moving parts of it to SCons to make it download all dependencies much like Go and gradle works. But I ditched Windoze, so no playground to test changes.

Contributor

techtonik commented Sep 22, 2018

I may think about moving parts of it to SCons to make it download all dependencies much like Go and gradle works. But I ditched Windoze, so no playground to test changes.

@sigurdfdragon

This comment has been minimized.

Show comment
Hide comment
@sigurdfdragon

sigurdfdragon Sep 22, 2018

Contributor

When I go to get my 1.15/master installation working again, if I can get the msys2 to work on windows, that's most likely what I'll do

Contributor

sigurdfdragon commented Sep 22, 2018

When I go to get my 1.15/master installation working again, if I can get the msys2 to work on windows, that's most likely what I'll do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment