[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Request for Review: APT manpages



On Fri, Jun 1, 2012 at 11:38 AM, Justin B Rye <jbr@edlug.org.uk> wrote:
>>    <orderedlist>
>>       <para>When an APT tool starts up it will read the configuration files
>>       in the following order:</para>
>>       <listitem><para>the file specified by the <envar>APT_CONFIG</envar>
>>        environment variable (if any)</para></listitem>
>>       <listitem><para>all files in <literal>Dir::Etc::Parts</literal> in
>                                  ~~
> Dir::Etc::Parts isn't a directory that literally contains files, so
> say "listed in".

The value of Dir::Etc::Parts defines a directory which contains
files, but "listed in" sounds like it would be a list of files instead.


>>        alphanumeric ascending order which have either no or "<literal>conf</literal>"
>
> Again, I don't like "no as filename extension".
>
> The trouble with running this statement about collation straight into
> the statements about which ones will be ignored is that you might be
> saying that only the ones listed in the right order will be
> considered!
>
>>        as filename extension and which only contain alphanumeric,
>>        hyphen (-), underscore (_) and period (.) characters.
>>        Otherwise APT will print a notice that it has ignored a file if the file
>>        doesn't match a pattern in the <literal>Dir::Ignore-Files-Silently</literal>
>>        configuration list - in this case it will be silently ignored.</para></listitem>
>
> Too notty.  Rephrase all this as:
>
>        <listitem>
>         <para>
>          all files listed in <literal>Dir::Etc::Parts</literal> in
>          alphanumeric ascending order. The filenames must have an extension of
>          "<literal>conf</literal>" or nothing, and may only contain
>          alphanumeric, hyphen (-), underscore (_) and period (.) characters;
>          otherwise APT will print a notice that it has ignored a file, unless
>          that file matches a pattern in the
>          <literal>Dir::Ignore-Files-Silently</literal> configuration list - in
>          this case it will be silently ignored.
>         </para>
>        </listitem>

Mhhh. Lets recap:
We have three places in which we deal with defining read-order,
filenames and extensions.

Filename rules and order are the same for all.
apt_preferences files have no extension or ".pref"
apt.conf files have no extension or ".conf"
sources.list must have the extension ".list"
Warning and ignoring is the same for all three.

The current state after your suggestions so far:
----
  <para>Note that the files in the <filename>/etc/apt/preferences.d</filename>
  directory are parsed in alphanumeric ascending order and need to obey the
  following naming convention: The files have either no or
  "<literal>pref</literal>"
  as filename extension and only contain alphanumeric, hyphen (-),
  underscore (_) and period (.) characters.
  Otherwise APT will print a notice that it has ignored a file, unless that
  file matches a pattern in the <literal>Dir::Ignore-Files-Silently</literal>
  configuration list - in which case it will be silently ignored.</para>
----
  <listitem><para>all files in <literal>Dir::Etc::Parts</literal> in
  alphanumeric ascending order which have either no or "<literal>conf</literal>"
  as filename extension and which only contain alphanumeric,
  hyphen (-), underscore (_) and period (.) characters.
  Otherwise APT will print a notice that it has ignored a file, unless that
  file matches a pattern in the <literal>Dir::Ignore-Files-Silently</literal>
  configuration list - in which case it will be silently
ignored.</para></listitem>
----
  <para>The <filename>/etc/apt/sources.list.d</filename> directory provides
  a way to add sources.list entries in separate files.
  The format is the same as for the regular
<filename>sources.list</filename> file.
  File names need to end with
  <filename>.list</filename> and may only contain letters (a-z and A-Z),
  digits (0-9), underscore (_), hyphen (-) and period (.) characters.
  Otherwise APT will print a notice that it has ignored a file, unless that
  file matches a pattern in the <literal>Dir::Ignore-Files-Silently</literal>
  configuration list - in which case it will be silently ignored.</para>
----


The last three lines about ignoring/warning are the same now.
As filenaming restrictions are the same for all, we should share them, too,
so only the start has to differ a bit:
----
  Note that the files in the /etc/apt/preferences.d directory (configurable
  with Dir::Etc::PreferencesParts) either have no filename extension or
  need to have "pref" as filename extension.
----
  all files in the /etc/apt/apt.conf.d directory (configurable with
  Dir::Etc::Parts) either have no filename extension or need to have "conf"
  as filename extension.
----
  The /etc/apt/sources.list.d (configurable with Dir::Etc::SourceParts)
  provides a way to add sources.list entries in separate files, which all
  need to have "list" as filename extension.
----

After that we could go on with that for all three:

  These files are parsed in alphanumeric ascending order and the filenames
  need to contain only letters (a-z and A-Z), digits (0-9), underscores (_)
  and hyphens (-); periods (.) are only allowed if the last period is
  followed by the required extension; otherwise APT will print a notice that
  it has ignored a file, unless that file matches a pattern in the
  Dir::Ignore-Files-Silently configuration list -
  in which case it will be silently ignored.

(I omitted the XML tags here to avoid bloating it up)
(and yes, that is a perfect example for German writing -
 a single sentence of the size of a small paragraph.)


>>    <para>Syntactically the configuration language is modeled after what the ISC tools
>>    such as bind and dhcp use. Lines starting with
>
> (Did we decide the software was called BIND and DHCP?)

(I haven't changed that so far as I am not sure what exactly is referred
 to here. DHCP is properly correct, I am just not sure about BIND as I have
 never touched the configuration for those so far, so off for research)


>>    <literal>//</literal> are treated as comments (ignored), as well as all text
>>    between <literal>/*</literal> and <literal>*/</literal>, just like C/C++ comments.
>>    Each line is of the form
>>    <literal>APT::Get::Assume-Yes "true";</literal>. The trailing
>>    semicolon and the quotes are required. The value must be on one line, and
>
> Quotation marks may be officially correct, but it seems to tolerate
> their absence...

The parser used in libapt is not particularly picky, but also not incredible
nice as it either just silently does the wrong thing or fails hard on ill-
formed entries. Alternative implementations (think "cupt" or "apt2" here)
[plan[ed] to] have a more picky parser, so this manpage defines a subset
which should be supported.

The current parser e.g. supports c-style string concatenation, it is just a
nightmare to implement and not that useful compared to the complications
it causes. We don't plan to change the parser anytime soon as it is frankly
speaking a very daunting task and I personally see a multitude of other
issues i want to tackle before even thinking about such things, but we
aren't all equal … and it is nice to know that if we would want, we could.


>>    there is no kind of string concatenation. It must not include inside quotes.
>
> By "inside" I assume it means "internal", but does "quotes" include
> *single* quotes (AKA apostrophes)?

I don't think so.


>>    The behavior of the backslash "\" and escaped characters inside a value is
>>    undefined and it should not be used. An option name may include
>
> The behaviour is undefined and should not be used?
>
>>    alphanumerical characters and the "/-:._+" characters. A new scope can
>>    be opened with curly braces, like:</para>
>
> I would suggest:
>
>     The quotation marks and trailing semicolon are required.
>     The value must be on one line, and there is no kind of string concatenation.
>     Values must not include backslashes or extra quotation marks.
>     Option names are made up of alphanumeric characters and the characters "/-:._+".
>     A new scope can be opened with curly braces, like this:</para>

(Just saying: I used this paragraph)


>>    <para>Names for the configuration items are optional if a list is defined as it can be see in
>>    the <literal>DPkg::Pre-Install-Pkgs</literal> example above. If you don't specify a name a
>>    new entry will simply add a new option to the list. If you specify a name you can override
>>    the option as every other option by reassigning a new value to the option.</para>
>
> There are errors here that I can patch up:
>  s/as it can be see/as can be seen/
>  s/as every other option/in the same way as any other option/
> but more importantly I don't understand what this is trying to say.
> What is it that's being "omitted" in the DPkg::Pre-Install-Pkgs
> example - that is, what would be present if they weren't omitted?
>
> (Is it just that the values don't each need to be associated with a
> different suboption name?  If so it's making really heavy weather of
> something utterly obvious...)

Exactly, it is just saying that an option doesn't need to have a name.
The only think which it tries to highlight as some people misunderstand it
is that if you have two options without a name the second doesn't override
the first.


>>      <varlistentry><term><option>Architectures</option></term>
>>      <listitem><para>All Architectures the system supports. Processors implementing the
>>      <literal>amd64</literal> (also called <literal>x86-64</literal>) instruction set are
>>      e.g. also able to execute binaries compiled for the <literal>i386</literal>
>>      (<literal>x86</literal>) instruction set; This list is use when fetching files and
>>      parsing package lists. The internal default is always the native architecture (<literal>APT::Architecture</literal>)
>>      and all foreign architectures it can retrieve by calling <command>dpkg --print-foreign-architectures</command>.
>>      </para></listitem>
>>      </varlistentry>
>
> Misplaced "e.g.", misspelled "used".  What's an "internal" default?
> And how does the --print-foreign-architectures part fit in?
>
> [Goes away hunting end-user docs for multiarch... much later:]

(You found one?)

> Oh, it's saying that architectures that have been registered via
> "dpkg --add-architecture" (which are bewilderingly known as "foreign"
> architectures, even though the point is that you're acknowledging them
> as functionally compatible) get added to the Architectures list?

Yeap, it is saying this - in the light it was written at a time it wasn't
configured as it it is configured now in dpkg (current ubuntu releases
 do not have "dpkg --add-architecture", but have
 "dpkg --print-foreign-architectures", because the option was previously
 called "--foreign-architecture" instead, but i guess they have to do
 a [possibly painful] transition anyway for their new release…)
But you are right, we are probably better of telling people what they
have to look for if they want to let an architecture end up in this list
rather than how they (or APT) can get the current list out of dpkg.

For the record:
They are called "foreign" because they are not the "native" architecture,
but still can be run on this machine as a "translator" is available.
(even through the given example with i386 and amd64 is from the functional
 compatible subset of foreign, there is no problem in using binary
 incompatible architectures as long as you have a "translator" like qemu -
 so there is no problem in running "armel" on the machine per se, it is
 just that most people can relate more to i386/amd64 than armel…)


> I would tentatively suggest expressing that as:
>
>       <listitem>
>        <para>
>         All Architectures the system supports. For instance, CPUs implementing
>         the <literal>amd64</literal> (also called <literal>x86-64</literal>)
>         instruction set are also able to execute binaries compiled for the
>         <literal>i386</literal> (<literal>x86</literal>) instruction set. This
>         list is used when fetching files and parsing package lists. The
>         initial default is always the system's native architecture
>         (<literal>APT::Architecture</literal>), and foreign architectures are
>         added to the [default] list when they are registered via
>         <command>dpkg --add-architecture</command>.
>        </para>
>       </listitem>

They are not really added at the time dpkg is called; APT will add them
to the default list (I changed it to that) at runtime by asking dpkg for the
configured architectures - if (and that is why I said "to the default list")
the user didn't provide a APT::Architectures list, but i guess the
difference is neglectable (if not kind of obvious).
(It's also why I don't like the first sentence that much - as a system can
 basically support any architecture given the "translator" is installed.
 This list is just a list of architectures you want to be considered supported
 by your package manager - but I don't really have a better idea)


>>      <varlistentry><term><option>Immediate-Configure</option></term>
>
> Oh, this following paragraph is such a mess:

(agreed - without looking at your review)


>>      <listitem><para>Defaults to on which will cause APT to install essential and important packages
>
> Is it really "Essential: yes" and "Priority: important" packages but
> not "Priority: required" ones?

It just means important packages. Writing "and other important"
might clear this up a bit as the following sentences will omit
essential and just say important instead.
(Defining what is an important package for APT is probably even
 more complicated than this paragraph is already)

>>      as fast as possible in the install/upgrade operation. This is done to limit the effect of a failing
>
> "Soon", not "fast", unless it decrements a "sleep $DELAY" variable.

(interesting idea for a business model …)

>>      &dpkg; call: If this option is disabled APT does treat an important package in the same way as
>>      an extra package: Between the unpacking of the important package A and his configuration can then
>
> "His" configuration?

package A's (I am so proud of little boyish a becoming a real A-man …)

>>      be many other unpack or configuration calls, e.g. for package B which has no relation to A, but
>>      causes the dpkg call to fail (e.g. because maintainer script of package B generates an error) which results
>>      in a system state in which package A is unpacked but unconfigured - each package depending on A is now no
>>      longer guaranteed to work as their dependency on A is not longer satisfied. The immediate configuration marker
>>      is also applied to all dependencies which can generate a problem if the dependencies e.g. form a circle
>
> When it says "e.g." here, what *other* kinds of problematic
> dependency besides circular ones is it choosing not to mention?

Mostly other kinds of "circles", but it's a bit of a stretch to
nitpick here so the e.g. is uncalled for
(and I already have a headache, no need to make it a pandemic by
 given more even less understandable examples)


>>      as a dependency with the immediate flag is comparable with a Pre-Dependency. So in theory it is possible
>>      that APT encounters a situation in which it is unable to perform immediate configuration, errors out and
>>      refers to this option so the user can deactivate the immediate configuration temporarily to be able to perform
>
> It's not clear what's going on in this (theoretical) example.  Is the
> idea that Immediate-Configure allows APT to *recognise* the problem,
> abort, and ask for advice instead of blindly running into worse
> trouble?  Does "refers to this option" mean that there's a dialogue
> suggesting that it should be temporarily deactivated?

APT will fail with an error message claiming that it couldn't find
an installation order which satisfies the immediate configuration
constraints and refers to this option for a description of the
problem and possible "solutions".


>>      an install/upgrade again. Note the use of the word "theory" here as this problem was only encountered by now
>>      in real world a few times in non-stable distribution versions and was caused by wrong dependencies of the package
>>      in question or by a system in an already broken state, so you should not blindly disable this option as
>>      the mentioned scenario above is not the only problem immediate configuration can help to prevent in the first place.
>>      Before a big operation like <literal>dist-upgrade</literal> is run with this option disabled it should be tried to
>                                                                                                    ^^^^^^^^^^^^^^^^^^^^^
> This use of an impersonal verb doesn't work, and is unnecessary when
> the rest of the paragraph (and sentence) addresses "you" directly.
>
>>      explicitly <literal>install</literal> the package APT is unable to configure immediately, but please make sure to
>>      report your problem also to your distribution and to the APT team with the buglink below so they can work on
>>      improving or correcting the upgrade process.</para></listitem>
>>      </varlistentry>
>
>      <listitem>
>       <para>
>        Defaults to on, which will cause APT to install essential and important
>        packages as soon as possible in an install/upgrade operation, in order
>        to limit the effect of a failing &dpkg; call. If this option is
>        disabled, APT treats an important package in the same way as an extra
>        package: between the unpacking of the package A and its configuration
>        there can be many other unpack or configuration calls for other
>        unrelated packages B, C etc. If these cause the &dpkg; call to fail
>        (e.g. because package B's maintainer scripts generate an error), this
>        results in a system state in which package A is unpacked but
>        unconfigured - so any package depending on A is now no longer
>        guaranteed to work, as its dependency on A is no longer satisfied.
>
> Optionally, insert </para><para> here.
>
>        The immediate configuration marker is also applied in the potentially
>        problematic case of circular dependencies, since a dependency with the
>        immediate flag is equivalent to a Pre-Dependency. In theory this allows
>        APT to recognise a situation in which it is unable to perform immediate
>        configuration, abort, and suggest to the user that the option should be
>        temporarily deactivated in order to allow the operation to proceed.
>        Note the use of the word "theory" here; in the real world this problem
>        has rarely been encountered, in non-stable distribution versions, and
>        was caused by wrong dependencies of the package in question or by a
>        system in an already broken state; so you should not blindly disable
>        this option, as the scenario mentioned above is not the only problem it
>        can help to prevent in the first place.
>
> Again, optional </para><para>.
>
>        Before a big operation like <literal>dist-upgrade</literal> is run
>        with this option disabled you should try to explicitly
>        <literal>install</literal> the package APT is unable to configure
>        immediately; but please make sure you also report your problem to your
>        distribution and to the APT team with the buglink below, so they can
>        work on improving or correcting the upgrade process.
>       </para>
>      </listitem>
>     </varlistentry>

Interesting. I think I have written parts of this paragraph above and still,
I get the impression after reading your version I have a better
understanding of it than before.
(I have things in my head i can't make sense of;
 seems like I should write it down and ask for review here …)


>>      <varlistentry><term><option>Cache-Start</option></term><term><option>Cache-Grow</option></term><term><option>Cache-Limit</option></term>
>>      <listitem><para>APT uses since version 0.7.26 a resizable memory mapped cache file to store the 'available'
>
> Should "'available'" be in <literal> tags or something?

Maybe. I am not sure what I meant here through (see last comment).
I guess it is a reference to the available file from dpkg, but only in the
sense that it includes the information it had available, so I guess
I could just remove the apostrophes here.


>>      <varlistentry><term><option>Build-Essential</option></term>
>>      <listitem><para>Defines which package(s) are considered essential build dependencies.</para></listitem>
>                                            ^^^
> Just "packages" - how likely is it to be singular?

Very likely: Currently it is only one package: build-essential
But I don't think we should care, so yes, just "packages".


>>  <refsect1><title>The Acquire Group</title>
>>    <para>The <literal>Acquire</literal> group of options controls the download of packages
>>    and the URI handlers.
>
> Make that something like
>
>     and the various "acquire methods" (that is, URI handling schemes).
>
> to establish this jargon before it crops up bafflingly much later.

How about:

  <para>The <literal>Acquire</literal> group of options controls the
  download of packages as well as the various "acquire methods" responsible
  for the download itself (see also &sources-list;).</para>


>>      <varlistentry><term><option>PDiffs</option></term>
>>        <listitem><para>Try to download deltas called <literal>PDiffs</literal> for
>>        Packages or Sources files instead of downloading whole ones. True
>
> Add tags, in this case I suppose <filename> tags.

Making it "indexes" with an example instead as Translation-* files are
downloaded with PDiffs, if possible, too.

  <listitem><para>Try to download deltas called <literal>PDiffs</literal> for
  indexes (like <filename>Packages</filename> files) instead of downloading
  whole ones. True by default.</para>


>>      <varlistentry><term><option>http</option></term>
>>      <listitem><para>HTTP URIs; http::Proxy is the default http proxy to use. It is in the
>
> Huh?  Oh, right, it's acting as if "HTTP URIs" was a sort of
> subheading.  No, just say:

(don't know where this is coming from. Looks a bit like the style which is
 used to document options for the apt-* commands)


>>      <para>Three settings are provided for cache control with HTTP/1.1 compliant
>>      proxy caches. <literal>No-Cache</literal> tells the proxy to not use its cached
>>      response under any circumstances, <literal>Max-Age</literal> is sent only for
>>      index files and tells the cache to refresh its object if it is older than
>
> If I understand this correctly it needs some grammar fixes:
>
>       proxy caches. <literal>No-Cache</literal> tells the proxy not to use its
>       cached response under any circumstances, sending
>       <literal>Max-Age</literal> only for index files and telling the cache to
>       refresh its object if it is older than

This, i think is the first time you got it wrong (but don't worry, i noticed
it only while re-reading what i wanted to commit):
Max-Age is the second option, not part of the description for the first.

  <literal>No-Cache</literal> tells the proxy not to use its cached response
  under any circumstances.
  <literal>Max-Age</literal> sets the allowed maximum
  age (in seconds) of an index file in the cache of the proxy.
  <literal>No-Store</literal> specifies that the proxy should not store the
  requested archive files in its cache, which can be used to prevent the proxy
  from polluting its cache with (big) .deb files.


>>      this applies to all things including connection timeout and data timeout.</para>
>
> All "things"?

  this value applies to the connection as well as the data timeout.</para>
?


>>      <para>The setting <literal>Acquire::http::Pipeline-Depth</literal> can be used to
>>      enabled HTTP pipeling (RFC 2616 section 8.1.2.2) which can be beneficial e.g. on
>
> "to enable HTTP pipelining"

(oh dear …)


>>        <para><literal>CaInfo</literal> suboption specifies place of file that
>>        holds info about trusted certificates.
>
> Shouldn't all these be formatted as a sublist or something?

Probably, but I leave it as is for now to not fuzzy the complete file.
And we have kind of better documentation for it in an obscure config file
(much like the cron thing) so it might be better to just merge and
move it over here for wheezy+1.


> How many possibilities is that?  Is it
>       This can be done a) globally, for connections that go through a proxy, or
>                        b) for a specific host
> or
>       This can be done a) globally,
>                        b) for connections that go through a proxy, or
>                        c) for a specific host
>
> If it's the former, put the "proxy" bit in parentheses; if it's the
> latter, add an "or" after "globally".

