Bug#430649: Please clarify splitting/syntax of DEB_BUILD_OPTIONS
Loïc Minier [EMAIL PROTECTED] writes: - separating with commas prevents passing options with commas; a sample use case which might be useful in the near future could be passing CFLAGS/LDFLAGS via DEB_BUILD_OPTIONS, and some of these flags require commas to reach the linker/assembler/preprocessor (e.g. -Wl,-O1) I think this is a convincing reason to standardize on a space-separated list, combined with the additional ease of parsing. Note: there's an interesting issue in that CFLAGS or LDFLAGS are typically space separated and hence we would need to define how to pass multiple such flags in DEB_BUILD_OPTIONS... Yeah, but so far there's no DEB_BUILD_OPTIONS tag for that, and I'm not sure it would be a good idea, as opposed to a more generic optimization level setting or the like. It seems to me that the more separators we allow, the less room for future expansion of DEB_BUILD_OPTIONS we keep and the more complex the early parsing of DEB_BUILD_OPTIONS will be. Agreed. Additionally to the request to document the syntax and remove the recommendation for findstring, I also wish Policy would document how to deal with a keyword being present multiple times. For example: DEB_BUILD_OPTIONS=noopt parallel=2 debug parallel=1 Currently, this is only an issue for parallel, since there are no other options that can contradict each other. (There is no debug option.) I think it's easiest to just say to not do that. -- Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/
Bug#430649: Please clarify splitting/syntax of DEB_BUILD_OPTIONS
Hi, On Wed, Jul 04, 2007, Russ Allbery wrote: At first glance, using a space-separated list of options would be a lot easier to parse since you could parse it entirely with internal make functions and not have to shell out to run sed. A couple of updates: - parsing a comma-separated list wouldn't require sed as shown in #209008 with this Make snippet to convert comma-separated to space-separated: , := , DEB_BUILD_OPTIONS := $(subst $(,), ,$(DEB_BUILD_OPTIONS)) - separating with commas prevents passing options with commas; a sample use case which might be useful in the near future could be passing CFLAGS/LDFLAGS via DEB_BUILD_OPTIONS, and some of these flags require commas to reach the linker/assembler/preprocessor (e.g. -Wl,-O1) Note: there's an interesting issue in that CFLAGS or LDFLAGS are typically space separated and hence we would need to define how to pass multiple such flags in DEB_BUILD_OPTIONS... It seems to me that the more separators we allow, the less room for future expansion of DEB_BUILD_OPTIONS we keep and the more complex the early parsing of DEB_BUILD_OPTIONS will be. Additionally to the request to document the syntax and remove the recommendation for findstring, I also wish Policy would document how to deal with a keyword being present multiple times. For example: DEB_BUILD_OPTIONS=noopt parallel=2 debug parallel=1 On this particular subject, I would recommend that Policy requires to parse DEB_BUILD_OPTIONS from left to right and to override any previous parameter when its met a second time (latest definition wins). Perhaps it makes sense to explicitely mention = in parameters and the allowed chars in parameter names; I could imagine we could at some point support +=: DEB_BUILD_OPTIONS=CFLAGS=-Os parallel=2 debug CFLAGS+=-g (The += words could be parsed in Make using eval for example.) -- Loïc Minier
Bug#430649: Please clarify splitting/syntax of DEB_BUILD_OPTIONS
Loïc Minier [EMAIL PROTECTED] writes: My preferred policy change would be to state that DEB_BUILD_OPTIONS must be a space-separated list of keywords and parameters with parameters taking the form name=value. The allowed chars for name and values would be limited to a-z, A-Z, 1-9, dash, and underscore. At first glance, using a space-separated list of options would be a lot easier to parse since you could parse it entirely with internal make functions and not have to shell out to run sed. Plus, using whitespace as a separator is much more natural for makefiles, and Policy now requires that debian/rules be a makefile. That to me argues for making it space-separated, but I'm not sure if that additional tightness in the specification would cause problems for existing usage. If everything is using the current Policy definition and just searching for the substring, we can tighten without breaking any of the parsers, but we might break what people are doing to set the value. If someone could come up with a snippet that's better than: OPTIONS := $(shell echo $$DEB_BUILD_OPTIONS|sed 's/[^-_=[:alnum:]]/ /g') in terms of both complexity and how much shell code it runs, that would make me more comfortable with the idea of supporting more arbitrary delimiters. It doesn't look like there's a make equivalent to that sed command, though. I suppose one alternative is to use: OPTIONS := $(shell echo $$DEB_BUILD_OPTIONS | sed 's/,/ /g') which is simpler, although still a shell invocation, and would support the two cases that people have specifically mentioned. We could then write Policy language saying that either whitespace or commas are supported as separators, and the example code would use $(filter ...) on $(OPTIONS). (I'm not particularly worried about supporting case insensitivity; we don't currently support that, which means that no one is expecting it.) -- Russ Allbery ([EMAIL PROTECTED]) http://www.eyrie.org/~eagle/
Bug#430649: Please clarify splitting/syntax of DEB_BUILD_OPTIONS
Package: debian-policy Version: 3.7.2.2 Severity: normal Hi, In the discussions of bug #209008 (parallel building), Peter Samuelson explained he uses comma-separated keywords in DEB_BUILD_OPTIONS. Please document how keywords in DEB_BUILD_OPTIONS are allowed to be separated. I find the recommendation of the Make function findstring a poor one as it will match subwords and will make it harder to introduce new keywords (as we will have to ensure the new keywords aren't matched in current rules). I would personally push for allowing only space separated keywords as this makes parsing of DEB_BUILD_OPTIONS really easy, especially in make. Peter Samuelson uses commas because it's doesn't impose quoting when spawning a build from a shell command-line. Bye, -- System Information: Debian Release: lenny/sid APT prefers unstable APT policy: (500, 'unstable'), (1, 'experimental') Architecture: i386 (i686) Kernel: Linux 2.6.22-rc5-686 (SMP w/2 CPU cores) Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/bash -- no debconf information -- Loïc Minier
Bug#430649: Please clarify splitting/syntax of DEB_BUILD_OPTIONS
On Tue, Jun 26, 2007 at 11:53:36AM +0200, Loïc Minier wrote: Package: debian-policy Version: 3.7.2.2 Severity: normal Hi, In the discussions of bug #209008 (parallel building), Peter Samuelson explained he uses comma-separated keywords in DEB_BUILD_OPTIONS. Please document how keywords in DEB_BUILD_OPTIONS are allowed to be separated. I agree it is important that all debian/rules parse the options the same way. However findstring actually allow both spaces and commas as separator. I find the recommendation of the Make function findstring a poor one as it will match subwords and will make it harder to introduce new keywords (as we will have to ensure the new keywords aren't matched in current rules). Could you provide a Makefile snippet that implement your proposal instead of findstring ? Cheers, -- Bill. [EMAIL PROTECTED] Imagine a large red swirl here.
Bug#430649: Please clarify splitting/syntax of DEB_BUILD_OPTIONS
On Tue, Jun 26, 2007, Bill Allombert wrote: I agree it is important that all debian/rules parse the options the same way. However findstring actually allow both spaces and commas as separator. Sure, but $findstring(debug,nodebug) will match debug for example. And it will also match build-debug, debug-deb, parallel-debug etc. I find the recommendation of the Make function findstring a poor one as it will match subwords and will make it harder to introduce new keywords (as we will have to ensure the new keywords aren't matched in current rules). Could you provide a Makefile snippet that implement your proposal instead of findstring ? Yes, for example $(filter debug,$(DEB_BUILD_OPTIONS)) will return debug if debug is listed as a separate word in DEB_BUILD_OPTIONS, if DEB_BUILD_OPTIONS is a space separated list of words. Peter Samuelson proposed a way to map DEB_BUILD_OPTIONS separated by other separators into a space separated variable: d_b_o:=$(shell echo $$DEB_BUILD_OPTIONS|sed 's/[^-_=[:alnum:]]/ /g'|tr a-z- A-Z_) There's also the sub-question of passing parameters with values (and not only a boolean flag), such as parallel=n in DEB_BUILD_OPTIONS. With a space-separated DEB_BUILD_OPTIONS, one can easily use: DEB_BUILD_OPTIONS_PARALLEL ?= $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) Peter Samuelson proposed an eval snippet importing all DEB_BUILD_OPTIONS vars into Make variables: $(foreach o, $(d_b_o), $(if $(findstring =,$o),$(eval DEB_BUILD_OPT_$o),$(eval DEB_BUILD_OPT_$o=1))) (This works if you build d_b_o like proposed above or if you have a space separated DEB_BUILD_OPTIONS.) My preferred policy change would be to state that DEB_BUILD_OPTIONS must be a space-separated list of keywords and parameters with parameters taking the form name=value. The allowed chars for name and values would be limited to a-z, A-Z, 1-9, dash, and underscore. -- Loïc Minier
