So, after looking a bit more closely: You weren't actually sorting (the predicate for std::sort expects a boolean return, but you were using string.compare() which returns an int like strcmp does) and calls with filelist hadn't changed there order as predicted by me but were "just" entirely broken as they weren't generating content anymore, so, in summary: no biggy. ;) Attached revised patch has a testcase for this as well, but before committing this to master I would prefer someone to run this against an actual repository first just in case as as said apt-ftparchive doesn't get a whole lot of attention and testing and I would like to avoid fixing regressions on emergency in stable buster. Best regards David Kalnischkies
From ec18a3647f678590ba4dc1112820fd19919ac0c8 Mon Sep 17 00:00:00 2001
From: David Kalnischkies <david@kalnischkies.de>
Date: Fri, 28 Jul 2017 18:20:14 +0200
Subject: [PATCH] ftparchive: sort discovered filenames before writing indexes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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
---
ftparchive/writer.cc | 57 +++++++++++++++++-----------
ftparchive/writer.h | 5 ++-
test/integration/test-apt-ftparchive | 72 ++++++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+), 23 deletions(-)
create mode 100755 test/integration/test-apt-ftparchive
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..eb4ea9617
--- /dev/null
+++ b/test/integration/test-apt-ftparchive
@@ -0,0 +1,72 @@
+#!/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='Package: bar
+Version: 1
+Package: bar
+Version: 2
+Package: baz
+Version: 1
+Package: foo
+Version: 1'
+
+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
--
2.13.2
Attachment:
signature.asc
Description: PGP signature