ALT Linux Bugzilla – #30482

Looks the same as https://bugzilla.altlinux.org/show_bug.cgi?id=29514

It works with apt from Sisyphus (after increasing Cache-Limit):

[root@z ~]# apt-get update 
Получено: 1 http://ftp.altlinux.org x86_64 release [914B]
Получено: 2 http://ftp.altlinux.org noarch release [708B]
Получено: 3 http://ftp.altlinux.org x86_64-i586 release [573B]
Получено: 4 http://ftp.altlinux.org x86_64 release [896B]
Получено: 5 http://ftp.altlinux.org noarch release [690B]
Получено: 6 http://ftp.altlinux.org x86_64-i586 release [555B]
Получено 4336B за 0s (43,5kB/s).             
Найдено http://ftp.altlinux.org x86_64/classic pkglist
Найдено http://ftp.altlinux.org x86_64/classic release
Найдено http://ftp.altlinux.org noarch/classic pkglist
Найдено http://ftp.altlinux.org noarch/classic release
Найдено http://ftp.altlinux.org x86_64-i586/classic pkglist
Найдено http://ftp.altlinux.org x86_64-i586/classic release
Получено: 1 http://ftp.altlinux.org x86_64/classic pkglist [16,0MB]
Получено: 2 http://ftp.altlinux.org x86_64/classic release [135B]                                                                                                                                                                           
Получено: 3 http://ftp.altlinux.org noarch/classic pkglist [3423kB]                                                                                                                                                                         
Получено: 4 http://ftp.altlinux.org noarch/classic release [135B]                                                                                                                                                                           
Получено: 5 http://ftp.altlinux.org x86_64-i586/classic pkglist [12,1MB]                                                                                                                                                                    
Получено: 6 http://ftp.altlinux.org x86_64-i586/classic release [140B]                                                                                                                                                                      
Получено 31,5MB за 27s (1160kB/s).                                                                                                                                                                                                          
Чтение списков пакетов... Ошибка!
E: Dynamic MMap ran out of room
E: При обработке i586-libwxGTK3.0.32bit (NewVersion1) возникла ошибка
E: Ошибка с MergeList /var/lib/apt/lists/ftp.altlinux.org_pub_distributions_ALTLinux_Sisyphus_x86%5f64-i586_base_pkglist.classic
E: Невозможно прочитать список пакетов или файл статуса.
[root@z ~]# apt-get -o APT::Cache-Limit=$(( 1024 * 1024 * 1024 / 2 )) update
Получено: 1 http://ftp.altlinux.org x86_64 release [914B]
Получено: 2 http://ftp.altlinux.org noarch release [708B]
Получено: 3 http://ftp.altlinux.org x86_64-i586 release [573B]
Получено: 4 http://ftp.altlinux.org x86_64 release [896B]
Получено: 5 http://ftp.altlinux.org noarch release [690B]
Получено: 6 http://ftp.altlinux.org x86_64-i586 release [555B]
Получено 4336B за 0s (41,4kB/s).           
Найдено http://ftp.altlinux.org x86_64/classic pkglist
Найдено http://ftp.altlinux.org x86_64/classic release
Найдено http://ftp.altlinux.org noarch/classic pkglist
Найдено http://ftp.altlinux.org noarch/classic release
Найдено http://ftp.altlinux.org x86_64-i586/classic pkglist
Найдено http://ftp.altlinux.org x86_64-i586/classic release
Найдено http://ftp.altlinux.org x86_64/classic pkglist
Найдено http://ftp.altlinux.org x86_64/classic release
Найдено http://ftp.altlinux.org noarch/classic pkglist
Найдено http://ftp.altlinux.org noarch/classic release
Найдено http://ftp.altlinux.org x86_64-i586/classic pkglist
Найдено http://ftp.altlinux.org x86_64-i586/classic release
Чтение списков пакетов... Завершено
Построение дерева зависимостей... Завершено
[root@z ~]# apt-repo
rpm [alt] http://ftp.altlinux.org/pub/distributions/ALTLinux/Sisyphus x86_64 classic
rpm [alt] http://ftp.altlinux.org/pub/distributions/ALTLinux/Sisyphus noarch classic
rpm [alt] http://ftp.altlinux.org/pub/distributions/ALTLinux/Sisyphus x86_64-i586 classic
rpm [updates] http://ftp.altlinux.org/pub/distributions/ALTLinux/p7/branch x86_64 classic
rpm [updates] http://ftp.altlinux.org/pub/distributions/ALTLinux/p7/branch noarch classic
rpm [updates] http://ftp.altlinux.org/pub/distributions/ALTLinux/p7/branch x86_64-i586 classic
[root@z ~]#

It works with apt from Sisyphus (after increasing Cache-Limit) -- apt-0.5.15lorg2-alt47 .

