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: Changed 10 years ago by pabs

comment:2 follow-up: 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: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: 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://stackoverflow.com/questions/16098509/automake-1-12-changes-bison-yacc-output-names-backwards-incompatible-change

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

In 63b1860973138ad8d0b4642599bce874cf72ac28:

Correct oversight of not including original bison/flex files for distribution.

patch by clueless
closes ticket:4151
refs ticket:4259
Note: See TracTickets for help on using tickets.