Merge lp:~widelands-dev/widelands/windows-logging into lp:widelands
- windows-logging
- Merge into trunk
Proposed by
GunChleoc
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 8816 | ||||||||
Proposed branch: | lp:~widelands-dev/widelands/windows-logging | ||||||||
Merge into: | lp:widelands | ||||||||
Diff against target: |
334 lines (+88/-34) 10 files modified
src/CMakeLists.txt (+0/-2) src/base/CMakeLists.txt (+1/-0) src/base/log.cc (+39/-11) src/base/log.h (+9/-0) src/economy/test/CMakeLists.txt (+1/-0) src/economy/test/test_road.cc (+6/-0) src/main.cc (+6/-12) src/notifications/test/CMakeLists.txt (+1/-0) src/notifications/test/notifications_test.cc (+5/-0) src/wlapplication.cc (+20/-9) |
||||||||
To merge this branch: | bzr merge lp:~widelands-dev/widelands/windows-logging | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
hessenfarmer | Approve | ||
Review via email: mp+354397@code.launchpad.net |
Commit message
Write Windows log output to homedir instead of the program dir.
Description of the change
To post a comment you must log in.
Revision history for this message
GunChleoc (gunchleoc) wrote : | # |
Revision history for this message
bunnybot (widelandsofficial) wrote : | # |
Continuous integration builds have changed state:
Travis build 3908. State: passed. Details: https:/
Appveyor build 3706. State: success. Details: https:/
Revision history for this message
hessenfarmer (stephan-lutz) wrote : | # |
tested the appveyor build as well. OS win 10. Install dir on drive D:/ stdout.txt in user/.widelands.
seems to be good to go
review:
Approve
Revision history for this message
GunChleoc (gunchleoc) wrote : | # |
Thanks!
@bunnybot merge
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/CMakeLists.txt' | |||
2 | --- src/CMakeLists.txt 2018-07-08 10:05:27 +0000 | |||
3 | +++ src/CMakeLists.txt 2018-09-06 15:39:53 +0000 | |||
4 | @@ -45,7 +45,6 @@ | |||
5 | 45 | ${WIN32_ICON_O} | 45 | ${WIN32_ICON_O} |
6 | 46 | USES_SDL2 | 46 | USES_SDL2 |
7 | 47 | DEPENDS | 47 | DEPENDS |
8 | 48 | base_log | ||
9 | 49 | base_exceptions | 48 | base_exceptions |
10 | 50 | widelands_ball_of_mud | 49 | widelands_ball_of_mud |
11 | 51 | build_info | 50 | build_info |
12 | @@ -56,7 +55,6 @@ | |||
13 | 56 | main.cc | 55 | main.cc |
14 | 57 | USES_SDL2 | 56 | USES_SDL2 |
15 | 58 | DEPENDS | 57 | DEPENDS |
16 | 59 | base_log | ||
17 | 60 | base_exceptions | 58 | base_exceptions |
18 | 61 | widelands_ball_of_mud | 59 | widelands_ball_of_mud |
19 | 62 | build_info | 60 | build_info |
20 | 63 | 61 | ||
21 | === modified file 'src/base/CMakeLists.txt' | |||
22 | --- src/base/CMakeLists.txt 2017-03-04 12:37:17 +0000 | |||
23 | +++ src/base/CMakeLists.txt 2018-09-06 15:39:53 +0000 | |||
24 | @@ -12,6 +12,7 @@ | |||
25 | 12 | DEPENDS | 12 | DEPENDS |
26 | 13 | base_macros | 13 | base_macros |
27 | 14 | base_exceptions | 14 | base_exceptions |
28 | 15 | build_info | ||
29 | 15 | ) | 16 | ) |
30 | 16 | 17 | ||
31 | 17 | wl_library(base_exceptions | 18 | wl_library(base_exceptions |
32 | 18 | 19 | ||
33 | === modified file 'src/base/log.cc' | |||
34 | --- src/base/log.cc 2018-04-07 16:59:00 +0000 | |||
35 | +++ src/base/log.cc 2018-09-06 15:39:53 +0000 | |||
36 | @@ -19,10 +19,12 @@ | |||
37 | 19 | 19 | ||
38 | 20 | #include "base/log.h" | 20 | #include "base/log.h" |
39 | 21 | 21 | ||
40 | 22 | #include <cassert> | ||
41 | 22 | #include <cstdarg> | 23 | #include <cstdarg> |
42 | 23 | #include <cstdio> | 24 | #include <cstdio> |
43 | 24 | #include <fstream> | 25 | #include <fstream> |
44 | 25 | #include <iostream> | 26 | #include <iostream> |
45 | 27 | #include <memory> | ||
46 | 26 | 28 | ||
47 | 27 | #include <SDL.h> | 29 | #include <SDL.h> |
48 | 28 | #ifdef _WIN32 | 30 | #ifdef _WIN32 |
49 | @@ -31,6 +33,9 @@ | |||
50 | 31 | 33 | ||
51 | 32 | #include "base/macros.h" | 34 | #include "base/macros.h" |
52 | 33 | #include "base/wexception.h" | 35 | #include "base/wexception.h" |
53 | 36 | #ifdef _WIN32 | ||
54 | 37 | #include "build_info.h" | ||
55 | 38 | #endif | ||
56 | 34 | 39 | ||
57 | 35 | namespace { | 40 | namespace { |
58 | 36 | 41 | ||
59 | @@ -38,7 +43,6 @@ | |||
60 | 38 | void sdl_logging_func(void* userdata, int, SDL_LogPriority, const char* message); | 43 | void sdl_logging_func(void* userdata, int, SDL_LogPriority, const char* message); |
61 | 39 | 44 | ||
62 | 40 | #ifdef _WIN32 | 45 | #ifdef _WIN32 |
63 | 41 | |||
64 | 42 | std::string get_output_directory() { | 46 | std::string get_output_directory() { |
65 | 43 | // This took inspiration from SDL 1.2 logger code. | 47 | // This took inspiration from SDL 1.2 logger code. |
66 | 44 | #ifdef _WIN32_WCE | 48 | #ifdef _WIN32_WCE |
67 | @@ -57,12 +61,17 @@ | |||
68 | 57 | // This Logger emulates the SDL1.2 behavior of writing a stdout.txt. | 61 | // This Logger emulates the SDL1.2 behavior of writing a stdout.txt. |
69 | 58 | class WindowsLogger { | 62 | class WindowsLogger { |
70 | 59 | public: | 63 | public: |
72 | 60 | WindowsLogger() : stdout_filename_(get_output_directory() + "\\stdout.txt") { | 64 | WindowsLogger(const std::string& dir) : stdout_filename_(dir + "\\stdout.txt") { |
73 | 61 | stdout_.open(stdout_filename_); | 65 | stdout_.open(stdout_filename_); |
74 | 62 | if (!stdout_.good()) { | 66 | if (!stdout_.good()) { |
76 | 63 | throw wexception("Unable to initialize logging to stdout.txt"); | 67 | throw wexception("Unable to initialize stdout logging destination: %s", stdout_filename_.c_str()); |
77 | 64 | } | 68 | } |
78 | 65 | SDL_LogSetOutputFunction(sdl_logging_func, this); | 69 | SDL_LogSetOutputFunction(sdl_logging_func, this); |
79 | 70 | std::cout << "Log output will be written to: " << stdout_filename_ << std::endl; | ||
80 | 71 | |||
81 | 72 | // Repeat version info so that we'll have it available in the log file too | ||
82 | 73 | stdout_ << "This is Widelands Version " << build_id() << " (" << build_type() << ")" << std::endl; | ||
83 | 74 | stdout_.flush(); | ||
84 | 66 | } | 75 | } |
85 | 67 | 76 | ||
86 | 68 | void log_cstring(const char* buffer) { | 77 | void log_cstring(const char* buffer) { |
87 | @@ -107,23 +116,42 @@ | |||
88 | 107 | static_cast<Logger*>(userdata)->log_cstring(message); | 116 | static_cast<Logger*>(userdata)->log_cstring(message); |
89 | 108 | } | 117 | } |
90 | 109 | #endif | 118 | #endif |
91 | 110 | |||
92 | 111 | } // namespace | 119 | } // namespace |
93 | 112 | 120 | ||
94 | 113 | // Default to stdout for logging. | 121 | // Default to stdout for logging. |
95 | 114 | bool g_verbose = false; | 122 | bool g_verbose = false; |
96 | 115 | 123 | ||
97 | 124 | #ifdef _WIN32 | ||
98 | 125 | // Start with nullptr so that we won't initialize an empty file in the program's directory | ||
99 | 126 | std::unique_ptr<WindowsLogger> logger(nullptr); | ||
100 | 127 | |||
101 | 128 | // Set the logging dir to the given homedir | ||
102 | 129 | bool set_logging_dir(const std::string& homedir) { | ||
103 | 130 | try { | ||
104 | 131 | logger.reset(new WindowsLogger(homedir)); | ||
105 | 132 | } catch (const std::exception& e) { | ||
106 | 133 | std::cout << e.what() << std::endl; | ||
107 | 134 | return false; | ||
108 | 135 | } | ||
109 | 136 | return true; | ||
110 | 137 | } | ||
111 | 138 | |||
112 | 139 | // Set the logging dir to the program's dir. For running test cases where we don't have a homedir. | ||
113 | 140 | void set_logging_dir() { | ||
114 | 141 | logger.reset(new WindowsLogger(get_output_directory())); | ||
115 | 142 | } | ||
116 | 143 | |||
117 | 144 | #else | ||
118 | 145 | std::unique_ptr<Logger> logger(new Logger()); | ||
119 | 146 | #endif | ||
120 | 147 | |||
121 | 116 | void log(const char* const fmt, ...) { | 148 | void log(const char* const fmt, ...) { |
127 | 117 | #ifdef _WIN32 | 149 | assert(logger != nullptr); |
128 | 118 | static WindowsLogger logger; | 150 | |
124 | 119 | #else | ||
125 | 120 | static Logger logger; | ||
126 | 121 | #endif | ||
129 | 122 | char buffer[2048]; | 151 | char buffer[2048]; |
130 | 123 | va_list va; | 152 | va_list va; |
131 | 124 | |||
132 | 125 | va_start(va, fmt); | 153 | va_start(va, fmt); |
133 | 126 | vsnprintf(buffer, sizeof(buffer), fmt, va); | 154 | vsnprintf(buffer, sizeof(buffer), fmt, va); |
134 | 127 | va_end(va); | 155 | va_end(va); |
136 | 128 | logger.log_cstring(buffer); | 156 | logger->log_cstring(buffer); |
137 | 129 | } | 157 | } |
138 | 130 | 158 | ||
139 | === modified file 'src/base/log.h' | |||
140 | --- src/base/log.h 2018-04-07 16:59:00 +0000 | |||
141 | +++ src/base/log.h 2018-09-06 15:39:53 +0000 | |||
142 | @@ -28,4 +28,13 @@ | |||
143 | 28 | 28 | ||
144 | 29 | extern bool g_verbose; | 29 | extern bool g_verbose; |
145 | 30 | 30 | ||
146 | 31 | #ifdef _WIN32 | ||
147 | 32 | /** Set the directory that stdout.txt shall be written to. | ||
148 | 33 | * This should be the same dir where widelands writes its config file. Returns true on success. | ||
149 | 34 | */ | ||
150 | 35 | bool set_logging_dir(const std::string& homedir); | ||
151 | 36 | // Set the directory that stdout.txt shall be written to to the directory the program is started from. Use this only for test cases. | ||
152 | 37 | void set_logging_dir(); | ||
153 | 38 | #endif | ||
154 | 39 | |||
155 | 31 | #endif // end of include guard: WL_BASE_LOG_H | 40 | #endif // end of include guard: WL_BASE_LOG_H |
156 | 32 | 41 | ||
157 | === modified file 'src/economy/test/CMakeLists.txt' | |||
158 | --- src/economy/test/CMakeLists.txt 2017-06-20 12:38:50 +0000 | |||
159 | +++ src/economy/test/CMakeLists.txt 2018-09-06 15:39:53 +0000 | |||
160 | @@ -4,6 +4,7 @@ | |||
161 | 4 | test_road.cc | 4 | test_road.cc |
162 | 5 | test_routing.cc | 5 | test_routing.cc |
163 | 6 | DEPENDS | 6 | DEPENDS |
164 | 7 | base_log | ||
165 | 7 | base_macros | 8 | base_macros |
166 | 8 | economy | 9 | economy |
167 | 9 | io_filesystem | 10 | io_filesystem |
168 | 10 | 11 | ||
169 | === modified file 'src/economy/test/test_road.cc' | |||
170 | --- src/economy/test/test_road.cc 2018-04-07 16:59:00 +0000 | |||
171 | +++ src/economy/test/test_road.cc 2018-09-06 15:39:53 +0000 | |||
172 | @@ -21,6 +21,9 @@ | |||
173 | 21 | 21 | ||
174 | 22 | #include <boost/test/unit_test.hpp> | 22 | #include <boost/test/unit_test.hpp> |
175 | 23 | 23 | ||
176 | 24 | #ifdef _WIN32 | ||
177 | 25 | #include "base/log.h" | ||
178 | 26 | #endif | ||
179 | 24 | #include "economy/flag.h" | 27 | #include "economy/flag.h" |
180 | 25 | #include "economy/road.h" | 28 | #include "economy/road.h" |
181 | 26 | #include "io/filesystem/layered_filesystem.h" | 29 | #include "io/filesystem/layered_filesystem.h" |
182 | @@ -51,6 +54,9 @@ | |||
183 | 51 | /*************************************************************************/ | 54 | /*************************************************************************/ |
184 | 52 | struct WlTestFixture { | 55 | struct WlTestFixture { |
185 | 53 | WlTestFixture() { | 56 | WlTestFixture() { |
186 | 57 | #ifdef _WIN32 | ||
187 | 58 | set_logging_dir(); | ||
188 | 59 | #endif | ||
189 | 54 | g_fs = new LayeredFileSystem(); | 60 | g_fs = new LayeredFileSystem(); |
190 | 55 | } | 61 | } |
191 | 56 | ~WlTestFixture() { | 62 | ~WlTestFixture() { |
192 | 57 | 63 | ||
193 | === modified file 'src/main.cc' | |||
194 | --- src/main.cc 2018-04-07 16:59:00 +0000 | |||
195 | +++ src/main.cc 2018-09-06 15:39:53 +0000 | |||
196 | @@ -24,23 +24,17 @@ | |||
197 | 24 | #include <SDL_main.h> | 24 | #include <SDL_main.h> |
198 | 25 | #include <unistd.h> | 25 | #include <unistd.h> |
199 | 26 | 26 | ||
200 | 27 | #include "base/log.h" | ||
201 | 28 | #include "base/wexception.h" | 27 | #include "base/wexception.h" |
202 | 29 | #include "build_info.h" | 28 | #include "build_info.h" |
203 | 30 | #include "config.h" | 29 | #include "config.h" |
204 | 31 | #include "wlapplication.h" | 30 | #include "wlapplication.h" |
205 | 32 | #include "wlapplication_messages.h" | 31 | #include "wlapplication_messages.h" |
206 | 33 | 32 | ||
207 | 34 | using std::cout; | ||
208 | 35 | using std::cerr; | ||
209 | 36 | using std::endl; | ||
210 | 37 | using std::flush; | ||
211 | 38 | |||
212 | 39 | /** | 33 | /** |
213 | 40 | * Cross-platform entry point for SDL applications. | 34 | * Cross-platform entry point for SDL applications. |
214 | 41 | */ | 35 | */ |
215 | 42 | int main(int argc, char* argv[]) { | 36 | int main(int argc, char* argv[]) { |
217 | 43 | log("This is Widelands Version %s (%s)\n", build_id().c_str(), build_type().c_str()); | 37 | std::cout << "This is Widelands Version " << build_id() << " (" << build_type() << ")" << std::endl; |
218 | 44 | 38 | ||
219 | 45 | WLApplication* g_app = nullptr; | 39 | WLApplication* g_app = nullptr; |
220 | 46 | try { | 40 | try { |
221 | @@ -53,7 +47,7 @@ | |||
222 | 53 | return 0; | 47 | return 0; |
223 | 54 | } catch (const ParameterError& e) { | 48 | } catch (const ParameterError& e) { |
224 | 55 | // handle wrong commandline parameters | 49 | // handle wrong commandline parameters |
226 | 56 | cerr << endl << e.what() << endl << endl; | 50 | std::cerr << std::endl << e.what() << std::endl << std::endl; |
227 | 57 | show_usage(build_id(), build_type()); | 51 | show_usage(build_id(), build_type()); |
228 | 58 | delete g_app; | 52 | delete g_app; |
229 | 59 | 53 | ||
230 | @@ -61,20 +55,20 @@ | |||
231 | 61 | } | 55 | } |
232 | 62 | #ifdef NDEBUG | 56 | #ifdef NDEBUG |
233 | 63 | catch (const WException& e) { | 57 | catch (const WException& e) { |
235 | 64 | cerr << "\nCaught exception (of type '" << typeid(e).name() | 58 | std::cerr << "\nCaught exception (of type '" << typeid(e).name() |
236 | 65 | << "') in outermost handler!\nThe exception said: " << e.what() | 59 | << "') in outermost handler!\nThe exception said: " << e.what() |
237 | 66 | << "\n\nThis should not happen. Please file a bug report on version " << build_id() | 60 | << "\n\nThis should not happen. Please file a bug report on version " << build_id() |
238 | 67 | << '(' << build_type() << ')' << ".\n" | 61 | << '(' << build_type() << ')' << ".\n" |
240 | 68 | << "and remember to specify your operating system.\n\n" << flush; | 62 | << "and remember to specify your operating system.\n\n" << std::flush; |
241 | 69 | delete g_app; | 63 | delete g_app; |
242 | 70 | 64 | ||
243 | 71 | return 1; | 65 | return 1; |
244 | 72 | } catch (const std::exception& e) { | 66 | } catch (const std::exception& e) { |
246 | 73 | cerr << "\nCaught exception (of type '" << typeid(e).name() | 67 | std::cerr << "\nCaught exception (of type '" << typeid(e).name() |
247 | 74 | << "') in outermost handler!\nThe exception said: " << e.what() | 68 | << "') in outermost handler!\nThe exception said: " << e.what() |
248 | 75 | << "\n\nThis should not happen. Please file a bug report on version " << build_id() | 69 | << "\n\nThis should not happen. Please file a bug report on version " << build_id() |
249 | 76 | << '(' << build_type() << ')' << ".\n" | 70 | << '(' << build_type() << ')' << ".\n" |
251 | 77 | << "and remember to specify your operating system.\n\n" << flush; | 71 | << "and remember to specify your operating system.\n\n" << std::flush; |
252 | 78 | delete g_app; | 72 | delete g_app; |
253 | 79 | 73 | ||
254 | 80 | return 1; | 74 | return 1; |
255 | 81 | 75 | ||
256 | === modified file 'src/notifications/test/CMakeLists.txt' | |||
257 | --- src/notifications/test/CMakeLists.txt 2017-06-20 12:38:50 +0000 | |||
258 | +++ src/notifications/test/CMakeLists.txt 2018-09-06 15:39:53 +0000 | |||
259 | @@ -2,6 +2,7 @@ | |||
260 | 2 | SRCS | 2 | SRCS |
261 | 3 | notifications_test.cc | 3 | notifications_test.cc |
262 | 4 | DEPENDS | 4 | DEPENDS |
263 | 5 | base_log | ||
264 | 5 | base_macros | 6 | base_macros |
265 | 6 | notifications | 7 | notifications |
266 | 7 | ) | 8 | ) |
267 | 8 | 9 | ||
268 | === modified file 'src/notifications/test/notifications_test.cc' | |||
269 | --- src/notifications/test/notifications_test.cc 2018-04-07 16:59:00 +0000 | |||
270 | +++ src/notifications/test/notifications_test.cc 2018-09-06 15:39:53 +0000 | |||
271 | @@ -23,6 +23,7 @@ | |||
272 | 23 | #define BOOST_TEST_MODULE Notifications | 23 | #define BOOST_TEST_MODULE Notifications |
273 | 24 | #include <boost/test/unit_test.hpp> | 24 | #include <boost/test/unit_test.hpp> |
274 | 25 | 25 | ||
275 | 26 | #include "base/log.h" | ||
276 | 26 | #include "base/macros.h" | 27 | #include "base/macros.h" |
277 | 27 | #include "notifications/notifications.h" | 28 | #include "notifications/notifications.h" |
278 | 28 | 29 | ||
279 | @@ -41,6 +42,10 @@ | |||
280 | 41 | BOOST_AUTO_TEST_SUITE(NotificationsTestSuite) | 42 | BOOST_AUTO_TEST_SUITE(NotificationsTestSuite) |
281 | 42 | 43 | ||
282 | 43 | BOOST_AUTO_TEST_CASE(SimpleTest) { | 44 | BOOST_AUTO_TEST_CASE(SimpleTest) { |
283 | 45 | #ifdef _WIN32 | ||
284 | 46 | set_logging_dir(); | ||
285 | 47 | #endif | ||
286 | 48 | |||
287 | 44 | std::vector<SimpleNote> received1; | 49 | std::vector<SimpleNote> received1; |
288 | 45 | auto subscriber1 = Notifications::subscribe<SimpleNote>( | 50 | auto subscriber1 = Notifications::subscribe<SimpleNote>( |
289 | 46 | [&received1](const SimpleNote& got) { received1.push_back(got); }); | 51 | [&received1](const SimpleNote& got) { received1.push_back(got); }); |
290 | 47 | 52 | ||
291 | === modified file 'src/wlapplication.cc' | |||
292 | --- src/wlapplication.cc 2018-07-29 11:27:33 +0000 | |||
293 | +++ src/wlapplication.cc 2018-09-06 15:39:53 +0000 | |||
294 | @@ -223,21 +223,32 @@ | |||
295 | 223 | 223 | ||
296 | 224 | } // namespace | 224 | } // namespace |
297 | 225 | 225 | ||
298 | 226 | |||
299 | 227 | // Set up the homedir. Exit 1 if the homedir is illegal or the logger couldn't be initialized for Windows. | ||
300 | 226 | void WLApplication::setup_homedir() { | 228 | void WLApplication::setup_homedir() { |
304 | 227 | // If we don't have a home directory don't do anything | 229 | // If we don't have a home directory, we exit with an error |
305 | 228 | if (homedir_.size()) { | 230 | if (homedir_.empty()) { |
306 | 229 | // Assume some dir exists | 231 | std::cout << "Unable to start Widelands, because the given homedir is empty" << std::endl; |
307 | 232 | delete g_fs; | ||
308 | 233 | exit(1); | ||
309 | 234 | } else { | ||
310 | 230 | try { | 235 | try { |
311 | 231 | log("Set home directory: %s\n", homedir_.c_str()); | ||
312 | 232 | |||
313 | 233 | std::unique_ptr<FileSystem> home(new RealFSImpl(homedir_)); | 236 | std::unique_ptr<FileSystem> home(new RealFSImpl(homedir_)); |
314 | 234 | home->ensure_directory_exists("."); | 237 | home->ensure_directory_exists("."); |
315 | 235 | g_fs->set_home_file_system(home.release()); | 238 | g_fs->set_home_file_system(home.release()); |
316 | 236 | } catch (const std::exception& e) { | 239 | } catch (const std::exception& e) { |
321 | 237 | log("Failed to add home directory: %s\n", e.what()); | 240 | std::cout << "Unable to start Widelands, because we were unable to add the home directory: " << e.what() << std::endl; |
322 | 238 | } | 241 | delete g_fs; |
323 | 239 | } else { | 242 | exit(1); |
324 | 240 | // TODO(unknown): complain | 243 | } |
325 | 244 | #ifdef _WIN32 | ||
326 | 245 | // Initialize the logger for Windows. Exit on failure. | ||
327 | 246 | if (!set_logging_dir(homedir_)) { | ||
328 | 247 | delete g_fs; | ||
329 | 248 | exit(1); | ||
330 | 249 | } | ||
331 | 250 | #endif | ||
332 | 251 | log("Set home directory: %s\n", homedir_.c_str()); | ||
333 | 241 | } | 252 | } |
334 | 242 | } | 253 | } |
335 | 243 | 254 |
I have tested the AppVeyor release build on Windows 10. The installer had to be run as administrator when installed into C:\Program Files, the game can then be run without admin privileges. stdout.txt is written to C:\Users\ <username> \.widelands