Document tagfile-keys.h as internal to apt

The previous regime of the file was to sort it on insert, but that
changes the values in the generated enum, which is fine as long as we
only use it in libapt itself, but breaks on other users.

The header was always intended to be private to apt itself, so we just
document this here now and lay the ground work to have the file in the
future only appended to, so that it remains sufficiently ABI stable that
we can use it outside the library in our apt tools.

We also remove some fields apt is unlikely to need or only uses in
certain cases outside of any (speed) critical path to have enough room
to add more fields soon as currently we are limited to 128 fields max
and it would be sad if we use up that allowance entirely already.

7 files changed, 82 insertions, 45 deletions

diff --git a/apt-pkg/CMakeLists.txt b/apt-pkg/CMakeLists.txt
index 5c97493af..a40b041b8 100644
--- a/apt-pkg/CMakeLists.txt
+++ b/apt-pkg/CMakeLists.txt
@@ -2,7 +2,8 @@
 include_directories(${PROJECT_BINARY_DIR}/include/apt-pkg)
 
 file(MAKE_DIRECTORY ${PROJECT_BINARY_DIR}/include/apt-pkg/)
-execute_process(COMMAND ${TRIEHASH_EXECUTABLE}
+execute_process(COMMAND grep -v "^#" "${CMAKE_CURRENT_SOURCE_DIR}/tagfile-keys.list"
+                COMMAND ${TRIEHASH_EXECUTABLE}
                         --ignore-case
                          --header ${PROJECT_BINARY_DIR}/include/apt-pkg/tagfile-keys.h
                          --code ${CMAKE_CURRENT_BINARY_DIR}/tagfile-keys.cc
@@ -10,8 +11,10 @@ execute_process(COMMAND ${TRIEHASH_EXECUTABLE}
                          --enum-name pkgTagSection::Key
                          --function-name pkgTagHash
                          --include "<apt-pkg/tagfile.h>"
-                         ${CMAKE_CURRENT_SOURCE_DIR}/tagfile-keys.list)
+                         --include "<apt-pkg/header-is-private.h>"
+                         /dev/stdin )
 set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "tagfile-keys.list")
+set_source_files_properties("${CMAKE_CURRENT_BINARY_DIR}/tagfile-keys.cc" PROPERTIES COMPILE_DEFINITIONS APT_COMPILING_APT)
 
 
 # Set the version of the library
diff --git a/apt-pkg/cachefilter-patterns.h b/apt-pkg/cachefilter-patterns.h
index e6ea16c68..284fcc1cf 100644
--- a/apt-pkg/cachefilter-patterns.h
+++ b/apt-pkg/cachefilter-patterns.h
@@ -11,6 +11,7 @@
 #include <apt-pkg/cachefile.h>
 #include <apt-pkg/cachefilter.h>
 #include <apt-pkg/error.h>
+#include <apt-pkg/header-is-private.h>
 #include <apt-pkg/string_view.h>
 #include <apt-pkg/strutl.h>
 #include <iostream>
@@ -20,10 +21,6 @@
 #include <vector>
 #include <assert.h>
 
