General rewrite of the codebase

0. Overview
===========

This commit message contains the following sections:

 * Rationale
 * Removed features
 * Changed behaviour
 * New features
 * Regressions
 * Code quality improvements

1. Rationale
============

The old code base suffered from:

 * The overuse of global variables (80) which were freely used by
   functions to store their results in as side-effects of their actual
   computation.
 * Underdocumented configuration file settings. Some values are only
   mentioned in the passing while documenting other things in the man
   page. Some values are not mentioned at all.
 * Repetition of code snippets. Some functions only differed slightly
   but were copy-pasted to create new versions. Configuration parsing
   and dumping copy-pasted value conversion multiple times instead of
   working with value types.
 * Abuse of system() and backticks without escaping special shell
   characters instead of passing an array to system().
 * Missing error checks when system() or backticks are used.
 * Non-existence of unit tests.
 * rm -rf your whole filesystem with the right configuration parameters

This commit *reduces* the lines of code in the main program from 1390
down to FIXME while at the same time *increasing* the ratio of comments
to lines of code from 0.06 to FIXME and *adding* additional features and
fixing the problems listed above.

2. Removed features
===================

 * The --source-dir command line option and retainsources configuration
   variable have been removed as the feature of downloading source
   packages corresponding to the installed binary packages was deemed
   out of scope for multistrap
 * The workarounds for dash not configuring (#546528) and absolute
   /lib64 symlinks (#553599) were dropped as they are not needed anymore
 * Removed support for dpkg without multiarch

3. Changed behaviour
====================

 * They keyring setting does not accept packages anymore but either
   keyring files or directories with .gpg files in them. This is so that
   multistrap does not have to install a package on the host system (which
   requires superuser privileges) nor is it anymore required that the apt
   sources of the host system contain the right mirrors to retrieve the
   desired keyring packages nor is it anymore required that the selected
   keyring package stores the keyring at the location where sbuild expects
   them.
 * Any additional keyring package now has to be manually listed in the
   packages setting as they are no longer installed automatically. This
   now also makes it possible to bootstrap a system without the keyring
   packages that were required to install it.
 * Die if setupscripts, configscripts or hook produce an error instead
   of continuing.
 * Die if the specified debconf seed file does not exist.
 * Be more unforgiving about problems. Previous to this commit,
   multistrap would
    - skip loop iterations with invalid values
    - print a warning if errors happen but continue anyways
   Now we die with an appropriate error message immediately.
 * The component now has to be passed explicitly in each section

4. New features
===============

 * Configuration files can now be nested arbitrarily deeply.
 * All configuration variables are documented together with their
   default values and data types.
 * Dumping the configuration settings creates another valid
   configuration file. This can be used to merge multiple configuration
   files into a single one containing the resulting settings.

5. Regressions
==============

 * The translations need updating for the new and changed strings.

6. Code quality improvements
============================

6.1. General
------------

 * Replaced 80 global variables by 2 which are only used for reading.
 * Removed unused variables.
 * Removed unused or superfluous code.
 * Functions have no side-effects on variables anymore. Everything they
   compute is part of their return value.
 * Instead of using "." and join() to create a whitespace delimitered
   string which is then split(), push to an array in the first place.
 * Make file a modulino to let it be used as a program and module at the
   same time. This allows the unit tests to import functions from it.
 * Use Getopt::Long for option parsing.
 * Use Pod::Usage for --help and man output.

6.2. Less dependencies
----------------------

 * Drop dependency on Config::Auto for Config::IniFiles. The latter
   comes with support for all sorts of configuration file formats which
   we do not need. The later offers features not exposed by the former
   (like automatically ignoring case) which in turn simplifies the code.
 * Replace Locale::gettext by Dpkg::Gettext which makes the availability
   of gettext optional. Dpkg::Gettext is available anyway because we
   plan to use Dpkg::Index instead of Parse::Debian::Packages.

6.3. Unit tests
---------------

Add tests:
 - t/config.t for the recursive ini file reader
 - t/critic.t for perlcritic
 - t/perltidy.t for perltidy
This commit is contained in:
Johannes 'josch' Schauer 2016-12-29 22:40:22 +01:00
parent 3f77f992e5
commit ff96767b2f
11 changed files with 801 additions and 438 deletions

1036
multistrap

File diff suppressed because it is too large Load diff

103
t/config.t Executable file
View file

@ -0,0 +1,103 @@
#!/usr/bin/perl
# Copyright (C) 2015-2017 Johannes Schauer <josch@mister-muffin.de>
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to
# deal in the Software without restriction, including without limitation the
# rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
# sell copies of the Software, and to permit persons to whom the Software is
# furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included in
# all copies or substantial portions of the Software.
use strict;
use warnings;
use Test::More tests => 6;
require "./multistrap";
my %tests = (
complex => {
general => {
include => [
[ 'branch1.ini', 'branch2.ini' ],
[
[ ['shared.ini'], [], ['t/data/branch1.ini'] ],
[
['intermediate.ini'],
[ [ ['shared.ini'], [], ['t/data/intermediate.ini'] ] ],
['t/data/branch2.ini']
]
],
['t/data/complex.ini']
],
property => [
['1'],
[
[
['2'], [ [ ['4'], [], ['t/data/shared.ini'] ] ],
['t/data/branch1.ini']
],
[
['3'], [ [ ['4'], [], ['t/data/shared.ini'] ] ],
['t/data/branch2.ini']
],
],
['t/data/complex.ini']
],
foo => [ ['bar'], [], ['t/data/intermediate.ini'] ]
}
},
shared => {
general => { property => [ ['4'], [], ['t/data/shared.ini'] ] }
},
intermediate => {
general => {
include => [ ['shared.ini'], [], ['t/data/intermediate.ini'] ],
property => [ ['4'], [], ['t/data/shared.ini'] ],
foo => [ ['bar'], [], ['t/data/intermediate.ini'] ]
}
},
branch1 => {
general => {
include => [ ['shared.ini'], [], ['t/data/branch1.ini'] ],
property => [
['2'], [ [ ['4'], [], ['t/data/shared.ini'] ] ],
['t/data/branch1.ini']
]
}
},
branch2 => {
general => {
include => [
['intermediate.ini'],
[ [ ['shared.ini'], [], ['t/data/intermediate.ini'] ] ],
['t/data/branch2.ini']
],
property => [
['3'], [ [ ['4'], [], ['t/data/shared.ini'] ] ],
['t/data/branch2.ini']
],
foo => [ ['bar'], [], ['t/data/intermediate.ini'] ]
}
},
concat => {
general => {
property => [
[ '4', '1' ], [], [ 't/data/shared.ini', 't/data/simple.ini' ]
],
include =>
[ [ 'shared.ini', 'simple.ini' ], [], ['t/data/concat.ini'] ]
}
}
);
use Data::Dumper;
print(Data::Dumper->Dump([multistrap::cascade("t/data/concat.ini")]));
foreach my $k ( sort keys %tests ) {
is_deeply( multistrap::cascade("t/data/$k.ini"), $tests{$k}, "$k.ini" );
}

21
t/critic.t Executable file
View file

@ -0,0 +1,21 @@
#!/usr/bin/perl
# Copyright (C) 2015-2017 Johannes Schauer <josch@mister-muffin.de>
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to
# deal in the Software without restriction, including without limitation the
# rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
# sell copies of the Software, and to permit persons to whom the Software is
# furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included in
# all copies or substantial portions of the Software.
use strict;
use warnings;
use Test::Perl::Critic (-severity => 1);
use Test::More tests => 1;
critic_ok("./multistrap");

3
t/data/branch1.ini Normal file
View file

@ -0,0 +1,3 @@
[general]
include=shared.ini
property=2

3
t/data/branch2.ini Normal file
View file

@ -0,0 +1,3 @@
[general]
include=intermediate.ini
property=3

4
t/data/complex.ini Normal file
View file

@ -0,0 +1,4 @@
[general]
include=branch1.ini
include=branch2.ini
property=1

3
t/data/concat.ini Normal file
View file

@ -0,0 +1,3 @@
[general]
include=shared.ini
include=simple.ini

3
t/data/intermediate.ini Normal file
View file

@ -0,0 +1,3 @@
[general]
include=shared.ini
foo=bar

2
t/data/shared.ini Normal file
View file

@ -0,0 +1,2 @@
[general]
property=4

2
t/data/simple.ini Normal file
View file

@ -0,0 +1,2 @@
[general]
property=1

5
t/perltidy.t Normal file
View file

@ -0,0 +1,5 @@
#!/usr/bin/perl
use Test::PerlTidy;
run_tests(perltidyrc => 't/perltidyrc');