[Makefile] `-fPIC` not added to externally defined `C[XX]FLAGS` of PLUGINS if `Make.config` not available
Dear VDR users,
in `Make.config.template` [1] which if renamed to `Make.config` gets included in all Makefiles there is
ifdef PLUGIN CFLAGS += -fPIC CXXFLAGS += -fPIC DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE endif
which just gets included if `PLUGIN` is defined, which is normally done in the Makefile of a plugin as for example in `Makefile` [3] belonging to the plugin hello [2].
Additionally in each Makefile of a plugin `C[XX]FLAGS` is set to
CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
if it has not been defined yet for example in the environment.
As far as I understand plugins have to be compiled with the `-fPIC` flag, so that the VDR can be linked against them [4]. Otherwise it would give an error message as the following example.
ld: hello.o: relocation R_ARM_MOVW_ABS_NC against `a local symbol' can not be used when making a shared object; recompile with -fPIC hello.o: could not read symbols: Bad value
The problem now is, that if `Make.config` does not contain the snippet from the top and `C[XX]FLAGS` are defined before without `-fPIC` building the plugins fails. This scenario is typical in cross compilation [5].
I do not know if the `DEFINES` from above are required, so I just concentrate on the `-fPIC` issue because this flag is needed for a successful build.
I thought of two solutions. Maybe you see something better.
1. Each `Makefile` of a plugin gets rewritten to always append `-fPIC` to `C[XX]FLAGS`. Here is an example for the plugin hello.
diff --git a/PLUGINS/src/hello/Makefile b/PLUGINS/src/hello/Makefile index ea5b806..a02d6c2 100644 --- a/PLUGINS/src/hello/Makefile +++ b/PLUGINS/src/hello/Makefile @@ -18,7 +18,8 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri ### The C++ compiler and options:
CXX ?= g++ -CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses +CXXFLAGS ?= -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses +CXXFLAGS += -fPIC
### The directory environment:
2. If `DEFINES` from the beginning is also needed, that we should factor the snippet out into a file `Make.plugins` and every plugin has to include it in its Makefile.
What do you think? What alternative is preferable? When this is decided I would create a patch to change that in VDR.
Thanks,
Paul
[1] http://git.gekrumbel.de/vdr.git?p=vdr.git;a=blob;f=Make.config.template [2] http://git.gekrumbel.de/vdr.git?p=vdr.git;a=tree;f=PLUGINS/src/hello [3] http://git.gekrumbel.de/vdr.git?p=vdr.git;a=blob;f=PLUGINS/src/hello/Makefil... [4] http://www.gentoo.org/proj/en/base/amd64/howtos/index.xml?part=1&chap=3 [5] http://lists.linuxtogo.org/pipermail/openembedded-devel/2010-January/016213....
On Mon, 25 Jan 2010 23:43:11 +0100, Paul Menzel wrote
- Each `Makefile` of a plugin gets rewritten to always append `-
fPIC` to `C[XX]FLAGS`. Here is an example for the plugin hello.
- If `DEFINES` from the beginning is also needed, that we should factor
the snippet out into a file `Make.plugins` and every plugin has to include it in its Makefile.
What do you think? What alternative is preferable? When this is decided I would create a patch to change that in VDR.
This has already been discussed during the last months, I just didn't take the time yet to propose a fix:
http://www.linuxtv.org/pipermail/vdr/2009-July/020977.html http://www.linuxtv.org/pipermail/vdr/2009-December/021807.html
Best regards, Frank
Am Dienstag, den 26.01.2010, 10:34 +0100 schrieb Frank Schmirler:
On Mon, 25 Jan 2010 23:43:11 +0100, Paul Menzel wrote
- Each `Makefile` of a plugin gets rewritten to always append `-
fPIC` to `C[XX]FLAGS`. Here is an example for the plugin hello.
- If `DEFINES` from the beginning is also needed, that we should factor
the snippet out into a file `Make.plugins` and every plugin has to include it in its Makefile.
What do you think? What alternative is preferable? When this is decided I would create a patch to change that in VDR.
This has already been discussed during the last months, I just didn't take the time yet to propose a fix:
http://www.linuxtv.org/pipermail/vdr/2009-July/020977.html http://www.linuxtv.org/pipermail/vdr/2009-December/021807.html
Thank you for the links. I could not figure out what alternativ is preferred though. If you tell me, I could prepare a patch.
Thanks,
Paul
On Tue, 26 Jan 2010 10:54:00 +0100, Paul Menzel wrote
Am Dienstag, den 26.01.2010, 10:34 +0100 schrieb Frank Schmirler:
This has already been discussed during the last months, I just didn't take the time yet to propose a fix:
http://www.linuxtv.org/pipermail/vdr/2009-July/020977.html http://www.linuxtv.org/pipermail/vdr/2009-December/021807.html
Thank you for the links. I could not figure out what alternativ is preferred though. If you tell me, I could prepare a patch.
Well, there hasn't been much feedback... At first I preferred the "copy Make.config.template to Make.config if it doesn't exist" approach as it doesn't require any change in plugin Makefiles and adds just a single line to VDR's Makefile. But the cleaner solution is clearly "include a Make.global in plugin plugin and VDR Makefiles before Make.config" as preferred by Udo Richter.
Would be great if you could prepare the patch.
Cheers, Frank
On tiistai, 26. tammikuuta 2010 12:09:11 Frank Schmirler wrote:
On Tue, 26 Jan 2010 10:54:00 +0100, Paul Menzel wrote
Am Dienstag, den 26.01.2010, 10:34 +0100 schrieb Frank Schmirler:
This has already been discussed during the last months, I just didn't take the time yet to propose a fix:
http://www.linuxtv.org/pipermail/vdr/2009-July/020977.html http://www.linuxtv.org/pipermail/vdr/2009-December/021807.html
Thank you for the links. I could not figure out what alternativ is preferred though. If you tell me, I could prepare a patch.
Well, there hasn't been much feedback... At first I preferred the "copy Make.config.template to Make.config if it doesn't exist" approach as it doesn't require any change in plugin Makefiles and adds just a single line to VDR's Makefile. But the cleaner solution is clearly "include a Make.global in plugin plugin and VDR Makefiles before Make.config" as preferred by Udo Richter.
I also prefer this approach. Make.global should contain the necessary stuff and Make.config should contain strictly user-defined optional stuff.
Would be great if you could prepare the patch.
Without this patch, if some options in `Makefile` were set outside `Makefile` and no `Make.config` existed with the necessary options, builds could fail. [1][2][3]
Therefore include strictly necessary options in `Make.global` and include this in all the Makefiles before `Make.config`.
This patch does not delete the those options which were factored out into `Make.global` to make it easier for people to understand the Makefile and what options are needed.
[1] http://www.linuxtv.org/pipermail/vdr/2009-July/020977.html [2] http://www.linuxtv.org/pipermail/vdr/2009-December/021807.html [3] http://www.linuxtv.org/pipermail/vdr/2010-January/022235.html
Signed-off-by: Paul Menzel paulepanter@users.sourceforge.net --- Make.config.template | 6 ------ Make.global | 14 ++++++++++++++ Makefile | 1 + PLUGINS/src/dvbsddevice/Makefile | 4 ++++ PLUGINS/src/hello/Makefile | 4 ++++ PLUGINS/src/osddemo/Makefile | 4 ++++ PLUGINS/src/pictures/Makefile | 4 ++++ PLUGINS/src/servicedemo/Makefile | 4 ++++ PLUGINS/src/skincurses/Makefile | 4 ++++ PLUGINS/src/status/Makefile | 4 ++++ PLUGINS/src/svdrpdemo/Makefile | 4 ++++ 11 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 Make.global
diff --git a/Make.config.template b/Make.config.template index 758fc14..3296992 100644 --- a/Make.config.template +++ b/Make.config.template @@ -16,12 +16,6 @@ CFLAGS = -g -O2 -Wall CXX = g++ CXXFLAGS = -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
-ifdef PLUGIN -CFLAGS += -fPIC -CXXFLAGS += -fPIC -DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -endif - ### The directory environment:
#DVBDIR = /usr/src/v4l-dvb/linux diff --git a/Make.global b/Make.global new file mode 100644 index 0000000..5ef70f0 --- /dev/null +++ b/Make.global @@ -0,0 +1,14 @@ +# +# Strictly necessary Makefile options for the Video Disk Recorder +# +# See the main source file 'vdr.c' for copyright information and +# how to reach the author. + +# Plugins need to be compiled with position independent code, otherwise linking +# VDR against it will fail. +ifdef PLUGIN +CFLAGS += -fPIC +CXXFLAGS += -fPIC +endif + +DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE diff --git a/Makefile b/Makefile index e13ea5e..faa36e7 100644 --- a/Makefile +++ b/Makefile @@ -32,6 +32,7 @@ CONFDIR = $(VIDEODIR) DOXYGEN = /usr/bin/doxygen DOXYFILE = Doxyfile
+-include Make.global -include Make.config
SILIB = $(LSIDIR)/libsi.a diff --git a/PLUGINS/src/dvbsddevice/Makefile b/PLUGINS/src/dvbsddevice/Makefile index 8ef273c..4eeec31 100644 --- a/PLUGINS/src/dvbsddevice/Makefile +++ b/PLUGINS/src/dvbsddevice/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +-include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/hello/Makefile b/PLUGINS/src/hello/Makefile index ea5b806..67a54d5 100644 --- a/PLUGINS/src/hello/Makefile +++ b/PLUGINS/src/hello/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +-include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/osddemo/Makefile b/PLUGINS/src/osddemo/Makefile index 1b1c622..802f678 100644 --- a/PLUGINS/src/osddemo/Makefile +++ b/PLUGINS/src/osddemo/Makefile @@ -18,6 +18,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +-include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/pictures/Makefile b/PLUGINS/src/pictures/Makefile index 46a262f..8305cd3 100644 --- a/PLUGINS/src/pictures/Makefile +++ b/PLUGINS/src/pictures/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +-include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/servicedemo/Makefile b/PLUGINS/src/servicedemo/Makefile index ea7e66a..9113217 100644 --- a/PLUGINS/src/servicedemo/Makefile +++ b/PLUGINS/src/servicedemo/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN1).c | awk '{ pr CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +-include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/skincurses/Makefile b/PLUGINS/src/skincurses/Makefile index cade795..42934d7 100644 --- a/PLUGINS/src/skincurses/Makefile +++ b/PLUGINS/src/skincurses/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +-include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/status/Makefile b/PLUGINS/src/status/Makefile index 81d4163..543b2ef 100644 --- a/PLUGINS/src/status/Makefile +++ b/PLUGINS/src/status/Makefile @@ -18,6 +18,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +-include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/svdrpdemo/Makefile b/PLUGINS/src/svdrpdemo/Makefile index c835f3c..53fa8a5 100644 --- a/PLUGINS/src/svdrpdemo/Makefile +++ b/PLUGINS/src/svdrpdemo/Makefile @@ -18,6 +18,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +-include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../..
On Thursday 28 January 2010, Paul Menzel wrote:
Therefore include strictly necessary options in `Make.global` and include this in all the Makefiles before `Make.config`.
If these options are strictly necessary, shouldn't the leading "-" be dropped from all "-include $(VDRDIR)/Make.global" lines?
Without this patch, if some options in `Makefile` were set outside `Makefile` and no `Make.config` existed with the necessary options, builds could fail. [1][2][3]
Therefore include strictly necessary options in `Make.global` and include this in all the Makefiles before `Make.config`.
This patch does not delete the those options which were factored out into `Make.global` to make it easier for people to understand the Makefile and what options are needed.
[1] http://www.linuxtv.org/pipermail/vdr/2009-July/020977.html [2] http://www.linuxtv.org/pipermail/vdr/2009-December/021807.html [3] http://www.linuxtv.org/pipermail/vdr/2010-January/022235.html
v2: Use `include` instead of `-include` to get a warning/error if it is not present. [4]
[4] http://www.gnu.org/software/make/manual/make.html#Include
Signed-off-by: Paul Menzel paulepanter@users.sourceforge.net --- Make.config.template | 6 ------ Make.global | 14 ++++++++++++++ Makefile | 1 + PLUGINS/src/dvbsddevice/Makefile | 4 ++++ PLUGINS/src/hello/Makefile | 4 ++++ PLUGINS/src/osddemo/Makefile | 4 ++++ PLUGINS/src/pictures/Makefile | 4 ++++ PLUGINS/src/servicedemo/Makefile | 4 ++++ PLUGINS/src/skincurses/Makefile | 4 ++++ PLUGINS/src/status/Makefile | 4 ++++ PLUGINS/src/svdrpdemo/Makefile | 4 ++++ 11 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 Make.global
diff --git a/Make.config.template b/Make.config.template index 758fc14..3296992 100644 --- a/Make.config.template +++ b/Make.config.template @@ -16,12 +16,6 @@ CFLAGS = -g -O2 -Wall CXX = g++ CXXFLAGS = -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
-ifdef PLUGIN -CFLAGS += -fPIC -CXXFLAGS += -fPIC -DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -endif - ### The directory environment:
#DVBDIR = /usr/src/v4l-dvb/linux diff --git a/Make.global b/Make.global new file mode 100644 index 0000000..5ef70f0 --- /dev/null +++ b/Make.global @@ -0,0 +1,14 @@ +# +# Strictly necessary Makefile options for the Video Disk Recorder +# +# See the main source file 'vdr.c' for copyright information and +# how to reach the author. + +# Plugins need to be compiled with position independent code, otherwise linking +# VDR against it will fail. +ifdef PLUGIN +CFLAGS += -fPIC +CXXFLAGS += -fPIC +endif + +DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE diff --git a/Makefile b/Makefile index e13ea5e..faa36e7 100644 --- a/Makefile +++ b/Makefile @@ -32,6 +32,7 @@ CONFDIR = $(VIDEODIR) DOXYGEN = /usr/bin/doxygen DOXYFILE = Doxyfile
+include Make.global -include Make.config
SILIB = $(LSIDIR)/libsi.a diff --git a/PLUGINS/src/dvbsddevice/Makefile b/PLUGINS/src/dvbsddevice/Makefile index 8ef273c..a3854a5 100644 --- a/PLUGINS/src/dvbsddevice/Makefile +++ b/PLUGINS/src/dvbsddevice/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/hello/Makefile b/PLUGINS/src/hello/Makefile index ea5b806..1132cc9 100644 --- a/PLUGINS/src/hello/Makefile +++ b/PLUGINS/src/hello/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/osddemo/Makefile b/PLUGINS/src/osddemo/Makefile index 1b1c622..54dc1e0 100644 --- a/PLUGINS/src/osddemo/Makefile +++ b/PLUGINS/src/osddemo/Makefile @@ -18,6 +18,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/pictures/Makefile b/PLUGINS/src/pictures/Makefile index 46a262f..6f95238 100644 --- a/PLUGINS/src/pictures/Makefile +++ b/PLUGINS/src/pictures/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/servicedemo/Makefile b/PLUGINS/src/servicedemo/Makefile index ea7e66a..e36757a 100644 --- a/PLUGINS/src/servicedemo/Makefile +++ b/PLUGINS/src/servicedemo/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN1).c | awk '{ pr CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/skincurses/Makefile b/PLUGINS/src/skincurses/Makefile index cade795..4a7a72f 100644 --- a/PLUGINS/src/skincurses/Makefile +++ b/PLUGINS/src/skincurses/Makefile @@ -20,6 +20,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/status/Makefile b/PLUGINS/src/status/Makefile index 81d4163..51281d3 100644 --- a/PLUGINS/src/status/Makefile +++ b/PLUGINS/src/status/Makefile @@ -18,6 +18,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../.. diff --git a/PLUGINS/src/svdrpdemo/Makefile b/PLUGINS/src/svdrpdemo/Makefile index c835f3c..053622e 100644 --- a/PLUGINS/src/svdrpdemo/Makefile +++ b/PLUGINS/src/svdrpdemo/Makefile @@ -18,6 +18,10 @@ VERSION = $(shell grep 'static const char *VERSION *=' $(PLUGIN).c | awk '{ pri CXX ?= g++ CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses
+### Make sure that necessary options are included: + +include $(VDRDIR)/Make.global + ### The directory environment:
VDRDIR = ../../..
Hi Paul,
thanks for the patch - looks good. Some suggestions from me:
1) from VDR's Makefile, remove the line DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE It is already included from Make.global
2) in all plugin Makefiles, remove -fPIC from the line CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses It will be added by Make.global anyway
3) in Make.config.template, remove only the line DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE The lines with "+= -fPIC" are still necessary, as Make.config resets CFLAGS/CXXFLAGS.
4) Script newplugin needs to be modified, too.
Best regards, Frank
Dear Frank,
Am Freitag, den 29.01.2010, 10:04 +0100 schrieb Frank Schmirler:
[…]
- from VDR's Makefile, remove the line
DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE It is already included from Make.global
- in all plugin Makefiles, remove -fPIC from the line
CXXFLAGS ?= -fPIC -g -O2 -Wall -Woverloaded-virtual -Wno-parentheses It will be added by Make.global anyway
My reasoning was that someone reading the Makefile would have it easier to see what options are needed. I will change it now.
I will also remove the line `DEFINES += -D_FILE …` in there too because it should be added by `Make.global` too.
- in Make.config.template, remove only the line
DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE The lines with "+= -fPIC" are still necessary, as Make.config resets CFLAGS/CXXFLAGS.
Correct.
- Script newplugin needs to be modified, too.
I did not know about `newplugin`.
Thanks for the review. I will send a new version of the patch,
Paul
Am 28.01.2010 22:52, schrieb Ville Skyttä:
On Thursday 28 January 2010, Paul Menzel wrote:
Therefore include strictly necessary options in `Make.global` and include this in all the Makefiles before `Make.config`.
If these options are strictly necessary, shouldn't the leading "-" be dropped from all "-include $(VDRDIR)/Make.global" lines?
In this case, yes, but many cross-version plugins will need to be more flexible here, either by doing a messy VDR version check, or by simply just using -inlcude anyway.
Cheers,
Udo
Am Samstag, den 30.01.2010, 11:57 +0100 schrieb Udo Richter:
Am 28.01.2010 22:52, schrieb Ville Skyttä:
On Thursday 28 January 2010, Paul Menzel wrote:
Therefore include strictly necessary options in `Make.global` and include this in all the Makefiles before `Make.config`.
If these options are strictly necessary, shouldn't the leading "-" be dropped from all "-include $(VDRDIR)/Make.global" lines?
In this case, yes, but many cross-version plugins will need to be more flexible here, either by doing a messy VDR version check, or by simply just using -inlcude anyway.
Sorry, I do not know if I understood you correctly. Do you mean that some other plugins (which are not shipped with VDR, i. e. the patch does not need to be changed in that regard) need to use `-include $(VDRDIR)/Make.global` instead of `include $(VDRDIR)/Make.global`?
Do the authors of those plugins generally know about that kind of stuff or does this have to be documented somewhere?
Thanks,
Paul
Am 30.01.2010 12:12, schrieb Paul Menzel:
Sorry, I do not know if I understood you correctly. Do you mean that some other plugins (which are not shipped with VDR, i. e. the patch does not need to be changed in that regard) need to use `-include $(VDRDIR)/Make.global` instead of `include $(VDRDIR)/Make.global`?
Do the authors of those plugins generally know about that kind of stuff or does this have to be documented somewhere?
Plugins shipped with VDR usually only need to work with this version of VDR, they (and the newplugin script) don't provide backwards compatibility. Plugin authors that want to support a wider range of VDR versions with one release will usually need to adapt their behavior to the VDR version they compile with.
In this case, they will have to add -fPIC and the FILE_OFFSET stuff for older versions of VDR, and only include Make.global for the upcoming VDR versions.
However, this is not a concern for VDR or for this patch. Its ok for VDR to look ahead, and not to carry endless compatibility hacks with it.
Cheers,
Udo
On Sat, Jan 30, 2010 at 4:21 AM, Udo Richter udo_richter@gmx.de wrote:
However, this is not a concern for VDR or for this patch. Its ok for VDR to look ahead, and not to carry endless compatibility hacks with it.
I couldn't agree more. I cant stand when something is bloated down with that, just because some users out there refuse to update their system. I understand 'if its not broke, dont fix it', but that doesnt mean be an anchor weighing everything down. If you choose to stay in the dark ages, then suffer the consequences. ;)
-
Anssi Hannula -
Frank Schmirler -
Paul Menzel -
Udo Richter -
VDR User -
Ville Skyttä