-#ifndef APT_COMPILING_APT
-#error Internal header
-#endif
-
 namespace APT
 {
 
diff --git a/apt-pkg/contrib/header-is-private.h b/apt-pkg/contrib/header-is-private.h
new file mode 100644
index 000000000..201a40c62
--- /dev/null
+++ b/apt-pkg/contrib/header-is-private.h
@@ -0,0 +1,3 @@
+#ifndef APT_COMPILING_APT
+#error Internal header without ABI stability. Should only be used by apt itself!
+#endif
diff --git a/apt-pkg/tagfile-keys.list b/apt-pkg/tagfile-keys.list
index 36da3435e..a93052c2f 100644
--- a/apt-pkg/tagfile-keys.list
+++ b/apt-pkg/tagfile-keys.list
@@ -1,7 +1,16 @@
+# This file is input for triehash(1) (after stripping comments)
+#
+# The fields listed here are accessible via pkgTagSection::Key::FIELD
+# Do *NOT* edit, remove or insert new fields in the sorted section here
+# as the file forms part of the ABI of the libapt library and is used
+# by our apt tools. External clients are forbidden though, so if really needed
+# we can use libapt Breaks: apt, but always prefer appending only.
+#
+# For Fields used in Packages, Sources and status files, see also tagfile-order.c
 Architecture
+Auto-Built-Package
 Binary
 Breaks
-Bugs
 Build-Conflicts
 Build-Conflicts-Arch
 Build-Conflicts-Indep
@@ -15,7 +24,6 @@ Checksums-Md5
 Checksums-Sha1
 Checksums-Sha256
 Checksums-Sha512
-Class
 Conffiles
 Config-Version
 Conflicts
@@ -23,7 +31,6 @@ Depends
 Description
 Description-md5
 Directory
-Dm-Upload-Allowed
 Enhances
 Essential
 Filename
@@ -32,29 +39,22 @@ Format
 Homepage
 Important
 Installed-Size
-Installer-Menu-Item
-Kernel-Version
 Maintainer
 MD5sum
-MSDOS-Filename
 Multi-Arch
 Optional
 Origin
 Original-Maintainer
 Package
 Package-List
-Package_Revision
-Package-Revision
 Package-Type
 Phased-Update-Percentage
 Pre-Depends
 Priority
 Protected
 Provides
-Recommended
 Recommends
 Replaces
-Revision
 Section
 SHA1
 SHA256
@@ -64,17 +64,13 @@ Source
 Standards-Version
 Static-Built-Using
 Status
-Subarchitecture
 Suggests
 Tag
 Task
 Testsuite
 Testsuite-Triggers
-Triggers-Awaited
-Triggers-Pending
 Uploaders
 Vcs-Arch
-Vcs-Browse
 Vcs-Browser
 Vcs-Bzr
 Vcs-Cvs
@@ -84,3 +80,4 @@ Vcs-Hg
 Vcs-Mtn
 Vcs-Svn
 Version
+### APPEND BELOW, sort in with next ABI break ###
diff --git a/apt-pkg/tagfile-order.c b/apt-pkg/tagfile-order.c
index 79d6992b4..510d6dd95 100644
--- a/apt-pkg/tagfile-order.c
+++ b/apt-pkg/tagfile-order.c
@@ -9,12 +9,12 @@ static const char *iTFRewritePackageOrder[] = {
    "Package",
    "Package-Type",
    "Architecture",
-   "Subarchitecture", // Used only by d-i
+   "Subarchitecture", // NO_KEY: Used only by d-i
    "Version",
-   "Revision",	       // Obsolete (warning in dpkg)
-   "Package-Revision", // Obsolete (warning in dpkg)
-   "Package_Revision", // Obsolete (warning in dpkg)
-   "Kernel-Version",   // Used only by d-i
+   "Revision",         // NO_KEY: Obsolete (warning in dpkg)
+   "Package-Revision", // NO_KEY: Obsolete (warning in dpkg)
+   "Package_Revision", // NO_KEY: Obsolete (warning in dpkg)
+   "Kernel-Version",   // NO_KEY: Used only by d-i
    "Built-Using",
    "Static-Built-Using",
    "Built-For-Profiles",
@@ -22,27 +22,29 @@ static const char *iTFRewritePackageOrder[] = {
    "Multi-Arch",
    "Status",
    "Priority",
-   "Class", // dpkg nickname for Priority
+   "Class", // NO_KEY: dpkg nickname for Priority
    "Build-Essential",
    "Protected",
+   "Important", // old name of Protected
    "Essential",
-   "Installer-Menu-Item", // Used only by d-i
+   "Installer-Menu-Item", // NO_KEY: Used only by d-i
    "Section",
    "Source",
    "Origin",
+   "Phased-Update-Percentage",
    "Maintainer",
    "Original-Maintainer", // unknown in dpkg order
-   "Bugs",
+   "Bugs", // NO_KEY: very uncommon encounter
    "Config-Version", // Internal of dpkg
    "Conffiles",
-   "Triggers-Awaited",
-   "Triggers-Pending",
+   "Triggers-Awaited", // NO_KEY: Internal of dpkg
+   "Triggers-Pending", // NO_KEY: Internal of dpkg
    "Installed-Size",
    "Provides",
    "Pre-Depends",
    "Depends",
    "Recommends",
-   "Recommended", // dpkg nickname for Recommends
+   "Recommended", // NO_KEY: dpkg nickname for Recommends
    "Suggests",
    "Optional", // dpkg nickname for Suggests
    "Conflicts",
@@ -50,7 +52,7 @@ static const char *iTFRewritePackageOrder[] = {
    "Replaces",
    "Enhances",
    "Filename",
-   "MSDOS-Filename", // Obsolete (used by dselect)
+   "MSDOS-Filename", // NO_KEY: Obsolete (used by dselect)
    "Size",
    "MD5sum",
    "SHA1",
@@ -58,6 +60,7 @@ static const char *iTFRewritePackageOrder[] = {
    "SHA512",
    "Homepage",
    "Description",
+   "Description-md5",
    "Tag",
    "Task",
    0,
@@ -70,13 +73,13 @@ static const char *iTFRewriteSourceOrder[] = {
    "Architecture",
    "Version",
    "Priority",
-   "Class", // dpkg nickname for Priority
+   "Class", // NO_KEY: dpkg nickname for Priority
    "Section",
    "Origin",
    "Maintainer",
    "Original-Maintainer", // unknown in dpkg order
    "Uploaders",
-   "Dm-Upload-Allowed", // Obsolete (ignored by dak)
+   "Dm-Upload-Allowed", // NO_KEY: Obsolete (ignored by dak)
    "Standards-Version",
    "Build-Depends",
    "Build-Depends-Arch",
@@ -89,7 +92,7 @@ static const char *iTFRewriteSourceOrder[] = {
    "Homepage",
    "Description",
    "Vcs-Browser",
-   "Vcs-Browse", // dak only (nickname?)
+   "Vcs-Browse", // NO_KEY: dak only (nickname?)
    "Vcs-Arch",
    "Vcs-Bzr",
    "Vcs-Cvs",
diff --git a/apt-pkg/tagfile.h b/apt-pkg/tagfile.h
index f6bdd12c9..54562d07c 100644
--- a/apt-pkg/tagfile.h
+++ b/apt-pkg/tagfile.h
@@ -70,18 +70,21 @@ class APT_PUBLIC pkgTagSection
    std::string FindS(APT::StringView sv) const { return Find(sv).to_string(); }
    std::string FindRawS(APT::StringView sv) const { return FindRaw(sv).to_string(); };
 
+#ifdef APT_COMPILING_APT
    // Functions for lookup with a perfect hash function
    enum class Key;
-   APT_HIDDEN bool Find(Key key,const char *&Start, const char *&End) const;
-   APT_HIDDEN bool Find(Key key,unsigned int &Pos) const;
-   APT_HIDDEN signed int FindI(Key key,signed long Default = 0) const;
-   APT_HIDDEN bool FindB(Key key, bool Default = false) const;
-   APT_HIDDEN unsigned long long FindULL(Key key, unsigned long long const &Default = 0) const;
-   APT_HIDDEN bool FindFlag(Key key,uint8_t &Flags, uint8_t const Flag) const;
-   APT_HIDDEN bool FindFlag(Key key,unsigned long &Flags, unsigned long Flag) const;
-   APT_HIDDEN bool Exists(Key key) const;
-   APT_HIDDEN APT::StringView Find(Key key) const;
-   APT_HIDDEN APT::StringView FindRaw(Key key) const;
+   bool Find(Key key,const char *&Start, const char *&End) const;
+   bool Find(Key key,unsigned int &Pos) const;
+   signed int FindI(Key key,signed long Default = 0) const;
+   bool FindB(Key key, bool Default = false) const;
+   unsigned long long FindULL(Key key, unsigned long long const &Default = 0) const;
+   bool FindFlag(Key key,uint8_t &Flags, uint8_t const Flag) const;
+   bool FindFlag(Key key,unsigned long &Flags, unsigned long Flag) const;
+   bool Exists(Key key) const;
+   APT::StringView Find(Key key) const;
+   APT::StringView FindRaw(Key key) const;
+#endif
+
    bool Find(APT::StringView Tag,const char *&Start, const char *&End) const;
    bool Find(APT::StringView Tag,unsigned int &Pos) const;
    APT::StringView Find(APT::StringView Tag) const;
diff --git a/test/integration/test-apt-tagfile-fields-order b/test/integration/test-apt-tagfile-fields-order
index 804d49c86..d7bf0d137 100755
--- a/test/integration/test-apt-tagfile-fields-order
+++ b/test/integration/test-apt-tagfile-fields-order
@@ -35,6 +35,37 @@ comparelsts() {
 	fi
 }
 
+grep -v '^#' "${SOURCEDIRECTORY}/apt-pkg/tagfile-keys.list" > tagfile-keys.list
+# Hardcoding this is a bit silly, but it might help preventing issues…
+msgtest 'File is append only: do not sort, remove or insert keys in' 'tagfile-keys.list'
+testequal --nomsg 'Version' sed '73q;d' tagfile-keys.list
+
+msgtest 'List has fewer entries than pkgTagSection buckets' 'tagfile-keys.list'
+ENTRIES_LIST="$(wc -l tagfile-keys.list | cut -d' ' -f 1)"
+ENTRIES_BUCKET="$(grep -m 1 'AlphaIndexes\[' "${SOURCEDIRECTORY}/apt-pkg/tagfile.h" | sed -e 's#.*\[\([0-9]\+\)\].*#\1#')"
+if test $ENTRIES_LIST -lt $ENTRIES_BUCKET; then
+	msgpass
+else
+	echo
+	echo "List has $ENTRIES_LIST entries, but pkgTagSection can only store $ENTRIES_BUCKET as AlphaIndexes"
+	msgfail
+fi
+
+msgtest 'Check for duplicates in' 'tagfile-keys.list'
+sort tagfile-keys.list > apt.lst
+testempty --nomsg uniq --repeated 'apt.lst'
+
+msgtest 'Check that apt knows all fields it orders' 'itself'
+grep -v "// NO_KEY: " "${SOURCEDIRECTORY}/apt-pkg/tagfile-order.c" | sed -ne 's#^   "\(.*\)",.*$#\1#p' | sort -u > dpkg.lst
+comparelsts
+
+msgtest 'Check tagfile-keys.list does not contain' 'obsoleted and internal fields'
+grep "// NO_KEY: " "${SOURCEDIRECTORY}/apt-pkg/tagfile-order.c" | sed -ne 's#^   "\(.*\)",.*$#\1#p' > obsolete.lst
+sed -n -e's#^ *// *"\(.*\)",.*$#\1#p' "${SOURCEDIRECTORY}/apt-pkg/tagfile-order.c" >> obsolete.lst
+sort -u obsolete.lst > obsolete-sorted.lst
+sort obsolete-sorted.lst tagfile-keys.list > obsolete-keys.lst
+testempty --nomsg uniq --repeat 'obsolete-keys.lst'
+
 msgtest 'Check that apt knows all fields dpkg orders in' 'Packages'
 dpkg_field_ordered_list 'CTRL_INDEX_PKG' > dpkg.lst
 sed -ne 's#^   "\(.*\)",.*$#\1#p' "${SOURCEDIRECTORY}/apt-pkg/tagfile-order.c" | sed -n '/^Package$/,/^Package$/ p' | head -n -1 | sort > apt.lst