ftparchive: sort discovered filenames before writing indexes

If 'apt-ftparchive packages /path/to/files/' (or sources) is used the
files to include in the generated index (on stdout) were included in the
order in which they were discovered, which isn't a very stable order
which could lead to indexes changing without actually changing content
causing needless changes in the repository changing hashsums, pdiffs,
rsyncs, downloads, ….

This does not effect apt-ftparchive calls which already have an order
defined via a filelist (like generate) which will still print in the
order given by the filelist.

Note that a similar effect can be achieved by post-processing index
files with apt-sortpkgs.

Closes: 869557
Thanks: Chris Lamb for initial patch & Stefan Lippers-Hollmann for testing

3 files changed, 103 insertions, 23 deletions

diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc
index d5c9735e7..bdf9893c2 100644
--- a/ftparchive/writer.cc
+++ b/ftparchive/writer.cc
@@ -118,32 +118,35 @@ int FTWScanner::ScannerFTW(const char *File,const struct stat * /*sb*/,int Flag)
    return ScannerFile(File, true);
 }
 									/*}}}*/
-// FTWScanner::ScannerFile - File Scanner				/*{{{*/
-// ---------------------------------------------------------------------
-/* */
-int FTWScanner::ScannerFile(const char *File, bool const &ReadLink)
+static bool FileMatchesPatterns(char const *const File, std::vector<std::string> const &Patterns) /*{{{*/
 {
    const char *LastComponent = strrchr(File, '/');
-   char *RealPath = NULL;
-
-   if (LastComponent == NULL)
+   if (LastComponent == nullptr)
       LastComponent = File;
    else
-      LastComponent++;
+      ++LastComponent;
 
-   vector<string>::const_iterator I;
-   for(I = Owner->Patterns.begin(); I != Owner->Patterns.end(); ++I)
-   {
-      if (fnmatch((*I).c_str(), LastComponent, 0) == 0)
-         break;
-   }
-   if (I == Owner->Patterns.end())
+   return std::any_of(Patterns.cbegin(), Patterns.cend(), [&](std::string const &pattern) {
+      return fnmatch(pattern.c_str(), LastComponent, 0) == 0;
+   });
+}
+									/*}}}*/
+int FTWScanner::ScannerFile(const char *const File, bool const ReadLink) /*{{{*/
+{
+   if (FileMatchesPatterns(File, Owner->Patterns) == false)
       return 0;
 
+   Owner->FilesToProcess.emplace_back(File, ReadLink);
+   return 0;
+}
+									/*}}}*/
+int FTWScanner::ProcessFile(const char *const File, bool const ReadLink) /*{{{*/
+{
    /* Process it. If the file is a link then resolve it into an absolute
       name.. This works best if the directory components the scanner are
       given are not links themselves. */
    char Jnk[2];
+   char *RealPath = NULL;
    Owner->OriginalPath = File;
    if (ReadLink &&
        readlink(File,Jnk,sizeof(Jnk)) != -1 &&
@@ -187,12 +190,12 @@ int FTWScanner::ScannerFile(const char *File, bool const &ReadLink)
 /* */
 bool FTWScanner::RecursiveScan(string const &Dir)
 {
-   char *RealPath = NULL;
    /* If noprefix is set then jam the scan root in, so we don't generate
       link followed paths out of control */
    if (InternalPrefix.empty() == true)
    {
-      if ((RealPath = realpath(Dir.c_str(),NULL)) == 0)
+      char *RealPath = nullptr;
+      if ((RealPath = realpath(Dir.c_str(), nullptr)) == 0)
 	 return _error->Errno("realpath",_("Failed to resolve %s"),Dir.c_str());
       InternalPrefix = RealPath;
       free(RealPath);
@@ -209,7 +212,15 @@ bool FTWScanner::RecursiveScan(string const &Dir)
 	 _error->Errno("ftw",_("Tree walking failed"));
       return false;
    }
-   
+
+   using PairType = decltype(*FilesToProcess.cbegin());
+   std::sort(FilesToProcess.begin(), FilesToProcess.end(), [](PairType a, PairType b) {
+      return a.first < b.first;
+   });
+   for (PairType it : FilesToProcess)
+      if (ProcessFile(it.first.c_str(), it.second) != 0)
+	 return false;
+   FilesToProcess.clear();
    return true;
 }
 									/*}}}*/
@@ -219,14 +230,14 @@ bool FTWScanner::RecursiveScan(string const &Dir)
    of files from another file. */
 bool FTWScanner::LoadFileList(string const &Dir, string const &File)
 {
-   char *RealPath = NULL;
    /* If noprefix is set then jam the scan root in, so we don't generate
       link followed paths out of control */
    if (InternalPrefix.empty() == true)
    {
-      if ((RealPath = realpath(Dir.c_str(),NULL)) == 0)
+      char *RealPath = nullptr;
+      if ((RealPath = realpath(Dir.c_str(), nullptr)) == 0)
 	 return _error->Errno("realpath",_("Failed to resolve %s"),Dir.c_str());
-      InternalPrefix = RealPath;      
+      InternalPrefix = RealPath;
       free(RealPath);
    }
    
@@ -263,8 +274,10 @@ bool FTWScanner::LoadFileList(string const &Dir, string const &File)
       if (stat(FileName,&St) != 0)
 	 Flag = FTW_NS;
 #endif
+      if (FileMatchesPatterns(FileName, Patterns) == false)
+	 continue;
 
-      if (ScannerFile(FileName, false) != 0)
+      if (ProcessFile(FileName, false) != 0)
 	 break;
    }
   
diff --git a/ftparchive/writer.h b/ftparchive/writer.h
index b2cef4f00..b7c6435bf 100644
--- a/ftparchive/writer.h
+++ b/ftparchive/writer.h
@@ -19,6 +19,7 @@
 #include <map>
 #include <set>
 #include <string>
+#include <utility>
 #include <vector>
 #include <stdio.h>
 #include <stdlib.h>
@@ -39,6 +40,7 @@ class FTWScanner
 {
    protected:
    vector<string> Patterns;
+   vector<std::pair<string, bool>> FilesToProcess;
    string Arch;
    bool IncludeArchAll;
    const char *OriginalPath;
@@ -49,7 +51,8 @@ class FTWScanner
 
    static FTWScanner *Owner;
    static int ScannerFTW(const char *File,const struct stat *sb,int Flag);
-   static int ScannerFile(const char *File, bool const &ReadLink);
+   static int ScannerFile(const char *const File, bool const ReadLink);
+   static int ProcessFile(const char *const File, bool const ReadLink);
 
    bool Delink(string &FileName,const char *OriginalPath,
 	       unsigned long long &Bytes,unsigned long long const &FileSize);
diff --git a/test/integration/test-apt-ftparchive b/test/integration/test-apt-ftparchive
new file mode 100755
index 000000000..378b05944
--- /dev/null
+++ b/test/integration/test-apt-ftparchive
@@ -0,0 +1,64 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+
+setupenvironment
+
+buildsimplenativepackage 'baz' 'all' '1'
+buildsimplenativepackage 'foo' 'all' '1'
+buildsimplenativepackage 'bar' 'all' '2'
+buildsimplenativepackage 'bar' 'all' '1'
+
+EXPECT_PKG='Package: bar
+Version: 1
+Package: bar
+Version: 2
+Package: baz
+Version: 1
+Package: foo
+Version: 1'
+EXPECT_SRC="$EXPECT_PKG"
+
+linkfiles() {
+	ln -s "../incoming/${2}.dsc" "${1}/${2}.dsc"
+	ln -s "../incoming/${2}.tar.xz" "${1}/${2}.tar.xz"
+	ln -s "../incoming/${2}_all.deb" "${1}/${2}_all.deb"
+}
+genoptions() {
+	echo 'baz_1'
+	echo 'foo_1'
+	echo 'bar_2'
+	echo 'bar_1'
+}
+gencombos() {
+	for a in $(genoptions); do
+		for b in $(genoptions); do
+			if [ "$a" = "$b" ]; then continue; fi
+			for c in $(genoptions); do
+				if [ "$a" = "$c" -o "$b" = "$c" ]; then continue; fi
+				for d in $(genoptions); do
+					if [ "$a" = "$d" -o "$b" = "$d" -o "$c" = "$d" ]; then continue; fi
+					echo "${a};${b};${c};${d}"
+				done
+			done
+		done
+	done
+}
+for combo in $(gencombos); do
+	msgmsg 'Running apt-ftparchive in configuration' "$combo"
+	incomedir="incoming${combo}"
+	mkdir "$incomedir"
+	for i in $(echo "$combo" | tr ';' '\n'); do
+		linkfiles "$incomedir" "$i"
+	done
+
+	testsuccess aptftparchive packages "$incomedir"
+	cp rootdir/tmp/testsuccess.output aptarchive/Packages
+	testsuccessequal "$EXPECT_PKG" grep -e '^Package: ' -e '^Version: ' aptarchive/Packages
+
+	testsuccess aptftparchive -qq sources "$incomedir"
+	cp rootdir/tmp/testsuccess.output aptarchive/Sources
+	testsuccessequal "$EXPECT_SRC" grep -e '^Package: ' -e '^Version: ' aptarchive/Sources
+done