(It is the later - but at least the german translation had first, too, …)


>>      <varlistentry><term><option>CompressionTypes</option></term>
>>      <listitem><para>List of compression types which are understood by the acquire methods.
>
> List?  What list?  s/List/This option contains the list/?

Mhhh. (Acquire::)CompressionTypes is the name of the list.
Your substitution sounds a bit like all types are enumerated in this
option (value) and not as sub-items.


>>      This can be used by the system administrator to let APT know that it should download also this files without
>>      actually use them if the environment doesn't specify this languages. So the following example configuration will
>>      result in the order "en, de" in an english and in "de, en" in a german localization. Note that "fr" is downloaded,
>>      but not used if APT is not used in a french localization, in such an environment the order would be "fr, de, en".
>
> I don't follow this.  It's something like:
>
>       This tells APT to download these translations too, without actually
>       using them unless the environment specifies the languages. So the
>       following example configuration will result in the order "en, de" in an
>       English locale or "de, en" in a German one. Note that "fr" is
>       downloaded, but not used unless APT is used in a French locale (where
>       the order would be "fr, de, en").
>
> But it seems to imply that there's some situation where you'd want
> (e.g) German translations to be automatically used when you're running
> in a French locale, and I don't understand why.

Imagine e.g. a german school with french exchange students.

