Have cmake and scons use same lists of source files #882
Conversation
I think you can reopen prs? By opening a new pr you somwhow 'hide' all previous discussionon on it. |
|
I looked, but I couldn't find any sort of "re-open PR" button, and pushing the above commit to the branch did not show up in the old PR nor did it re-open it. I did link to the previous PR though, so the discussion isn't exactly hidden. |
|
Is the file |
|
My checks from a month or two ago showed gui/widget/debug.cpp was not referenced. I never noticed it was compiled by CMake. |
|
It seems like something you'd only want in a special debug build. Even if you compiled it, it wouldn't have any effect without the appropriate -D. |
|
So should I keep it as part of the build, or remove it? |
|
I have left The tools I haven't tried building, since it seems they have their own issues right now as per #874, but copy/pasting:
into an otherwise unmodified and up-to-date local repository results in a successful compilation for both cmake and scons. This branch itself won't compile though, due to a recent change which, among other things, renamed |
|
Feel free to ignore my PR. If yours causes a merge conflict on any of mine, I'll fix 'em. The issues are that that either they fail to link or too much dead code is needed to get them to link. |
|
@GregoryLundberg Yeah, git is already reporting this as causing a merge conflict with |
|
So, unless anyone else sees any issues/has any suggestions, I don't have anything else to add to this PR. |
Pentarctagon
changed the title
| @@ -6,42 +6,42 @@ from glob import glob | |||
|
|
|||
| Import("*") | |||
|
|
|||
| # | |||
| # load list of sources from the file in scons/sources/ | |||
There was a problem hiding this comment.
This comment is inaccurate?
There was a problem hiding this comment.
Whoops, that where it was when I first submitted this and it was just for scons. Will fix.
| test = test_env.WesnothProgram("test", test_sources + [libwesnoth_extras, libwesnoth_core, libwesnoth, libwesnoth_sdl, libwesnoth_extras], have_test_prereqs) | ||
| #---end valid scons targets---# | ||
|
|
||
| #---start invalid scons targets---# |
There was a problem hiding this comment.
What do you mean by "invalid" here...
There was a problem hiding this comment.
in the root SConstruct, the default_target variable is specified as only accepting the values wesnoth wesnothd campaignd cutter exploder test.
|
So, I think this is a good idea and should be merged, but the conflicts need to be resolved. Presumably that's not too difficult to do? (Most likely they're from added/removed/moved files since the PR was opened.) |
|
hold off until I fix my PR it's looking like this and @Vultraz latest in this area will render it moot but there may be a tweak or two needed here |
|
@CelticMinstrel The conflicts are, I believe, that the lists of source files in |
|
@CelticMinstrel Also, I suppose I should add, I have no idea how to resolve merge conflicts with git. I think it's supposed to involve |
|
One needs to rebase the branch. You resolve conflicts while doing that. However, GitHub now has a Resolve Conflicts button. |
|
Just to be sure, there have been no CMake/scons changes since this PR was submitted, right? Other than the source lists. If that's the case, then resolving the conflicts means simply ignoring the changes on the master side of the merge. |
Move lists of source files into their own directory Make scons use the lists of source files Move lua build step to src/SConscript and delete src/lua/SConscript Make cmake use the separate file with the list of lua sources to build
scons move source files from libraries that don't appear in cmake over to existing source lists - libcampaignd, libcutter, libdummy_video, libtest_utils add the defines FIFODIR and WESNOTH_PATH to all compiled source files, as cmake does, and move the single files those defines had previously been defined on into the source lists remove WESNOTH_PREFIX, as not used added schema_validator sources from cmake added OBJPREFIX to - cutter, exploder, campaignd, schema_generator, schema_validator cmake change over to using the source lists move source files from libraries that don't appear in scons over to existing source lists - wesnoth-gui_types, wesnoth-gui_event, wesnoth-gui_iterator, wesnoth-gui_placer, wesnoth-gui_tooltip, wesnoth-gui_widget, wesnoth-gui_widget_definition, wesnoth-gui1_widgets, wesnoth-schema_validator
|
Alright, I'm going to double-check a few things, but I think I managed to do a rebase without horribly breaking the PR this time. @CelticMinstrel With the exception of this one, yes. Though in that case I had already removed scons' |
|
Okay, just waiting to see what @GregoryLundberg has to say, I guess. |
Move lists of source files into their own directory
Make scons use the lists of source files
Move lua build step to src/SConscript and delete src/lua/SConscript
Make cmake use the separate file with the list of lua sources to build
@CelticMinstrel @GregoryLundberg @gfgtdf This is essentially a re-submission of #879 due to me screwing stuff up enough with git while trying to fix something that I decided to just try deleting all the commits, and then resubmitting them. Unfortunately, a PR with no commits is automatically closed, so I renamed the branch to something more fitting, and made this PR.