(В ответ на комментарий №1)
> Looks the same as https://bugzilla.altlinux.org/show_bug.cgi?id=29514

Probably, yes: 0.5.15lorg2-alt44 fixed that problem, and apt from Sisyphus doesn't have the current problem.

В p7 это чиниться не будет, а в Sisyphus предлагаю сделать динамическое выделение памяти в apt для кешей по аналогии с Debian.

(В ответ на комментарий №5)
> В p7 это чиниться не будет, а в Sisyphus предлагаю сделать динамическое
> выделение памяти в apt для кешей по аналогии с Debian.

Сделана тестовая сборка в задании #231978, ждёт аппрува.

Я установил себе пакеты из задания #231978 и проблем не заметил.
Стало чуть быстрее во время установки.
Увеличение количества репозиториев перестало приводить к ошибке  Dynamic MMap ran out of room

(In reply to comment #6)
> (В ответ на комментарий №5)
> > В p7 это чиниться не будет, а в Sisyphus предлагаю сделать динамическое
> > выделение памяти в apt для кешей по аналогии с Debian.
> 
> Сделана тестовая сборка в задании #231978, ждёт аппрува.

Спасибо за улучшения!

Так получилось, что я тут кое-что ещё замеченное недавно исправил в apt в 0.5.15lorg2-alt68.1 (задание 233886 или ветка alt у меня) -- на другую тему, но есть пересечения с затрагиваемым в этом патчсете механизме. Может, сначала 0.5.15lorg2-alt68.1?

* * *

Вот некоторые замечания, но не успел ещё все изменения посмотреть.
(Спасибо arei@ за обсуждение прочитанных.)

# b36a3a2 Fix every use of va_list: add proper cleanup

Замечаний нет.

# d854f27 strutil.cc: rework string iterating

Зачем менять

-   for (string::const_iterator I = Str.begin(); I != Str.end(); I++)
+   for (size_t i = 0; i < Str.size(); ++i)

? Может быть, это как в Debian?

# 17aa799 gpg.cc: fix potential memory leak

ok

Просто комментарий (не замечание):

В этом консервативном изменении делать необязательно, но можно ещё в
таких случаях использовать strdupa() (что, конечно, нехорошо с
т.зр. обработки ошибок). Да и не только в консерватизме дело, а том,
что strdupa() ускоряет работу, но для функций, на которые небольшая
нагрузка, это несущественно.

# 64bef88 RPM ArchiveURI: check file length before using it

ok

# 4ef776b Use 'override' keyword

ok (более строгий код; если что, компилятор сообщит об ошибке)

Проверили, что нет других изменений с помощью:

find -type f '-(' -name '*.cc' -o -name '*.h' '-)' -print0 | xargs -0 sed -i -re 's: override( |;|$):\1:'

# 0e8318e Use const reference to string instead of copies

Теоретически есть опасность, что в функциях возьмут указатель на
строку (ссылку) и через него поменяют значение снаружи или что-нибудь
в этом роде, но скорее всего так никто не делает в этом коде.

Проверяем что нет других нетривиальных изменений с помощью:

    find -type f '-(' -name '*.cc' -o -name '*.h' '-)' -print0 | xargs -0 sed -i -re 's:const string &:string :g'
    git commit ...
    git diff @~2.. -w

Видим нетривиальные изменения:

1. (может в отдельный коммит с объяснением?)

diff --git a/apt/apt-pkg/acquire-method.cc b/apt/apt-pkg/acquire-method.cc
index d72a984..b72a90c 100644
--- a/apt/apt-pkg/acquire-method.cc
+++ b/apt/apt-pkg/acquire-method.cc
@@ -93,10 +93,12 @@ void pkgAcqMethod::Fail(bool Transient)
 // AcqMethod::Fail - A fetch has failed                                        /*{{{*/
 // ---------------------------------------------------------------------
 /* */
-void pkgAcqMethod::Fail(string Err,bool Transient)
+void pkgAcqMethod::Fail(const char *Why, bool Transient)
 {
+   std::string Err = Why;
+
    // Strip out junk from the error messages
-   for (string::iterator I = Err.begin(); I != Err.end(); I++)
+   for (auto I = Err.begin(); I != Err.end(); ++I)
    {
       if (*I == '\r') 
         *I = ' ';
diff --git a/apt/apt-pkg/acquire-method.h b/apt/apt-pkg/acquire-method.h
index 42bd9d3..ac66e30 100644
--- a/apt/apt-pkg/acquire-method.h
+++ b/apt/apt-pkg/acquire-method.h
@@ -66,8 +66,8 @@ class pkgAcqMethod
    
    // Outgoing messages
    void Fail(bool Transient = false);
-   inline void Fail(const char *Why, bool Transient = false) {Fail(string(Why),Transient);};
-   void Fail(string Why, bool Transient = false);
+   inline void Fail(string Why, bool Transient = false) { Fail(Why.c_str(), Transient);};
+   void Fail(const char *Why, bool Transient = false);
    void URIStart(FetchResult &Res);
    void URIDone(FetchResult &Res,FetchResult *Alt = 0);
    bool MediaFail(string Required,string Drive);

2. Похожее изменение, но не подходит под commit message

diff --git a/apt/apt-pkg/algorithms.h b/apt/apt-pkg/algorithms.h
index 7b90623..d4ad7c6 100644
--- a/apt/apt-pkg/algorithms.h
+++ b/apt/apt-pkg/algorithms.h
@@ -109,9 +109,9 @@ class pkgProblemResolver
    
    public:
    
-   inline void Protect(pkgCache::PkgIterator Pkg) {Flags[Pkg->ID] |= Protected;};
-   inline void Remove(pkgCache::PkgIterator Pkg) {Flags[Pkg->ID] |= ToRemove;};
-   inline void Clear(pkgCache::PkgIterator Pkg) {Flags[Pkg->ID] &= ~(Protected | ToRemove);};
+   inline void Protect(const pkgCache::PkgIterator &Pkg) {Flags[Pkg->ID] |= Protected;};
+   inline void Remove(const pkgCache::PkgIterator &Pkg) {Flags[Pkg->ID] |= ToRemove;};
+   inline void Clear(const pkgCache::PkgIterator &Pkg) {Flags[Pkg->ID] &= ~(Protected | ToRemove);};
    
    // Try to intelligently resolve problems by installing and removing packages   
    bool Resolve(bool BrokenFix = false);

и ещё много похожих случаев. Но я думаю, можно расширить commit
message для других типов (помимо string). А проверить я смог их другим
sed:

    find -type f '-(' -name '*.cc' -o -name '*.h' '-)' -print0 | xargs -0 sed -i -re 's:const ([^ ]+) &:\1 :g'


3. тоже не подходит под commit message:

diff --git a/apt/apt-pkg/cacheiterators.h b/apt/apt-pkg/cacheiterators.h
index 3fcec06..6e69d51 100644
--- a/apt/apt-pkg/cacheiterators.h
+++ b/apt/apt-pkg/cacheiterators.h
@@ -186,7 +186,7 @@ class pkgCache::DepIterator
    inline unsigned long Index() const {return Dep - Owner->DepP;};
    // CNC:2003-02-17 - This is a very used function, so it's now
    //                 inlined here.
-   inline bool IsCritical()
+   inline bool IsCritical() const
                {
                        switch (Dep->Type) {
                                case pkgCache::Dep::Conflicts:

Отдельный коммит сделать?

4. Не подходит под commit message:

apt/apt-pkg/contrib/configuration.cc ---------------------
index a8a0e68..67d0c8a 100644
@@ -52,14 +52,14 @@ Configuration::Configuration(const Item *Root) : Root((Item *)Root), ToFree(fals
 };
 
 // CNC:2003-02-23 - Copy constructor.
-Configuration::Configuration(Configuration &Conf) : ToFree(true)
+Configuration::Configuration(const Configuration &Conf) : ToFree(true)
 {
    Root = new Item;
    if (Conf.Root->Child)
       CopyChildren(Conf.Root, Root);
 }
 
-void Configuration::CopyChildren(Item *From, Item *To)
+void Configuration::CopyChildren(const Item *From, Item *To)
 {
    Item *Parent = To;
    To->Child = new Item;
@@ -325,7 +325,7 @@ string Configuration::FindAny(const char *Name,const char *Default) const
 // Configuration::CndSet - Conditinal Set a value			/*{{{*/
 // ---------------------------------------------------------------------
 /* This will not overwrite */


и соответествующие изменения в configuration.h. (Лучше в отдельно
коммите?)

Конечно, добавление const хуже не сделает. Есть ещё такое:

diff --git a/apt/apt-pkg/contrib/progress.h b/apt/apt-pkg/contrib/progress.h
index a563d76..e9bb2ab 100644
--- a/apt/apt-pkg/contrib/progress.h
+++ b/apt/apt-pkg/contrib/progress.h
@@ -84,7 +84,7 @@ class OpTextProgress : public OpProgress
    
    OpTextProgress(bool NoUpdate = false) : NoUpdate(NoUpdate), 
                 NoDisplay(false), LastLen(0) {};
-   OpTextProgress(Configuration &Config);
+   OpTextProgress(const Configuration &Config);
    virtual ~OpTextProgress() {Done();};
 };
 
5. Использование stringstream вместо string (отдельным коммитом?)

diff --git a/apt/apt-pkg/contrib/strutl.cc b/apt/apt-pkg/contrib/strutl.cc
index c664833..732345c 100644
--- a/apt/apt-pkg/contrib/strutl.cc
+++ b/apt/apt-pkg/contrib/strutl.cc
@@ -322,26 +322,28 @@ string SubstVar(string Str,string Subst,string Contents)
 {
    string::size_type Pos = 0;
    string::size_type OldPos = 0;
-   string Temp;
+   std::stringstream Temp;
    
    while (OldPos < Str.length() && 
          (Pos = Str.find(Subst,OldPos)) != string::npos)
    {
-      Temp += string(Str,OldPos,Pos) + Contents;
+      Temp << string(Str,OldPos,Pos) << Contents;
       OldPos = Pos + Subst.length();      
    }
    
    if (OldPos == 0)
       return Str;
    
-   return Temp + string(Str,OldPos);
+   Temp << string(Str,OldPos);
+   return Temp.str();
 }
 
6. Тоже другое (в том же файле):

diff --git a/apt/apt-pkg/contrib/strutl.cc b/apt/apt-pkg/contrib/strutl.cc
index c664833..732345c 100644
--- a/apt/apt-pkg/contrib/strutl.cc
+++ b/apt/apt-pkg/contrib/strutl.cc
@@ -353,14 +355,14 @@ string URItoFileName(string URI)
 {
    // Nuke 'sensitive' items
    ::URI U(URI);
-   U.User = string();
-   U.Password = string();
-   U.Access = "";
+   U.User.clear();
+   U.Password.clear();
+   U.Access.clear();
    
    // "\x00-\x20{}|\\\\^\\[\\]<>\"\x7F-\xFF";
    URI = QuoteString(U,"\\|{}[]<>\"^~_=!@#$%^&*");
-   string::iterator J = URI.begin();
-   for (; J != URI.end(); J++)
+   auto J = URI.begin();
+   for (; J != URI.end(); ++J)
       if (*J == '/') 
         *J = '_';
    return URI;

@@ -1178,9 +1180,9 @@ URI::operator string()
 string URI::SiteOnly(string URI)
 {
    ::URI U(URI);
-   U.User = string();
-   U.Password = string();
-   U.Path = string();
+   U.User.clear();
+   U.Password.clear();
+   U.Path.clear();
    U.Port = 0;
    return U;
 }

В остальном вроде ok.

# 4ed2c0e wrap the mmap actions in the CacheGenerator in their own methods to be able to react on condition changes later then we can move mmap

Для аргумента типа string выбранная реализация, кажется, хуже
(неоптимальна):

+   unsigned long WriteStringInMap(const std::string &String) { return WriteStringInMap(String.c_str()); };


чем в аналогичном коде в mmap.h:

   inline unsigned long WriteString(string S) {return WriteString(S.c_str(),S.length());};

без необязательного параметра-длины будет лишний вызов strlen(), хотя
можно узнать значение из String.length().

# 33509fe Use references instead of copies in the Cache generation methods

В то, что ничего не портится, в этом изменении гораздо труднее
поверить или проверить, потому что не добавляется к ссылкам const.

Можно ли как-то попробовать с const это всё сделать?

# a523050 Support large files

В StrToNum() char S[30] не хватит для двоичной записи 32-битного или
64-битного числа.

В остальном ok. (Конечно, в http.c sscanf() может прочитать большое
unsigned long long в StartPos, который объявлен и в других местах
используется как signed long long. Т.е. будет как бы отрицательное
число. Что произойдёт?..)

Проверили с помощью:

    find -type f '-(' -name '*.cc' -o -name '*.h' '-)' -print0 | xargs -0 sed -i -re 's:(unsigned long) long:\1:g; s:%ll:%l:g'


# 55b9b4f apt-pkg/pkgcachegen.{cc,h} changes

Может быть, здесь во всех ReMap(), чтобы не ошибиться в типах, использовать
обозначение для типов Pkg, Ver, Prv и т.п., определяющее тип по
переменной. (Где-то я видел decltype(...) или что-то в этом роде.) Ну или
шаблон, может быть. Точнее даже не чтобы не ошибиться сейчас, а чтобы в
будущем, в случае изменений оно не разъезжалось, а не тихо
пропускалось компилятором.

# 78c93fe Add and document APT::Cache-{Start,Grow,Limit} options for mmap control
# bdf8763 DynamicMMap::Grow: add optional debug output
# be0c967 Use special type to return allocation failure since 0 is a valid offset value

Почему бы не выкинуть свою реализацию, а использовать std::optional

# 112d2ea Remove ABI compat stuff
# f56c14e Improve allocation failure error message
# 111ef88 Add workaround for packages with missing tags
# cfe4ad9 Bump soname
# 45b01f6 (tag: 0.5.15lorg2-alt69) 0.5.15lorg2-alt69
# 7c4fc38 Port pkgCacheFile::GetSourceList and it's dependencies from Debian
# 89457f4 Port ListUpdate function from Debian
# 99c05dc (HEAD -> sisyphus, tag: 0.5.15lorg2-alt70, darktemplar@ALT/sisyphus, darktemplar@ALT/HEAD) 0.5.15lorg2-alt70

(В ответ на комментарий №8)
> (In reply to comment #6)
> > (В ответ на комментарий №5)
> > > В p7 это чиниться не будет, а в Sisyphus предлагаю сделать динамическое
> > > выделение памяти в apt для кешей по аналогии с Debian.
> > 
> > Сделана тестовая сборка в задании #231978, ждёт аппрува.
> 
> Спасибо за улучшения!
> 
> Так получилось, что я тут кое-что ещё замеченное недавно исправил в apt в
> 0.5.15lorg2-alt68.1 (задание 233886 или ветка alt у меня) -- на другую тему, но
> есть пересечения с затрагиваемым в этом патчсете механизме. Может, сначала
> 0.5.15lorg2-alt68.1?
> 

Меня устроит любой порядок. Если надо, я сделаю rebase своих изменений.

> * * *
> 
> Вот некоторые замечания, но не успел ещё все изменения посмотреть.
> (Спасибо arei@ за обсуждение прочитанных.)
> 
> # b36a3a2 Fix every use of va_list: add proper cleanup
> 
> Замечаний нет.
> 
> # d854f27 strutil.cc: rework string iterating
> 
> Зачем менять
> 
> -   for (string::const_iterator I = Str.begin(); I != Str.end(); I++)
> +   for (size_t i = 0; i < Str.size(); ++i)
> 
> ? Может быть, это как в Debian?
> 

Когда есть возможность сделать лучше, я стараюсь так сделать. Даже если это не как в Debian.

Парой строк ниже было:
  if (*I == '%' && I + 2 < Str.end())

Допустим, I + 1 == Str.end(), тогда является ли I + 2 валидным итератором и можно ли его использовать (в том числе для сравнения) или это UB?
Даже если оно работает сейчас, это не значит что при других входных данных или после сборки новой версией компилятора оно продолжит работать.
Поэтому я поменял такие места.

> # 17aa799 gpg.cc: fix potential memory leak
> 
> ok
> 
> Просто комментарий (не замечание):
> 
> В этом консервативном изменении делать необязательно, но можно ещё в
> таких случаях использовать strdupa() (что, конечно, нехорошо с
> т.зр. обработки ошибок). Да и не только в консерватизме дело, а том,
> что strdupa() ускоряет работу, но для функций, на которые небольшая
> нагрузка, это несущественно.
> 
> # 64bef88 RPM ArchiveURI: check file length before using it
> 
> ok
> 
> # 4ef776b Use 'override' keyword
> 
> ok (более строгий код; если что, компилятор сообщит об ошибке)
> 
> Проверили, что нет других изменений с помощью:
> 
> find -type f '-(' -name '*.cc' -o -name '*.h' '-)' -print0 | xargs -0 sed -i
> -re 's: override( |;|$):\1:'
> 
> # 0e8318e Use const reference to string instead of copies
> 
> Теоретически есть опасность, что в функциях возьмут указатель на
> строку (ссылку) и через него поменяют значение снаружи или что-нибудь
> в этом роде, но скорее всего так никто не делает в этом коде.
> 
> Проверяем что нет других нетривиальных изменений с помощью:
> 
>     find -type f '-(' -name '*.cc' -o -name '*.h' '-)' -print0 | xargs -0 sed
> -i -re 's:const string &:string :g'
>     git commit ...
>     git diff @~2.. -w
> 
> Видим нетривиальные изменения:
> 

Да, изначально изменение было только про строки, но потом нашёл и несколько мест с другими
данными. А commit message остался старый. Это практически всё релевантные изменения. Я перепишу commit message.
Все эти изменения избавляют от излишнего копирования различных объектов.
Не всегда это замена копии на const reference, в нескольких местах меняется string на const char*,
при этом это тоже просто избавление от ненужного копирования.

> 1. (может в отдельный коммит с объяснением?)
> 

Это разве что единственное место, где от копирования совсем избавиться не удалось,
но я всё равно оставил это изменение.

> diff --git a/apt/apt-pkg/acquire-method.cc b/apt/apt-pkg/acquire-method.cc
> index d72a984..b72a90c 100644
> --- a/apt/apt-pkg/acquire-method.cc
> +++ b/apt/apt-pkg/acquire-method.cc
> @@ -93,10 +93,12 @@ void pkgAcqMethod::Fail(bool Transient)
>  // AcqMethod::Fail - A fetch has failed                                       
> /*{{{*/
>  // ---------------------------------------------------------------------
>  /* */
> -void pkgAcqMethod::Fail(string Err,bool Transient)
> +void pkgAcqMethod::Fail(const char *Why, bool Transient)
>  {
> +   std::string Err = Why;
> +
>     // Strip out junk from the error messages
> -   for (string::iterator I = Err.begin(); I != Err.end(); I++)
> +   for (auto I = Err.begin(); I != Err.end(); ++I)
>     {
>        if (*I == '\r') 
>          *I = ' ';
> diff --git a/apt/apt-pkg/acquire-method.h b/apt/apt-pkg/acquire-method.h
> index 42bd9d3..ac66e30 100644
> --- a/apt/apt-pkg/acquire-method.h
> +++ b/apt/apt-pkg/acquire-method.h
> @@ -66,8 +66,8 @@ class pkgAcqMethod
> 
>     // Outgoing messages
>     void Fail(bool Transient = false);
> -   inline void Fail(const char *Why, bool Transient = false)
> {Fail(string(Why),Transient);};
> -   void Fail(string Why, bool Transient = false);
> +   inline void Fail(string Why, bool Transient = false) { Fail(Why.c_str(),
> Transient);};
> +   void Fail(const char *Why, bool Transient = false);
>     void URIStart(FetchResult &Res);
>     void URIDone(FetchResult &Res,FetchResult *Alt = 0);
>     bool MediaFail(string Required,string Drive);
> 
> 2. Похожее изменение, но не подходит под commit message
> 
> diff --git a/apt/apt-pkg/algorithms.h b/apt/apt-pkg/algorithms.h
> index 7b90623..d4ad7c6 100644
> --- a/apt/apt-pkg/algorithms.h
> +++ b/apt/apt-pkg/algorithms.h
> @@ -109,9 +109,9 @@ class pkgProblemResolver
> 
>     public:
> 
> -   inline void Protect(pkgCache::PkgIterator Pkg) {Flags[Pkg->ID] |=
> Protected;};
> -   inline void Remove(pkgCache::PkgIterator Pkg) {Flags[Pkg->ID] |=
> ToRemove;};
> -   inline void Clear(pkgCache::PkgIterator Pkg) {Flags[Pkg->ID] &= ~(Protected
> | ToRemove);};
> +   inline void Protect(const pkgCache::PkgIterator &Pkg) {Flags[Pkg->ID] |=
> Protected;};
> +   inline void Remove(const pkgCache::PkgIterator &Pkg) {Flags[Pkg->ID] |=
> ToRemove;};
> +   inline void Clear(const pkgCache::PkgIterator &Pkg) {Flags[Pkg->ID] &=
> ~(Protected | ToRemove);};
> 
>     // Try to intelligently resolve problems by installing and removing
> packages   
>     bool Resolve(bool BrokenFix = false);
> 
> и ещё много похожих случаев. Но я думаю, можно расширить commit
> message для других типов (помимо string). А проверить я смог их другим
> sed:
> 
>     find -type f '-(' -name '*.cc' -o -name '*.h' '-)' -print0 | xargs -0 sed
> -i -re 's:const ([^ ]+) &:\1 :g'
> 
> 
> 3. тоже не подходит под commit message:
> 
> diff --git a/apt/apt-pkg/cacheiterators.h b/apt/apt-pkg/cacheiterators.h
> index 3fcec06..6e69d51 100644
> --- a/apt/apt-pkg/cacheiterators.h
> +++ b/apt/apt-pkg/cacheiterators.h
> @@ -186,7 +186,7 @@ class pkgCache::DepIterator
>     inline unsigned long Index() const {return Dep - Owner->DepP;};
>     // CNC:2003-02-17 - This is a very used function, so it's now
>     //                 inlined here.
> -   inline bool IsCritical()
> +   inline bool IsCritical() const
>                 {
>                         switch (Dep->Type) {
>                                 case pkgCache::Dep::Conflicts:
> 
> Отдельный коммит сделать?
> 

Я перепишу коммит message, как я писал выше.
Но это релевантное изменение.
В некоторых местах теперь используется const pkgCache::DepIterator& вместо pkgCache::DepIterator.
Если не отметить эту функцию как const, то это приведёт к ошибкам сборки,
поскольку будут попытки использовать не-const функцию у const объекта.
Добавление же такого атрибута не имеет минусов в данном случае.

> 4. Не подходит под commit message:
> 
> apt/apt-pkg/contrib/configuration.cc ---------------------
> index a8a0e68..67d0c8a 100644
> @@ -52,14 +52,14 @@ Configuration::Configuration(const Item *Root) : Root((Item
> *)Root), ToFree(fals
>  };
> 
>  // CNC:2003-02-23 - Copy constructor.
> -Configuration::Configuration(Configuration &Conf) : ToFree(true)
> +Configuration::Configuration(const Configuration &Conf) : ToFree(true)
>  {
>     Root = new Item;
>     if (Conf.Root->Child)
>        CopyChildren(Conf.Root, Root);
>  }
> 
> -void Configuration::CopyChildren(Item *From, Item *To)
> +void Configuration::CopyChildren(const Item *From, Item *To)
>  {
>     Item *Parent = To;
>     To->Child = new Item;
> @@ -325,7 +325,7 @@ string Configuration::FindAny(const char *Name,const char
> *Default) const
>  // Configuration::CndSet - Conditinal Set a value            /*{{{*/
>  // ---------------------------------------------------------------------
>  /* This will not overwrite */
> 
> 
> и соответествующие изменения в configuration.h. (Лучше в отдельно
> коммите?)
> 
> Конечно, добавление const хуже не сделает. Есть ещё такое:
> 
> diff --git a/apt/apt-pkg/contrib/progress.h b/apt/apt-pkg/contrib/progress.h
> index a563d76..e9bb2ab 100644
> --- a/apt/apt-pkg/contrib/progress.h
> +++ b/apt/apt-pkg/contrib/progress.h
> @@ -84,7 +84,7 @@ class OpTextProgress : public OpProgress
> 
>     OpTextProgress(bool NoUpdate = false) : NoUpdate(NoUpdate), 
>                  NoDisplay(false), LastLen(0) {};
> -   OpTextProgress(Configuration &Config);
> +   OpTextProgress(const Configuration &Config);
>     virtual ~OpTextProgress() {Done();};
>  };
> 
> 5. Использование stringstream вместо string (отдельным коммитом?)
> 

Пожалуй, эту часть можно выкинуть.

> diff --git a/apt/apt-pkg/contrib/strutl.cc b/apt/apt-pkg/contrib/strutl.cc
> index c664833..732345c 100644
> --- a/apt/apt-pkg/contrib/strutl.cc
> +++ b/apt/apt-pkg/contrib/strutl.cc
> @@ -322,26 +322,28 @@ string SubstVar(string Str,string Subst,string Contents)
>  {
>     string::size_type Pos = 0;
>     string::size_type OldPos = 0;
> -   string Temp;
> +   std::stringstream Temp;
> 
>     while (OldPos < Str.length() && 
>           (Pos = Str.find(Subst,OldPos)) != string::npos)
>     {
> -      Temp += string(Str,OldPos,Pos) + Contents;
> +      Temp << string(Str,OldPos,Pos) << Contents;
>        OldPos = Pos + Subst.length();      
>     }
> 
>     if (OldPos == 0)
>        return Str;
> 
> -   return Temp + string(Str,OldPos);
> +   Temp << string(Str,OldPos);
> +   return Temp.str();
>  }
> 
> 6. Тоже другое (в том же файле):
> 

Ок, вынесу в отдельный коммит.

> diff --git a/apt/apt-pkg/contrib/strutl.cc b/apt/apt-pkg/contrib/strutl.cc
> index c664833..732345c 100644
> --- a/apt/apt-pkg/contrib/strutl.cc
> +++ b/apt/apt-pkg/contrib/strutl.cc
> @@ -353,14 +355,14 @@ string URItoFileName(string URI)
>  {
>     // Nuke 'sensitive' items
>     ::URI U(URI);
> -   U.User = string();
> -   U.Password = string();
> -   U.Access = "";
> +   U.User.clear();
> +   U.Password.clear();
> +   U.Access.clear();
> 
>     // "\x00-\x20{}|\\\\^\\[\\]<>\"\x7F-\xFF";
>     URI = QuoteString(U,"\\|{}[]<>\"^~_=!@#$%^&*");
> -   string::iterator J = URI.begin();
> -   for (; J != URI.end(); J++)
> +   auto J = URI.begin();
> +   for (; J != URI.end(); ++J)
>        if (*J == '/') 
>          *J = '_';
>     return URI;
> 
> @@ -1178,9 +1180,9 @@ URI::operator string()
>  string URI::SiteOnly(string URI)
>  {
>     ::URI U(URI);
> -   U.User = string();
> -   U.Password = string();
> -   U.Path = string();
> +   U.User.clear();
> +   U.Password.clear();
> +   U.Path.clear();
>     U.Port = 0;
>     return U;
>  }
> 
> В остальном вроде ok.
> 
> # 4ed2c0e wrap the mmap actions in the CacheGenerator in their own methods to
> be able to react on condition changes later then we can move mmap
> 
> Для аргумента типа string выбранная реализация, кажется, хуже
> (неоптимальна):
> 
> +   unsigned long WriteStringInMap(const std::string &String) { return
> WriteStringInMap(String.c_str()); };
> 
> 
> чем в аналогичном коде в mmap.h:
> 
>    inline unsigned long WriteString(string S) {return
> WriteString(S.c_str(),S.length());};
> 
> без необязательного параметра-длины будет лишний вызов strlen(), хотя
> можно узнать значение из String.length().
> 

Да, действительно так. Я поправлю.

> # 33509fe Use references instead of copies in the Cache generation methods
> 
> В то, что ничего не портится, в этом изменении гораздо труднее
> поверить или проверить, потому что не добавляется к ссылкам const.
> 
> Можно ли как-то попробовать с const это всё сделать?
> 

Как и написано в commit message, это порт изменений из Debian.
Commit 32b9a14cb4c6bdcddfe84c4451c225ced1a34bb7 из репозитория Apt Debian.
Они нужны именно в таком виде для последующих изменений, насколько я понял эти изменения в Debian.

> # a523050 Support large files
> 
> В StrToNum() char S[30] не хватит для двоичной записи 32-битного или
> 64-битного числа.
> 
> В остальном ok. (Конечно, в http.c sscanf() может прочитать большое
> unsigned long long в StartPos, который объявлен и в других местах
> используется как signed long long. Т.е. будет как бы отрицательное
> число. Что произойдёт?..)
> 

В Debian сейчас тоже S[30] используется, но я увеличу этот буфер.
Посмотрел код http.cc. Если StartPos будет отрицательным, то будет пропущен вызов ftruncate для скачиваемого файла.

> Проверили с помощью:
> 
>     find -type f '-(' -name '*.cc' -o -name '*.h' '-)' -print0 | xargs -0 sed
> -i -re 's:(unsigned long) long:\1:g; s:%ll:%l:g'
> 
> 
> # 55b9b4f apt-pkg/pkgcachegen.{cc,h} changes
> 
> Может быть, здесь во всех ReMap(), чтобы не ошибиться в типах, использовать
> обозначение для типов Pkg, Ver, Prv и т.п., определяющее тип по
> переменной. (Где-то я видел decltype(...) или что-то в этом роде.) Ну или
> шаблон, может быть. Точнее даже не чтобы не ошибиться сейчас, а чтобы в
> будущем, в случае изменений оно не разъезжалось, а не тихо
> пропускалось компилятором.
> 

Не вижу где тут можно ошибиться в типах.
В Debian у этих итераторов вообще часть функций вынесена в отдельный класс-предок,
и функция ReMap() добавлена к этому классу. Такие изменения я пока что не портировал.

> # 78c93fe Add and document APT::Cache-{Start,Grow,Limit} options for mmap
> control
> # bdf8763 DynamicMMap::Grow: add optional debug output
> # be0c967 Use special type to return allocation failure since 0 is a valid
> offset value
> 
> Почему бы не выкинуть свою реализацию, а использовать std::optional
> 

Насколько я знаю, для этого придётся поднимать требование до c++17.
Можно ещё взять boost::optional из boost, но тащить такую зависимость я пока не хотел бы.

> # 112d2ea Remove ABI compat stuff
> # f56c14e Improve allocation failure error message
> # 111ef88 Add workaround for packages with missing tags
> # cfe4ad9 Bump soname
> # 45b01f6 (tag: 0.5.15lorg2-alt69) 0.5.15lorg2-alt69
> # 7c4fc38 Port pkgCacheFile::GetSourceList and it's dependencies from Debian
> # 89457f4 Port ListUpdate function from Debian
> # 99c05dc (HEAD -> sisyphus, tag: 0.5.15lorg2-alt70, darktemplar@ALT/sisyphus,
> darktemplar@ALT/HEAD) 0.5.15lorg2-alt70

> 2019-Jul-11 12:05:34 :: task #231978 for sisyphus EPERM

Поправил согласно замечаниям и сделал rebase на 0.5.15lorg2-alt68.1.

2 bircoph: это про comment 9 (c++17); несрочно.

(In reply to comment #9)
> (В ответ на комментарий №8)
[...]
> > # 78c93fe Add and document APT::Cache-{Start,Grow,Limit} options for mmap
> > control
> > # bdf8763 DynamicMMap::Grow: add optional debug output
> > # be0c967 Use special type to return allocation failure since 0 is a valid
> > offset value
> > 
> > Почему бы не выкинуть свою реализацию, а использовать std::optional
> > 
> 
> Насколько я знаю, для этого придётся поднимать требование до c++17.
> Можно ещё взять boost::optional из boost, но тащить такую зависимость я пока не
> хотел бы.

Очень прошу никаких C++17 и поменьше C++14, иначе это всё мне придётся переписывать на C++11 под Эльбрус.

> 2019-Jul-22 13:04:16 :: task #231978 for sisyphus EPERM

Сделал rebase на версию alt69 из Сизифа, откатил требования до с++14.

[#231978] DONE (try 31) apt.git=0.5.15lorg2-alt70 apt-repo-tools.git=apt-repo-tools-0.6.0.22-alt1 apt-indicator.git=0.3.12-alt4 perl-AptPkg.git=0.1.26-alt5 aptitude.git=0.4.5-alt12 PackageKit.git=1.1.12-alt6 synaptic.git=0.58-alt23

Коллеги, спасибо!