Yeap, the example is a bit fabricated as it tries to show everything
mentioned in the previous paragraph at once, but nobody said we need
to make sense, we just should document it… ;)


>>      multiple calls of dpkg. Without further options dpkg will use triggers only in between his
>>      own run. Activating these options can therefore decrease the time needed to perform the
>
> "Only in between his own run"?  Does it mean "only once each time it runs"?

(yes, expect that i removed the "only" here as this suggests the wrong
 meaning - its not "only once" but "once each time!!!!111eleven")


>>      <emphasis>These options are therefore currently experimental and should not be used in
>>      productive environments.</emphasis> Also it breaks the progress reporting so all frontends will
>              ion                          It also breaks progress reporting such that all front-ends will
>
> (What does it mean by "frontends" anyway?  Seemingly not aptitude etc;
> and things like the http acquire are more like "back-ends".)

(It really means aptitude, synaptics, … The progress reporting is calculated
 by us currently, but only some front-ends (aka all but apt-get) display it.
 I guess we should implement it in apt-get and fix it…)


>>        if you want to run APT multiple times in a row - e.g. in an installer. In these sceneries you could
>
> When you say "installer" you mean "initial OS installer", right?

Any installer really. Debian installer, but maybe even puppet or
whatever script/program people use to install a bunch of packages.


>>  <refsect1>
>>    <title>Periodic and Archives options</title>
>>    <para><literal>APT::Periodic</literal> and <literal>APT::Archives</literal>
>>    groups of options configure behavior of apt periodic updates, which is
>>    done by <literal>/etc/cron.daily/apt</literal> script. See header of
>           ^the
>>    this script for the brief documentation of these options.
>
> There's no header field; you mean "See the comments in this script for
> brief documentation of these options".

"See the comments" sounds like these will be scattered all around the file
(with the picky remark that we don't know what a comment looks like).
So how about just saying "See the top of this script …"?


>>      <!-- Question: why doesn't this do anything?  The code says it should. -->
>>      <varlistentry>
>>        <term><option>Debug::pkgInitConfig</option></term>
>>        <listitem>
>>        <para>
>>          Dump the default configuration to standard error on
>>          startup.
>>        </para>
>>        </listitem>
>>      </varlistentry>
>
> Shouldn't this admit to not working (and preferably point at a
> bugreport)?

It does work, it just doesn't work if you expect it to work if provided
on the command line - as the command line interpreter is executed after
the pkgInitConfig class methods of loading defaults and files.
aka: You have to set the option in a file to get the dump.

I know that some people disagree with me on that part, but i just nuked
that option from the manpage because i don't feel like we have to document
all option we have, just for the sake of documenting all, especially if the
amount of users for this option who needs to look it up in the manpage
(and will find it there) is probably approaching zero - from the negative
side - as everyone needing that information is far better of with just
using 'apt-config dump' (because, lets face it, that is the thing which
 can actually be found).


> Finished!  But I'll need a few bits of feedback..

I would say "brilliant", "excellent" and personally I have probably
learned already more in this thread than from my last English teacher,
but I guess that is not the kind of feedback you asked for …


Best regards

David Kalnischkies


Reply to: