Fix Build/Update Boost on Windows (non-PR-754 version) #789

stephengtuggy · 2023-06-19T01:28:53Z

Thank you for submitting a pull request and becoming a contributor to the Vega Strike Core Engine.

Please answer the following:

Code Changes:

Have the PR Validation Tests been run? See https://github.com/vegastrike/Vega-Strike-Engine-Source/wiki/Pull-Request-Validation

Issues:

Purpose:

What is this pull request trying to do? Update Boost on Windows, on the master branch.
What release is this for? 0.9.x
Is there a project or milestone we should apply this to? 0.9.x

engine/src/cmd/collide_map.cpp

stephengtuggy · 2023-06-19T02:17:14Z

Please note also that this looks like a very large changed-line count, but probably at least 1,000 of those are just whitespace changes.

BenjamenMeyer · 2023-06-21T23:32:40Z

@stephengtuggy Some links to try out:

boost 1.67.0 with python 3.7.0: undefined reference to `PyUnicode_Type' in python/converter/builtin_converters.hpp:157 boostorg/python#233 - using the pymalloc version (python3.7m)
https://stackoverflow.com/questions/65407610/relocation-against-xxx-in-read-only-section-text-wrong-compiler-or-linux-se -- order of linking; we might want to try a link group (more complex to setup in CMake but still doable)

…; copyleft updates; etc.

…ments

…om corresponding pr-754 branch

… (and other headers) after all

…ros/versions, some of which are commented out

engine/src/cmd/collide_map.cpp

royfalk · 2023-09-12T20:19:49Z

Gameplay seems fine. I'll try and review the code on my machine tomorrow.

royfalk

Looks fine overall. I'd appreciate separating the cosmetic from the code and the code from the CICD stuff where possible.

royfalk · 2023-09-13T19:07:23Z

.github/workflows/Windows-CI.yml

-            cmakeGenerator: VS16Win64
-          - os:             windows-2022
-            cmakeGenerator: Ninja
+          - os:              windows-2019


Are we really testing all these variants? It may be a bit excessive IMO.

Good point. We can probably cut back a bit.

let's just stick to 1 Windows build for the time being; that should at least get the ball rolling.

How about 2 Windows builds? One on Windows (Server) 2019, and one on Windows (Server) 2022. I think that will be enough even to satisfy me, haha.

@stephengtuggy if we need to

well, VS2022 on Windows 2022 passed; while VS2019 on Windows 2019 failed. Just noting since we're going to merge this PR without fully fixing that.

royfalk · 2023-09-13T19:09:41Z

.github/workflows/gh-actions-pr.yml

+          #  OpenGL_GL_PREFERENCE: 'GLVND'
+          #  ENABLE_PIE: 'OFF'
+          #  allow_failure: false
+          - FROM:     'ubuntu:kinetic'


Again, I think we need to restrict our officially supported everything. We are not a large corporation and even there I don't recall supporting so many versions.

I agree. The policy we decided on last was to support the LTS versions of distros such as Ubuntu that are still in mainstream support, as well as the latest. We may have to revisit that a bit; it's turning out to be more than I can handle lately.

It's generally best effort right now. If we need to drop something because we don't have the bandwidth then that's fine - let's just document it and why.

That said, most of our distros are highly related with some simple packaging changes between them so I don't think it's a huge effort for Linux. Mac/Windows, OTOH, are still in a larger need of support and we could certainly use some help there.

Definitely.

So Kinetic is no longer supported since Manic is out. Will fix in master.

.github/workflows/gh-actions-pr.yml

engine/CMakeLists.txt

engine/src/cmd/base_init.cpp

royfalk · 2023-09-13T19:48:37Z

engine/src/cmd/collide_map.h

@@ -45,7 +43,7 @@ class Collidable {
        Unit *unit;
        unsigned int bolt_index;
    }
-            ref;
+            ref{};


What does this do? I assume it's defined either way.
Also, can we fix the indentation. It's handing out there.

Adding the empty braces initializes the value of ref. It was uninitialized previously, as far as I can tell.

C++11 initialization method; we've generally stuck to constructor initialization but we've started getting more C++11 brace initialization in. We just have to make sure it's C++11 format and not C++20 feature in the process.

Intuitively, it seemed to me it was initialized. However, on closer inspection, this is a non-static variable, so it really is uninitialized. it would be clearer if we were to specify :

Unit *unit = nullptr; unsigned int bolt_index; } ref;`

