Bug#272557: provide NotAutomatic option
On Fri, Nov 11, 2016 at 04:21:07PM +0100, David Kalnischkies wrote:
> Hi,
>
> looks good, applies cleanly & works, but let me be picky and
> ask for some more changes anyhow:
Picky is good :)
> On Fri, Nov 11, 2016 at 12:05:52PM +0000, James Clarke wrote:
> > On Wed, Nov 09, 2016 at 02:31:02PM +0100, David Kalnischkies wrote:
> > e.g. <literal>APT::FTPArchive::Release::Origin</literal>. The supported fields
> > are <literal>Origin</literal>, <literal>Label</literal>, <literal>Suite</literal>,
> > <literal>Version</literal>, <literal>Codename</literal>, <literal>Date</literal>,
> > + <literal>NotAutomatic</literal>, <literal>ButAutomaticUpgrades</literal>,
> > <literal>Valid-Until</literal>, <literal>Signed-By</literal>, <literal>Architectures</literal>,
> > <literal>Components</literal> and <literal>Description</literal>.</para></listitem>
>
> add "Acquire-By-Hash" here as your code supports it and the other option
> (might) do more (in future), so it shouldn't hurt™.
>
>
> > + // Read configuration for string fields, but don't output them
> > + for(map<string,string>::iterator I = Fields.begin();
> > + I != Fields.end();
> > + ++I)
>
> Please use c++11 for loops here, e.g.: for (auto &&I: Fields)
The && doesn't play nicely with making I const, so I dropped the const
on the third loop. I believe `auto const &I` should work if you'd like
to keep the const-ness.
> > + {
> > + string Config = string("APT::FTPArchive::Release::") + I->first;
> > + I->second = _config->Find(Config, I->second.c_str());
> ^^^^^^^^
> Unneeded as that parameter can be a std::string instance.
>
> > - if (Value == "")
> > + if (I->second == "")
>
> as you change that line use .empty() here. (Same difference)
>
> > # ensure we have it in the Release file
> > -testsuccess grep "Acquire-By-Hash: true" aptarchive/dists/unstable/*Release
> > +testsuccess grep "Acquire-By-Hash: yes" aptarchive/dists/unstable/*Release
>
> Good find! Thansk deity@ apt is so lax in syntax checking… :)
>
> > +#
> > +# main()
> > +#
>
> No idea why we ended up with testcases having those strangers,
> but lets not add more of those…
I had no clue what it was for either!
> > +configcompression 'gz' '.'
> > +confighashes 'SHA256' 'SHA512'
>
> Remove those two as well as the testcase has no specific needs.
>
> > +# build one package
> > +buildsimplenativepackage 'foo' 'i386' '1' 'unstable'
> > +buildaptarchivefromincoming
>
> You aren't using a 'real' package in this test, so we can save some
> cpu cycles by just faking a few as needed and play with them:
>
> insertpackage 'unstable' 'foo' 'i386' '1'
> insertpackage 'experimental' 'foo' 'i386' '3'
> setupaptarchive
>
> # … greps here …
>
> testsuccessequal '<output installing foo in version 1>' apt install foo -s
>
>
> And while we are at it, even if not strictly needed, lets add a But…
> case by sprinkling in a few more lines in the right places (we have to
> waste the cpu cycles we saved above somehow after all ;) )
>
> getnotautomaticfromsuite() {
> case "$1" in
> experimental|backports) echo 'yes';;
> esac
> }
> # FIXME: add support for that in framework
> getbutautomaticupgradesfromsuite() {
> case "$1" in
> backports) echo 'yes';;
> esac
> }
> insertpackage 'backports' 'foo' 'i386' '3~bpo1'
>
> insertinstalledpackage 'foo' 'i386' '2'
> testsuccessequal '<output installing foo in version 3~bpo1>' apt install foo -s
Revised v3 attached.
Regards,
James
>From 4e8fc865b0f52df95c92b9ca40302dad3911959a Mon Sep 17 00:00:00 2001
From: James Clarke <jrtc27@jrtc27.com>
Date: Fri, 11 Nov 2016 16:33:25 +0000
Subject: [PATCH v3] apt-ftparchive: Support NotAutomatic and
ButAutomaticUpgrades fields
This also changes Acquire-By-Hash to be "yes" rather than "true", so it
is consistent with dak's output.
Closes: #272557
---
doc/apt-ftparchive.1.xml | 4 +-
ftparchive/writer.cc | 36 ++++++++++-----
test/integration/framework | 14 ++++--
test/integration/test-apt-ftparchive-by-hash | 2 +-
test/integration/test-apt-ftparchive-notautomatic | 54 +++++++++++++++++++++++
5 files changed, 93 insertions(+), 17 deletions(-)
diff --git a/doc/apt-ftparchive.1.xml b/doc/apt-ftparchive.1.xml
index 705f416..66d4b4f 100644
--- a/doc/apt-ftparchive.1.xml
+++ b/doc/apt-ftparchive.1.xml
@@ -106,7 +106,9 @@
e.g. <literal>APT::FTPArchive::Release::Origin</literal>. The supported fields
are <literal>Origin</literal>, <literal>Label</literal>, <literal>Suite</literal>,
<literal>Version</literal>, <literal>Codename</literal>, <literal>Date</literal>,
- <literal>Valid-Until</literal>, <literal>Signed-By</literal>, <literal>Architectures</literal>,
+ <literal>NotAutomatic</literal>, <literal>ButAutomaticUpgrades</literal>,
+ <literal>Acquire-By-Hash</literal>, <literal>Valid-Until</literal>,
+ <literal>Signed-By</literal>, <literal>Architectures</literal>,
<literal>Components</literal> and <literal>Description</literal>.</para></listitem>
</varlistentry>
diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc
index 018cf00..eb17521 100644
--- a/ftparchive/writer.cc
+++ b/ftparchive/writer.cc
@@ -1004,6 +1004,7 @@ ReleaseWriter::ReleaseWriter(FileFd * const GivenOutput, string const &/*DB*/) :
time_t const now = time(NULL);
time_t const validuntil = now + _config->FindI("APT::FTPArchive::Release::ValidTime", 0);
+ map<string,bool> BoolFields;
map<string,string> Fields;
Fields["Origin"] = "";
Fields["Label"] = "";
@@ -1017,19 +1018,32 @@ ReleaseWriter::ReleaseWriter(FileFd * const GivenOutput, string const &/*DB*/) :
Fields["Components"] = "";
Fields["Description"] = "";
Fields["Signed-By"] = "";
- if (_config->FindB("APT::FTPArchive::DoByHash", false) == true)
- Fields["Acquire-By-Hash"] = "true";
-
- for(map<string,string>::const_iterator I = Fields.begin();
- I != Fields.end();
- ++I)
+ BoolFields["Acquire-By-Hash"] = _config->FindB("APT::FTPArchive::DoByHash", false);
+ BoolFields["NotAutomatic"] = false;
+ BoolFields["ButAutomaticUpgrades"] = false;
+
+ // Read configuration for string fields, but don't output them
+ for (auto &&I : Fields)
{
- string Config = string("APT::FTPArchive::Release::") + (*I).first;
- string Value = _config->Find(Config, (*I).second.c_str());
- if (Value == "")
- continue;
+ string Config = string("APT::FTPArchive::Release::") + I.first;
+ I.second = _config->Find(Config, I.second);
+ }
- std::string const out = I->first + ": " + Value + "\n";
+ // Read configuration for bool fields, and add them to Fields if true
+ for (auto &&I : BoolFields)
+ {
+ string Config = string("APT::FTPArchive::Release::") + I.first;
+ I.second = _config->FindB(Config, I.second);
+ if (I.second)
+ Fields[I.first] = "yes";
+ }
+
+ // All configuration read and stored in Fields; output
+ for (auto &&I : Fields)
+ {
+ if (I.second.empty())
+ continue;
+ std::string const out = I.first + ": " + I.second + "\n";
Output->Write(out.c_str(), out.length());
}
diff --git a/test/integration/framework b/test/integration/framework
index 0222f2b..9a114ae 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1053,6 +1053,12 @@ getreleaseversionfromsuite() { true; }
getlabelfromsuite() { true; }
getoriginfromsuite() { true; }
getarchitecturesfromreleasefile() { echo "all $(getarchitectures)"; }
+getnotautomaticfromsuite() {
+ case "$1" in
+ experimental|experimental2) echo "yes";;
+ esac
+}
+getbutautomaticupgradesfromsuite() { true; }
aptftparchiverelease() {
aptftparchive -qq release "$@" | sed -e '/0 Release$/ d' # remove the self reference
@@ -1071,6 +1077,8 @@ generatereleasefiles() {
local VERSION="$(getreleaseversionfromsuite $SUITE)"
local LABEL="$(getlabelfromsuite $SUITE)"
local ORIGIN="$(getoriginfromsuite $SUITE)"
+ local NOTAUTOMATIC="$(getnotautomaticfromsuite $SUITE)"
+ local BUTAUTOMATICUPGRADES="$(getbutautomaticupgradesfromsuite $SUITE)"
aptftparchiverelease "$dir" \
-o APT::FTPArchive::Release::Suite="${SUITE}" \
-o APT::FTPArchive::Release::Codename="${CODENAME}" \
@@ -1078,11 +1086,9 @@ generatereleasefiles() {
-o APT::FTPArchive::Release::Label="${LABEL}" \
-o APT::FTPArchive::Release::Origin="${ORIGIN}" \
-o APT::FTPArchive::Release::Version="${VERSION}" \
+ -o APT::FTPArchive::Release::NotAutomatic="${NOTAUTOMATIC}" \
+ -o APT::FTPArchive::Release::ButAutomaticUpgrades="${BUTAUTOMATICUPGRADES}" \
> "$dir/Release"
- if [ "$SUITE" = "experimental" -o "$SUITE" = "experimental2" ]; then
- sed -i '/^Date: / a\
-NotAutomatic: yes' "$dir/Release"
- fi
done
else
msgninfo "\tGenerate Release files for flat… "
diff --git a/test/integration/test-apt-ftparchive-by-hash b/test/integration/test-apt-ftparchive-by-hash
index d08b942..75fb0cf 100755
--- a/test/integration/test-apt-ftparchive-by-hash
+++ b/test/integration/test-apt-ftparchive-by-hash
@@ -46,7 +46,7 @@ verify_by_hash
testsuccess stat aptarchive/dists/unstable/main/binary-i386/by-hash/SHA256/$previous_hash
# ensure we have it in the Release file
-testsuccess grep "Acquire-By-Hash: true" aptarchive/dists/unstable/*Release
+testsuccess grep "Acquire-By-Hash: yes" aptarchive/dists/unstable/*Release
# now ensure gc work
for i in $(seq 3); do
diff --git a/test/integration/test-apt-ftparchive-notautomatic b/test/integration/test-apt-ftparchive-notautomatic
new file mode 100755
index 0000000..b2f65cc
--- /dev/null
+++ b/test/integration/test-apt-ftparchive-notautomatic
@@ -0,0 +1,54 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'i386'
+
+getnotautomaticfromsuite() {
+ case "$1" in
+ experimental|backports) echo 'yes';;
+ esac
+}
+getbutautomaticupgradesfromsuite() {
+ case "$1" in
+ backports) echo 'yes';;
+ esac
+}
+
+insertpackage 'unstable' 'foo' 'i386' '1'
+insertpackage 'backports' 'foo' 'i386' '3~bpo1'
+insertpackage 'experimental' 'foo' 'i386' '3'
+setupaptarchive
+
+# check no unstable NotAutomatic field
+testfailure grep "NotAutomatic:" aptarchive/dists/unstable/*Release
+# check backports NotAutomatic field
+testsuccess grep "NotAutomatic: yes" aptarchive/dists/backports/*Release
+# check experimental NotAutomatic field
+testsuccess grep "NotAutomatic: yes" aptarchive/dists/experimental/*Release
+
+# check no unstable ButAutomaticUpgrades field
+testfailure grep "ButAutomaticUpgrades:" aptarchive/dists/unstable/*Release
+# check backports ButAutomaticUpgrades field
+testsuccess grep "ButAutomaticUpgrades: yes" aptarchive/dists/backports/*Release
+# check no experimental ButAutomaticUpgrades field
+testfailure grep "ButAutomaticUpgrades:" aptarchive/dists/experimental/*Release
+
+testsuccessequal 'Reading package lists...
+Building dependency tree...
+The following NEW packages will be installed:
+ foo
+0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
+Inst foo (1 unstable [i386])
+Conf foo (1 unstable [i386])' apt install foo -s
+
+insertinstalledpackage 'foo' 'i386' '2'
+testsuccessequal 'Reading package lists...
+Building dependency tree...
+The following packages will be upgraded:
+ foo
+1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
+Inst foo [2] (3~bpo1 backports [i386])
+Conf foo (3~bpo1 backports [i386])' apt install foo -s
--
2.10.2
Reply to: