[pacman-dev] [PATCH 1/2] makepkg-template: ignore duplicates per option
When templates recursively include other templates, a diamond problem, similar to multiple inheritance, may occur. It is illustrated in the test case. As templates may consist of arbitrary code, it may be unwanted to execute a template twice. This patch provides a solution to some simple cases of this problem by completely ignoring every template occurrence but the first for each template name. Signed-off-by: Dominik Fischer <d.f.fischer@web.de> --- doc/makepkg-template.1.txt | 6 ++++++ scripts/makepkg-template.pl.in | 12 ++++++++++++ .../makepkg-template-tests/diamond-input/PKGBUILD | 5 +++++ .../diamond-input/PKGBUILD.expanded | 13 +++++++++++++ .../diamond-input/templates/base-1.template | 1 + .../diamond-input/templates/first-include-1.template | 1 + .../diamond-input/templates/second-include-1.template | 1 + .../diamond-input/testcase-config | 18 ++++++++++++++++++ 8 files changed, 57 insertions(+) create mode 100644 test/scripts/makepkg-template-tests/diamond-input/PKGBUILD create mode 100644 test/scripts/makepkg-template-tests/diamond-input/PKGBUILD.expanded create mode 100644 test/scripts/makepkg-template-tests/diamond-input/templates/base-1.template create mode 100644 test/scripts/makepkg-template-tests/diamond-input/templates/first-include-1.template create mode 100644 test/scripts/makepkg-template-tests/diamond-input/templates/second-include-1.template create mode 100644 test/scripts/makepkg-template-tests/diamond-input/testcase-config diff --git a/doc/makepkg-template.1.txt b/doc/makepkg-template.1.txt index 03b2a38..bd2355c 100644 --- a/doc/makepkg-template.1.txt +++ b/doc/makepkg-template.1.txt @@ -76,6 +76,12 @@ Options given multiple times in which case files found in directory given last will take precedence. +*-t, \--no-duplicates*:: + Do not include any template twice, even when templates recursively include + other templates. Only the first occurrence of each template will be + expanded, all further references to the same name, also with different + version numbers, will be ignored. + Example PKGBUILD ---------------- diff --git a/scripts/makepkg-template.pl.in b/scripts/makepkg-template.pl.in index 9877796..e9f6e08 100755 --- a/scripts/makepkg-template.pl.in +++ b/scripts/makepkg-template.pl.in @@ -33,6 +33,8 @@ my %opts = ( my $template_name_charset = qr/[[:alnum:]+_.@-]/; my $template_marker = qr/# template/; +my %seen = (); + # runtime loading to avoid dependency on cpan since this is the only non-core module my $loaded_gettext = can_load(modules => {'Locale::gettext' => undef}); if ($loaded_gettext) { @@ -96,6 +98,15 @@ sub parse_template_line { sub load_template { my ($values) = @_; + if ($opts{dups}) { + # omit already included templates. + if (exists $seen{$values->{name}}) { + return ""; + } else { + $seen{$values->{name}} = (); + } + } + my $ret = ""; my $template_name = "$values->{name}"; @@ -205,6 +216,7 @@ GetOptions( "output|o=s" => \$opts{output}, "newest|n" => \$opts{newest}, "template-dir=s@" => \$opts{template_dir}, + "no-duplicates|t" => \$opts{dups}, ) or usage(1); $opts{output} = $opts{input} unless $opts{output}; diff --git a/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD b/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD new file mode 100644 index 0000000..6bb4fa5 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD @@ -0,0 +1,5 @@ +pkgname=foo +pkgver=1 + +# template input; name=first-include; version=1; +# template input; name=second-include; version=1; diff --git a/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD.expanded b/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD.expanded new file mode 100644 index 0000000..8077581 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD.expanded @@ -0,0 +1,13 @@ +pkgname=foo +pkgver=1 + +# template start; name=first-include; version=1; +# template start; name=base; version=1; +package() {} +# template end; +# template end; +# template start; name=second-include; version=1; +# template start; name=base; version=1; +package() {} +# template end; +# template end; diff --git a/test/scripts/makepkg-template-tests/diamond-input/templates/base-1.template b/test/scripts/makepkg-template-tests/diamond-input/templates/base-1.template new file mode 100644 index 0000000..056096f --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input/templates/base-1.template @@ -0,0 +1 @@ +package() {} diff --git a/test/scripts/makepkg-template-tests/diamond-input/templates/first-include-1.template b/test/scripts/makepkg-template-tests/diamond-input/templates/first-include-1.template new file mode 100644 index 0000000..9916370 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input/templates/first-include-1.template @@ -0,0 +1 @@ +# template input; name=base; version=1; diff --git a/test/scripts/makepkg-template-tests/diamond-input/templates/second-include-1.template b/test/scripts/makepkg-template-tests/diamond-input/templates/second-include-1.template new file mode 100644 index 0000000..9916370 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input/templates/second-include-1.template @@ -0,0 +1 @@ +# template input; name=base; version=1; diff --git a/test/scripts/makepkg-template-tests/diamond-input/testcase-config b/test/scripts/makepkg-template-tests/diamond-input/testcase-config new file mode 100644 index 0000000..65bb8d0 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input/testcase-config @@ -0,0 +1,18 @@ +arguments+=(-t) +expected_exitcode=0 + +IFS="" read -d '' expected_output <<'EOF' +EOF + +IFS="" read -d '' expected_result <<'EOF' +pkgname=foo +pkgver=1 + +# template start; name=first-include; version=1; +# template start; name=base; version=1; +package() {} +# template end; +# template end; +# template start; name=second-include; version=1; +# template end; +EOF -- 2.4.2
It may be necessary to just suppress duplication of some templates while others are safe and intended to occur multiple times. As information about the intended duplication cannot easily be placed into the template file itself without additional introspection measures, this patch allows to suppress duplication via an option in the template markers. Signed-off-by: Dominik Fischer <d.f.fischer@web.de> --- doc/makepkg-template.1.txt | 10 +++++++--- scripts/makepkg-template.pl.in | 10 ++++++++-- .../makepkg-template-tests/diamond-input-key/PKGBUILD | 5 +++++ .../diamond-input-key/PKGBUILD.expanded | 10 ++++++++++ .../diamond-input-key/templates/base-1.template | 1 + .../templates/first-include-1.template | 1 + .../templates/second-include-1.template | 1 + .../diamond-input-key/testcase-config | 17 +++++++++++++++++ 8 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 test/scripts/makepkg-template-tests/diamond-input-key/PKGBUILD create mode 100644 test/scripts/makepkg-template-tests/diamond-input-key/PKGBUILD.expanded create mode 100644 test/scripts/makepkg-template-tests/diamond-input-key/templates/base-1.template create mode 100644 test/scripts/makepkg-template-tests/diamond-input-key/templates/first-include-1.template create mode 100644 test/scripts/makepkg-template-tests/diamond-input-key/templates/second-include-1.template create mode 100644 test/scripts/makepkg-template-tests/diamond-input-key/testcase-config diff --git a/doc/makepkg-template.1.txt b/doc/makepkg-template.1.txt index bd2355c..a368b1e 100644 --- a/doc/makepkg-template.1.txt +++ b/doc/makepkg-template.1.txt @@ -34,8 +34,9 @@ and # template end; -Currently used keys are: name (mandatory) and version. Template names are limited to -alphanumerics, "@", "+", ".", "-", and "_". Versions are limited to numbers and ".". +Currently used keys are: name (mandatory), version and duplicate. Template names are +limited to alphanumerics, "@", "+", ".", "-", and "_". Versions are limited to +numbers and ".". For initial creation there is a one line short cut which does not need an end marker: @@ -80,7 +81,10 @@ Options Do not include any template twice, even when templates recursively include other templates. Only the first occurrence of each template will be expanded, all further references to the same name, also with different - version numbers, will be ignored. + version numbers, will be ignored. To only suppress the duplication of + selected templates, add "duplicate=no;" to all relevant template markers. + Note that all occurrences of the template have to be marked, not just the + first one. Example PKGBUILD diff --git a/scripts/makepkg-template.pl.in b/scripts/makepkg-template.pl.in index e9f6e08..5ac8bef 100755 --- a/scripts/makepkg-template.pl.in +++ b/scripts/makepkg-template.pl.in @@ -78,6 +78,8 @@ sub parse_template_line { $values{$key} = $val; } + $values{duplicate} = not(defined($values{duplicate}) and $values{duplicate} =~ /no|false/); + # end doesn't take arguments if ($values{command} ne "end") { if (!$values{name}) { @@ -98,7 +100,7 @@ sub parse_template_line { sub load_template { my ($values) = @_; - if ($opts{dups}) { + if ($opts{dups} or not $values->{duplicate}) { # omit already included templates. if (exists $seen{$values->{name}}) { return ""; @@ -127,7 +129,11 @@ sub load_template { my $parsed = process_file($path); - $ret .= "# template start; name=$values->{name}; version=$version;\n"; + $ret .= "# template start; name=$values->{name}; version=$version;"; + if (not $values->{duplicate}) { + $ret .= " duplicate=no;"; + } + $ret .= "\n"; $ret .= $parsed; $ret .= "# template end;\n"; return $ret; diff --git a/test/scripts/makepkg-template-tests/diamond-input-key/PKGBUILD b/test/scripts/makepkg-template-tests/diamond-input-key/PKGBUILD new file mode 100644 index 0000000..6bb4fa5 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input-key/PKGBUILD @@ -0,0 +1,5 @@ +pkgname=foo +pkgver=1 + +# template input; name=first-include; version=1; +# template input; name=second-include; version=1; diff --git a/test/scripts/makepkg-template-tests/diamond-input-key/PKGBUILD.expanded b/test/scripts/makepkg-template-tests/diamond-input-key/PKGBUILD.expanded new file mode 100644 index 0000000..c5dc8cd --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input-key/PKGBUILD.expanded @@ -0,0 +1,10 @@ +pkgname=foo +pkgver=1 + +# template start; name=first-include; version=1; +# template start; name=base; version=1;duplicate=false; +package() {} +# template end; +# template end; +# template start; name=second-include; version=1; +# template end; diff --git a/test/scripts/makepkg-template-tests/diamond-input-key/templates/base-1.template b/test/scripts/makepkg-template-tests/diamond-input-key/templates/base-1.template new file mode 100644 index 0000000..056096f --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input-key/templates/base-1.template @@ -0,0 +1 @@ +package() {} diff --git a/test/scripts/makepkg-template-tests/diamond-input-key/templates/first-include-1.template b/test/scripts/makepkg-template-tests/diamond-input-key/templates/first-include-1.template new file mode 100644 index 0000000..3a27d66 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input-key/templates/first-include-1.template @@ -0,0 +1 @@ +# template input; name=base; version=1; duplicate=no; diff --git a/test/scripts/makepkg-template-tests/diamond-input-key/templates/second-include-1.template b/test/scripts/makepkg-template-tests/diamond-input-key/templates/second-include-1.template new file mode 100644 index 0000000..b862538 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input-key/templates/second-include-1.template @@ -0,0 +1 @@ +# template input; name=base; version=1; duplicate=false; diff --git a/test/scripts/makepkg-template-tests/diamond-input-key/testcase-config b/test/scripts/makepkg-template-tests/diamond-input-key/testcase-config new file mode 100644 index 0000000..bf86efb --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input-key/testcase-config @@ -0,0 +1,17 @@ +expected_exitcode=0 + +IFS="" read -d '' expected_output <<'EOF' +EOF + +IFS="" read -d '' expected_result <<'EOF' +pkgname=foo +pkgver=1 + +# template start; name=first-include; version=1; +# template start; name=base; version=1; duplicate=no; +package() {} +# template end; +# template end; +# template start; name=second-include; version=1; +# template end; +EOF -- 2.4.2
On 08.06.2015 11:28, Dominik Fischer wrote:
When templates recursively include other templates, a diamond problem, similar to multiple inheritance, may occur. It is illustrated in the test case. As templates may consist of arbitrary code, it may be unwanted to execute a template twice. This patch provides a solution to some simple cases of this problem by completely ignoring every template occurrence but the first for each template name.
I'm not a fan of this being a command line option, but I'm also unsure if I like the option at all. templates shouldn't change their meaning (they shouldn't go from a simple variable definition to a package() function) and if a PKGBUILD includes 2 templates that both provide package() then that's a bug in the PKGBUILD and will be noticed when the templates are used the first time. Anyone else wants to weigh in?
diff --git a/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD.expanded b/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD.expanded new file mode 100644 index 0000000..8077581 --- /dev/null +++ b/test/scripts/makepkg-template-tests/diamond-input/PKGBUILD.expanded
This file is not used anywhere?
On 09/06/15 05:20, Florian Pritz wrote:
On 08.06.2015 11:28, Dominik Fischer wrote:
When templates recursively include other templates, a diamond problem, similar to multiple inheritance, may occur. It is illustrated in the test case. As templates may consist of arbitrary code, it may be unwanted to execute a template twice. This patch provides a solution to some simple cases of this problem by completely ignoring every template occurrence but the first for each template name.
I'm not a fan of this being a command line option, but I'm also unsure if I like the option at all. templates shouldn't change their meaning (they shouldn't go from a simple variable definition to a package() function) and if a PKGBUILD includes 2 templates that both provide package() then that's a bug in the PKGBUILD and will be noticed when the templates are used the first time.
Not really... If you provide two package functions in a PKGBUILD, the last one will be used.
Anyone else wants to weigh in?
Can we just error out if the recursive inclusion brings in any template twice? If someone ever demonstrates the need for a diamond inclusion (that is not stupid...), we can rethink our position. Do we have infinite recursion protection as it is? (A -> B -> A -> ...) I see it being reasonable to use the same template twice in a package (e.g. in split package_*() functions). So this needs detected only during recursion. A
On 09.06.2015 07:34, Allan McRae wrote:
I see it being reasonable to use the same template twice in a package (e.g. in split package_*() functions). So this needs detected only during recursion.
I'm pretty sure I totally ignored the problem of endless recursion when writing the script and I don't believe it has been fixed since. Throwing an error if one tree has a loop sounds like a good idea.
participants (3)
-
Allan McRae
-
Dominik Fischer
-
Florian Pritz