This initialization is for any kind of variable, not simply non-static. The C++ committee's idea is that the constructors don't need to do all the initialization of the class since they have this other means.
I think Strostroup's idea is that the constructor isn't necessary in 99% of cases and this lets them do that.

That said, I still kind of like having explicit constructors doing all the initialization.

engine/src/python/unit_exports.cpp

royfalk · 2023-09-13T19:52:19Z

engine/src/python/unit_wrapper_class.h

@@ -162,15 +167,15 @@ class UnitWrapper : public UnitContainer {
    boost::python::tuple GetOrientation() {
        {
        CHECKME VS_BOOST_MAKE_TUPLE(VS_BOOST_MAKE_TUPLE(0, 0, 0), VS_BOOST_MAKE_TUPLE(0, 0, 0), VS_BOOST_MAKE_TUPLE(0,
-                    0,


Probably better to just not break the line or break earlier for easier reading. Same for the rest of the file.

engine/src/universe_util_generic.cpp

royfalk · 2023-09-13T19:55:33Z

engine/src/universe_util_generic.cpp

                    }
                }
-            }
-            if (collision == false) {


Did you move this code block to an inner brace intentionally?

Let me take another look. I think so, but now I'm not sure which way is correct.

Update: Yes, this was intentional. I think a previous PR of mine messed up the structure of this file a bit. This is an attempt to fix it.

BenjamenMeyer · 2023-10-06T20:11:25Z

@stephengtuggy please merge in master

stephengtuggy · 2023-10-11T03:53:48Z

@stephengtuggy please merge in master

Done, I believe.

BenjamenMeyer · 2023-10-17T05:09:19Z

@stephengtuggy please merge in master

Done, I believe.

Unfortunately not, or at least the merge didn't work right. It's failing at minimum because it can't find SDL2; and checking the bootstrap script it doesn't seem to be trying to install it so it doesn't have some of the required changes.
It's almost acting like it has the wrong Docker images too. Hopefully it's just the bootstrap scripts and some related CMake settings for SDL2.

…4_checkpoint-merge Update boost windows non 754 checkpoint merge

stephengtuggy · 2023-11-10T15:47:35Z

@stephengtuggy please merge in master

Done, I believe.

Unfortunately not, or at least the merge didn't work right. It's failing at minimum because it can't find SDL2; and checking the bootstrap script it doesn't seem to be trying to install it so it doesn't have some of the required changes. It's almost acting like it has the wrong Docker images too. Hopefully it's just the bootstrap scripts and some related CMake settings for SDL2.

@BenjamenMeyer I merged your PR into mine. 😃 Hopefully it works now. Thanks.

BenjamenMeyer · 2023-11-10T19:29:59Z

Well more successes than failures overall.

Windows 2022 passed; 2019 did not
most Mac builds failed, but 3 passed
all Linux builds passed except Ubuntu Kinetic
So progress

NOTE: Kinetic seems to be missing from the Linux Bootstrap support; not sure why but that's likely the cause. (UPDATE: Doesn't seem to be in master either.) Surprised it took until SDL2 for it fail though.

stephengtuggy · 2023-11-11T02:07:59Z

To me, the fact that we got any Windows builds to succeed at all this run was a success. Let alone macOS. And if Kinetic is the only Linux build that failed, I would say we can go ahead and merge this PR. We need to get unstuck, as far as our development efforts go. We can clean up around the edges later.

stephengtuggy · 2023-11-11T02:12:04Z

(Yes, I know that's a change in tune from what I usually say 😄 )

BenjamenMeyer · 2023-11-12T04:07:35Z

@stephengtuggy I can live with that.

.github/workflows/macos-ci.yml

BenjamenMeyer

We'll fix the Mac/Windows builds and swap Ubuntu Kinetic for Mantic on master.

stephengtuggy · 2023-11-24T01:08:46Z

Merged, finally!

stephengtuggy added 15 commits June 17, 2023 18:20

More GH Action updates

5657972

codeql.yml: Uncomment with: languages clause for CodeQL Init

3e82023

Update Windows-CI.yml

673fdc8

Update codeql.yml

8ab1214

Update fortify-on-demand-scan.yml

6ae4f71

gh-actions-pr.yml: whitespace updates

accfb12

Update gh-actions-release.yml

773e0c5

More on GH Actions

a403c7a

Updates to engine/CMakeLists.txt

f8a57d5

Add engine/CMakePresets.json

45053ae

Updates to 9 .cpp and .h files

d0a4c70

vcpkg.json: updates

7efdac1

Update bootstrap.ps1 and build.ps1; add test.ps1

b2c873d

32 .cpp and .h files: various updates

7a9a423

updates to 13 more .cpp and .h files

e2af41a

stephengtuggy added this to the 0.9.x milestone Jun 19, 2023

stephengtuggy self-assigned this Jun 19, 2023

stephengtuggy added the CI/CD label Jun 19, 2023

stephengtuggy commented Jun 19, 2023

View reviewed changes

engine/src/cmd/collide_map.cpp Show resolved Hide resolved

stephengtuggy added 9 commits June 24, 2023 17:08

CMakeLists.txt: Only set BOOST_USE_WINAPI_VERSION, etc. on Windows

6c5853d

6 copies of CMakeLists.txt: Troubleshooting Position-Independent Code…

690ddd6

…; copyleft updates; etc.

LICENSE, cmake-config.h.in: Copyleft updates

4b115c0

cmake-config.h.in: Reformat copyleft header block

3ea8071

CMakeLists.txt: Omit APPEND clause from INCLUDE_DIRECTORIES state…

988a257

…ments

engine/CMakeLists.txt, CMakePresets.json: Replicate latest changes fr…

dc9e179

…om corresponding pr-754 branch

engine/CMakeLists.txt: Missed a spot

3f9ec5c

41 .cpp and .h files: #include <boost/python.hpp> ahead of Python.h…

b945b39

… (and other headers) after all

gh-actions-pr.yml: Add ENABLE_PIE flag, as well as several add'l dist…

dd72ed0

…ros/versions, some of which are commented out

stephengtuggy added 3 commits September 4, 2023 14:36

Add vcpkg-configuration.json

e205873

vcpkg.json: Update schema file URL

3cbbbf9

vcpkg-configuration.json: Update baseline

a574ed3

stephengtuggy mentioned this pull request Sep 4, 2023

Upgrade SDL to version 2 #792

Merged

1 task

BenjamenMeyer requested changes Sep 5, 2023

View reviewed changes

engine/src/cmd/collide_map.cpp Outdated Show resolved Hide resolved

royfalk reviewed Sep 13, 2023

View reviewed changes

stephengtuggy changed the title ~~Update Boost on Windows (non-PR-754 version)~~ Fix Build/Update Boost on Windows (non-PR-754 version) Sep 14, 2023

This was referenced Sep 15, 2023

Revisit whether to keep code that references Boost version 1.28 #797

Open

Add virtual destructor to each class intended to be inheritable #798

Open

Discussion: Whether to use auto for iterators #799

Closed

royfalk mentioned this pull request Sep 16, 2023

Task aug unit #796

Merged

1 task

BenjamenMeyer mentioned this pull request Oct 7, 2023

Refactor cmake #802

Closed

3 tasks

stephengtuggy added 2 commits October 10, 2023 20:45

collide_map.cpp: Don't cache list_end, as discussed

eef1f8d

Merge branch 'master' into update-boost-windows-non-754

723441d

stephengtuggy and others added 3 commits November 8, 2023 23:21

collide_map.cpp: Don't cache list_end, as discussed

31bcad7

Merge branch 'master' into update-boost-windows-non-754_checkpoint-merge

11e5b23

Merge pull request #10 from BenjamenMeyer/update-boost-windows-non-75…

848e8c3

…4_checkpoint-merge Update boost windows non 754 checkpoint merge

BenjamenMeyer reviewed Nov 12, 2023

View reviewed changes

.github/workflows/macos-ci.yml Show resolved Hide resolved

BenjamenMeyer approved these changes Nov 12, 2023

View reviewed changes

stephengtuggy merged commit 524ea3b into vegastrike:master Nov 24, 2023
54 of 67 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Build/Update Boost on Windows (non-PR-754 version) #789

Fix Build/Update Boost on Windows (non-PR-754 version) #789

stephengtuggy commented Jun 19, 2023 •

edited

Loading

stephengtuggy commented Jun 19, 2023

BenjamenMeyer commented Jun 21, 2023 •

edited

Loading

royfalk commented Sep 12, 2023

royfalk left a comment

royfalk Sep 13, 2023

stephengtuggy Sep 14, 2023

BenjamenMeyer Oct 14, 2023

stephengtuggy Oct 15, 2023

BenjamenMeyer Nov 10, 2023

BenjamenMeyer Nov 12, 2023 •

edited

Loading

royfalk Sep 13, 2023

stephengtuggy Sep 14, 2023

BenjamenMeyer Sep 15, 2023

stephengtuggy Sep 15, 2023

BenjamenMeyer Nov 12, 2023

royfalk Sep 13, 2023

stephengtuggy Sep 14, 2023

BenjamenMeyer Sep 15, 2023

royfalk Sep 16, 2023 •

edited

Loading

BenjamenMeyer Sep 17, 2023

royfalk Sep 13, 2023

royfalk Sep 13, 2023

stephengtuggy Sep 14, 2023

stephengtuggy Nov 24, 2023

BenjamenMeyer commented Oct 6, 2023

stephengtuggy commented Oct 11, 2023

BenjamenMeyer commented Oct 17, 2023

stephengtuggy commented Nov 10, 2023

BenjamenMeyer commented Nov 10, 2023 •

edited

Loading

stephengtuggy commented Nov 11, 2023

stephengtuggy commented Nov 11, 2023

BenjamenMeyer commented Nov 12, 2023

BenjamenMeyer left a comment

stephengtuggy commented Nov 24, 2023

Fix Build/Update Boost on Windows (non-PR-754 version) #789

Fix Build/Update Boost on Windows (non-PR-754 version) #789

Conversation

stephengtuggy commented Jun 19, 2023 • edited Loading

stephengtuggy commented Jun 19, 2023

BenjamenMeyer commented Jun 21, 2023 • edited Loading

royfalk commented Sep 12, 2023

royfalk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenjamenMeyer Nov 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

royfalk Sep 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenjamenMeyer commented Oct 6, 2023

stephengtuggy commented Oct 11, 2023

BenjamenMeyer commented Oct 17, 2023

stephengtuggy commented Nov 10, 2023

BenjamenMeyer commented Nov 10, 2023 • edited Loading

stephengtuggy commented Nov 11, 2023

stephengtuggy commented Nov 11, 2023

BenjamenMeyer commented Nov 12, 2023

BenjamenMeyer left a comment

Choose a reason for hiding this comment

stephengtuggy commented Nov 24, 2023

stephengtuggy commented Jun 19, 2023 •

edited

Loading

BenjamenMeyer commented Jun 21, 2023 •

edited

Loading

BenjamenMeyer Nov 12, 2023 •

edited

Loading

royfalk Sep 16, 2023 •

edited

Loading

BenjamenMeyer commented Nov 10, 2023 •

edited

Loading