Opened 10 years ago
Closed 9 years ago
#4151 closed bug (fixed)
[regression] fails to build from source after deleting generated files
| Reported by: | pabs | Owned by: | vexed |
|---|---|---|---|
| Priority: | normal | Milestone: | unspecified |
| Component: | Build System Issues | Version: | 3.1.1 |
| Keywords: | Cc: | ||
| Blocked By: | Blocking: | ||
| Operating System: | All/Non-Specific |
Description
When deleting generated files (inc lib/framework/resource_lexer.cpp) I get this error:
make[4]: *** No rule to make target `resource_lexer.o', needed by `libframework.a'. Stop.
This is a regression from 3.1.0, here is the diff from the previous version, please revert it:
--- warzone2100-3.1.0/lib/framework/Makefile.am 2013-01-15 07:54:07.000000000 +0800 +++ warzone2100-3.1.1/lib/framework/Makefile.am 2013-11-21 11:00:51.000000000 +0800 @@ -1,8 +1,6 @@ AM_CPPFLAGS = -DYY_NO_INPUT $(SDL_CFLAGS) $(WZ_CPPFLAGS) $(QT4_CFLAGS) AM_CFLAGS = $(WZ_CFLAGS) AM_CXXFLAGS = $(WZ_CXXFLAGS) $(QT4_CFLAGS) -AM_LFLAGS = $(FLEX_FLAGS) -AM_YFLAGS = -d MOCHEADER = \ wzconfig.h @@ -11,9 +9,6 @@ %_moc.cpp: %.h $(MOC4) -o $@ $< -BUILT_SOURCES = \ - resource_parser.h \ - strres_parser.h CLEANFILES = \ $(MOCEDFILES) @@ -46,9 +41,11 @@ physfs_ext.h \ rational.h \ resly.h \ + resource_parser.h \ stdio_ext.h \ string_ext.h \ strres.h \ + strres_parser.h \ strresly.h \ treap.h \ trig.h \ @@ -70,12 +67,12 @@ geometry.cpp \ i18n.cpp \ lexer_input.cpp \ - resource_lexer.lpp \ - resource_parser.ypp \ + resource_lexer.cpp \ + resource_parser.cpp \ stdio_ext.cpp \ strres.cpp \ - strres_lexer.lpp \ - strres_parser.ypp \ + strres_lexer.cpp \ + strres_parser.cpp \ treap.cpp \ trig.cpp \ utf.cpp \
Change History (20)
comment:1 follow-up: ↓ 15 Changed 10 years ago by pabs
comment:2 follow-up: ↓ 3 Changed 10 years ago by pabs
This also means that any distributors of warzone2100 3.1.1 binary packages based solely on warzone2100 3.1.1 released tarballs are violating the GNU GPL. I can't upload this to Debian until this is fixed.
comment:3 in reply to: ↑ 2 Changed 10 years ago by clueless
Replying to pabs:
This also means that any distributors of warzone2100 3.1.1 binary packages based solely on warzone2100 3.1.1 released tarballs are violating the GNU GPL. I can't upload this to Debian until this is fixed.
I find this and odd statement, in what way does it violate gpl?
comment:4 Changed 10 years ago by clueless
I mean I built it from source from the tarball and all the files in the tarball are source files not binary blobs like closed source releases.
the game works so nothing is missing so I don't see where there is a gpl violation
comment:5 Changed 10 years ago by pabs
Your patch to remove the bison/flex dependencies also removed some of the source code from the tarball. The files (bison/flex source) listed below are the source code of the C++ files that were generated from them. They are missing from the tarball and therefore anyone distributing only the tarball is not distributing the complete source and therefore violating the GPL when they build and distribute binaries built from that source.
In addition, even if distributors manually copy the files listed below into their distributions, your build system is missing the parts needed to convert the bison/flex code to C++ code, which is required as per GPLv2 item 3 ("plus the scripts used to control compilation and installation of the executable").
Here is a general guide to source issues, once I've fixed my account issues on that wiki I will fix it to mention bison/yacc/flex.
http://www.freedesktop.org/wiki/Games/Upstream/#source
The reason that the game still builds and works is that you committed generated C++ code to your version control system and still ship it in the tarball but don't ship the bison/flex source for it in the tarball. So your statement "all the files in the tarball are source files" is incorrect.
This is very similar to the GNU Emacs GPL violations caused by Emacs upstream a few years ago:
https://lwn.net/Articles/453374/
https://lwn.net/Articles/453970/
lib/framework/resource_lexer.lpp lib/framework/resource_parser.ypp lib/framework/strres_lexer.lpp lib/framework/strres_parser.ypp lib/gamelib/audp_lexer.lpp lib/gamelib/audp_parser.ypp lib/script/chat_lexer.lpp lib/script/chat_parser.ypp lib/script/script_lexer.lpp lib/script/script_parser.ypp src/level_lexer.lpp src/scriptvals_lexer.lpp src/scriptvals_parser.ypp
comment:6 Changed 10 years ago by clueless
sorry? what patch? I didn't include a patch?
comment:7 Changed 10 years ago by pabs
comment:8 Changed 10 years ago by clueless
oh, by stating 'your patch' I thought you were talking about me, not the devs
comment:9 follow-up: ↓ 17 Changed 10 years ago by pabs
I thought you were one of the devs, the one who wrote the patches in fact.
comment:10 Changed 10 years ago by clueless
looks like a simple oversite.
for the problem all they need to do is add them since I see them in the repo
EXTRA_DIST= \ lib/framework/resource_lexer.lpp \ lib/framework/resource_parser.ypp \ lib/framework/strres_lexer.lpp \ lib/framework/strres_parser.ypp \ lib/gamelib/audp_lexer.lpp \ lib/gamelib/audp_parser.ypp \ lib/script/chat_lexer.lpp \ lib/script/chat_parser.ypp \ lib/script/script_lexer.lpp \ lib/script/script_parser.ypp \ src/level_lexer.lpp \ src/scriptvals_lexer.lpp \ src/scriptvals_parser.ypp \ build_tools/autorevision \ autogen.sh \ config.rpath \ configure.ac \ pkg/dpkg \ macosx \ 3rdparty/SDL/mac \ 3rdparty/glm \ tools/image/image.cpp \ tools/image/configs \ tools/image/Image.xcodeproj \ po/custom/mac-infoplist.txt \ po/custom/warzone2100.desktop.txt
comment:11 Changed 10 years ago by pabs
Personally I would suggest reverting the commit entirely. Adding generated code/data to VCSes is a recipe for disaster.
comment:12 Changed 10 years ago by clueless
I did a quick test to see what happens if you revert that and it looks like autotools fails unless you go back to a version that those commits talk about.
they should use CMAKE for the build system, I have no clue how to fix autotools errors.
maybe this is why the version in Ubuntu is so very old?
I also noticed they got some other build files that are not included in the tarball and nobody told them that they are missing before but it seems those are for macs and maybe they are not needed?
comment:13 Changed 10 years ago by pabs
The Ubuntu version is old because they just copy the Debian package and Debian hasn't updated it yet. I'm the maintainer of the Debian package and I haven't updated it yet because of this issue, other issues and lack of time.
comment:14 Changed 10 years ago by clueless
ah cool! nice to meet you!
We just got this game 2 days ago and thought I would help them out with some bug reports and observations to help them improve things.
comment:15 in reply to: ↑ 1 Changed 10 years ago by cybersphinx
Replying to pabs:
Looks like this commit is at fault, please revert it:
https://github.com/Warzone2100/warzone2100/commit/d13bc0de7dd0588351af24dc45d55110d0eed6e8.patch
As mentioned in the referenced ticket #3538, automake made an incompatible change in version 1.12. Do you know a way to make the build system support automake 1.11 (which is at least in the current stable Debian and LTS Ubuntu) and 1.12+ while using flex/yacc?
comment:16 Changed 10 years ago by pabs
A 5 minute web search found these ways:
https://lists.gnu.org/archive/html/automake/2012-07/msg00048.html
The second one looks cleaner to me, it checks the automake version in autogen.sh and for older automake it creates a compatability layer. This needs to be repeated for each bison parser.
comment:17 in reply to: ↑ 9 Changed 10 years ago by vexed
Replying to pabs:
Personally I would suggest reverting the commit entirely. Adding generated code/data to VCSes is a recipe for disaster.
Whoops, yeah, this is a minor oversight, and will be corrected as soon as someone gets some free time to make a new tarball.
However, reverting doesn't solve any of the issues talked about in the original patch.
All bison/flex stuff is basically deprecated for all intensive purposes (you can read that as, those files will not have any code changes done to them, and are being replaced.), and caused more grief than they are worth on the multiple build systems we support. We wanted all builds that we support to have the same code, and that was not the case with multiple versions of bison/flex that produced garbage on various platforms.
So, no, no reverting those commits. Yes, a new tarball will be made.
Clueless, thanks for the patch.
We did have a few people that tried using CMake, but that got nowhere, feel free to try it yourself.
Sidenote, trac isn't meant for 'small talk', keep that to the forums please.
Replying to pabs:
I thought you were one of the devs, the one who wrote the patches in fact.
… sigh
comment:18 Changed 10 years ago by pabs
See comment:16 for a simple method to solve the issues mentioned in the original patch (automake 1.12 compat issues). If there are other issues, I failed to find them, sorry.
By "make a new tarball" I hope you mean make a new 3.1.2 release, never a good idea to modify a tarball after release.
What do you mean by "are being replaced"? It would be interesting to read your plans in this area.
Did you report bugs about the bison/flex code generation issues you encountered?
comment:19 Changed 10 years ago by vexed
Because of the lack of free time, and for the time being, I have uploaded the inadvertent missing files to warzone2100-3.1.1_extra_dist.tar.xz @ our normal host @ SF.
There was no script that was used, it was all done by hand, since we had to fix the files for all build systems that we support.
As for reporting bugs, latest version of Bison & flex for windows is way behind what is available on linux distros. So, besides asking nicely for a new version… no. As I mentioned, we want all supported platforms to use the exact same code, so, it isn't a option to use different versions on different platforms when the output is different.
They are being replaced, as in, we are phasing it out. Most everything will be moved to JS instead, or outright avoided (like lazy texture loading) but this is getting outside the scope of this ticket, please take it to forums for development discussions.
comment:20 Changed 9 years ago by vexed
- Owner set to vexed
- Resolution set to fixed
- Status changed from new to closed
Looks like this commit is at fault, please revert it:
https://github.com/Warzone2100/warzone2100/commit/d13bc0de7dd0588351af24dc45d55110d0eed6e8.patch