Bug#272557: provide NotAutomatic option
On Wed, Nov 09, 2016 at 02:31:02PM +0100, David Kalnischkies wrote:
> Hi,
>
> (doh… I thought I had sent this mail 7 days ago…)
Heh, no problem, it happens!
> On Sat, Oct 29, 2016 at 03:48:11PM +0100, James Clarke wrote:
> > diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc index
> > index 018cf00..0bfe6ba 100644
> > --- a/ftparchive/writer.cc
> > +++ b/ftparchive/writer.cc
> > @@ -1019,7 +1019,9 @@ ReleaseWriter::ReleaseWriter(FileFd * const GivenOutput, string const &/*DB*/) :
> > Fields["Signed-By"] = "";
> > if (_config->FindB("APT::FTPArchive::DoByHash", false) == true)
> > Fields["Acquire-By-Hash"] = "true";
> > -
> > + Fields["NotAutomatic"] = "";
> > + Fields["ButAutomaticUpgrades"] = "";
> > +
> > for(map<string,string>::const_iterator I = Fields.begin();
> > I != Fields.end();
> > ++I)
>
> … that works I am sure, but it feels like the wrong interface – after
> all: The only reasonable setting is "yes", anything else is bogus. In
> sofar I guess I would prefer a boolean config option defaulting to false
> and emitting the field with the right value on true instead of being
> string based (with the bonus that apt accepts yes/no, 1/0 and a few more
> as boolean values, too. People might get "confused" by this and set the
> string-based value to true with unexpected results…).
I've attached an updated patch which distinguishes boolean fields from
string fields.
> A test would be nifty, too. As noted in an earlier mail our testing
> framework adds the field via sed at the moment. It shouldn't be hard to
> change it to use the new setting instead. No need to come up with a new
> testcase here to be able to confidently say "well tested". :)
I've added a separate test case. While test/integration/framework was
adding NotAutomatic for experimental{,2}, nothing was actually using
this (which I verified by just removing the bit of code and seeing what
changed). The only other reference to NotAutomatic was in
test-policy-pinning, but that tests all kinds of values for
NotAutomatic, not just missing and yes, so adapting that to use the new
option wasn't really going to work out as far as I could see.
Adding yet another test case for ButAutomaticUpgrades seems unnecessary,
given that it will behave exactly the same as NotAutomatic.
Regards,
James
>From c9f52217429fea9b98c514fc8dbb18735c9de6e7 Mon Sep 17 00:00:00 2001
From: James Clarke <jrtc27@jrtc27.com>
Date: Fri, 11 Nov 2016 11:55:32 +0000
Subject: [PATCH v2] 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 | 1 +
ftparchive/writer.cc | 36 ++++++++++++++++++-----
test/integration/framework | 11 ++++---
test/integration/test-apt-ftparchive-by-hash | 2 +-
test/integration/test-apt-ftparchive-notautomatic | 31 +++++++++++++++++++
5 files changed, 68 insertions(+), 13 deletions(-)
diff --git a/doc/apt-ftparchive.1.xml b/doc/apt-ftparchive.1.xml
index 705f416..95d7876 100644
--- a/doc/apt-ftparchive.1.xml
+++ b/doc/apt-ftparchive.1.xml
@@ -106,6 +106,7 @@
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>
diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc
index 018cf00..30d7083 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,38 @@ 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";
-
+ 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(map<string,string>::iterator I = Fields.begin();
+ I != Fields.end();
+ ++I)
+ {
+ string Config = string("APT::FTPArchive::Release::") + I->first;
+ I->second = _config->Find(Config, I->second.c_str());
+ }
+
+ // Read configuration for bool fields, and add them to Fields if true
+ for(map<string,bool>::iterator I = BoolFields.begin();
+ I != BoolFields.end();
+ ++I)
+ {
+ 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(map<string,string>::const_iterator I = Fields.begin();
I != Fields.end();
++I)
{
- string Config = string("APT::FTPArchive::Release::") + (*I).first;
- string Value = _config->Find(Config, (*I).second.c_str());
- if (Value == "")
+ if (I->second == "")
continue;
-
- std::string const out = I->first + ": " + Value + "\n";
+ 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..be711a6 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1053,6 +1053,11 @@ getreleaseversionfromsuite() { true; }
getlabelfromsuite() { true; }
getoriginfromsuite() { true; }
getarchitecturesfromreleasefile() { echo "all $(getarchitectures)"; }
+getnotautomaticfromsuite() {
+ case "$1" in
+ experimental|experimental2) echo "yes";;
+ esac
+}
aptftparchiverelease() {
aptftparchive -qq release "$@" | sed -e '/0 Release$/ d' # remove the self reference
@@ -1071,6 +1076,7 @@ generatereleasefiles() {
local VERSION="$(getreleaseversionfromsuite $SUITE)"
local LABEL="$(getlabelfromsuite $SUITE)"
local ORIGIN="$(getoriginfromsuite $SUITE)"
+ local NOTAUTOMATIC="$(getnotautomaticfromsuite $SUITE)"
aptftparchiverelease "$dir" \
-o APT::FTPArchive::Release::Suite="${SUITE}" \
-o APT::FTPArchive::Release::Codename="${CODENAME}" \
@@ -1078,11 +1084,8 @@ 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}" \
> "$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..0661326
--- /dev/null
+++ b/test/integration/test-apt-ftparchive-notautomatic
@@ -0,0 +1,31 @@
+#!/bin/sh
+set -e
+
+#
+# main()
+#
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'i386'
+configcompression 'gz' '.'
+confighashes 'SHA256' 'SHA512'
+
+# build one package
+buildsimplenativepackage 'foo' 'i386' '1' 'unstable'
+buildaptarchivefromincoming
+
+# check no NotAutomatic field
+testfailure grep "NotAutomatic:" aptarchive/dists/unstable/*Release
+
+# insert new package for experimental
+buildsimplenativepackage 'bar' 'i386' '1' 'experimental'
+# delete ftparchive.conf and dists so they get regenerated with experimental included
+rm -rf aptarchive/ftparchive.conf aptarchive/dists
+# and build again
+buildaptarchivefromincoming
+
+# check no unstable NotAutomatic field
+testfailure grep "NotAutomatic:" aptarchive/dists/unstable/*Release
+# check experimental NotAutomatic field (option added by framework)
+testsuccess grep "NotAutomatic: yes" aptarchive/dists/experimental/*Release
--
2.10.2
Reply to: