More on Meson build plus fix for Brewtarget/brewtarget#685

meson.build

@@ -745,19 +745,19 @@ filesToInstall_macIcons = files([
    'mac/' + capitalisedProjectName + 'Icon.icns'
 ])
 
-filesToInstall_changeLogUncompressed = files([
-   'CHANGES.markdown'
-])
+# This has to be a string because we're going to pass it into a script.
+# AFAICT Meson does not provide a way for you to extract, say, full path from a file object
+filesToInstall_changeLogUncompressed = 'CHANGES.markdown'
 
 filesToInstall_copyright = files([
    'COPYRIGHT'
 ])
 
-# This is not wrapped in files() because it's not yet created!
+# This is not wrapped in files() (a) because it's not yet created and (b) because, as with
+# filesToInstall_changeLogUncompressed, we're going to pass it into a bash script
+#
 # See below for how this one gets created from filesToInstall_changeLogUncompressed
-filesToInstall_changeLogCompressed = [
-   'changelog.gz'
-]
+filesToInstall_changeLogCompressed = 'changelog.gz'
 
 #=======================================================================================================================
 #============================================ Installation sub-directories =============================================
@@ -850,6 +850,21 @@ configure_file(input : 'config.in',
                output : 'config.h',
                configuration : conf_data)
 
+#=======================================================================================================================
+#=================================== Generate compressed changelog for Debian package ==================================
+#=======================================================================================================================
+# TODO do this only when we're packaging, not on all builds!
+scriptParams = environment()
+scriptParams.set('build_projectName'          , projectName                         )
+scriptParams.set('build_changeLogUncompressed', join_paths(meson.current_source_dir(), filesToInstall_changeLogUncompressed))
+scriptParams.set('build_changeLogCompressed'  , join_paths(meson.current_build_dir(),  filesToInstall_changeLogCompressed  ))
+scriptParams.set('build_packageMaintainer'    , 'Matt Young <mfsy@yahoo.com>'       )
+scriptResult = run_command('packaging/generateCompressedChangeLog.sh', env : scriptParams, check : true)
+message('Generating compressed changlog output:', scriptResult.stdout().strip())
+if scriptResult.returncode() != 0
+   error('Generating compressed changlog ERRORS:', scriptResult.stderr().strip())
+endif
+
 #=======================================================================================================================
 #======================= Install files that we ship with the software (sounds, default DB, etc) ========================
 #=======================================================================================================================
@@ -878,10 +893,11 @@ if host_machine.system() == 'linux'
    # contents).  So the simplest thing is to rename COPYRIGHT to copyright for both.
    install_data(filesToInstall_copyright, rename : 'copyright', install_dir : installSubDir_doc)
 
-   # TODO: Need to generate this file
    # Each Debian package (which provides a /usr/share/doc/pkg directory) must install a Debian changelog file in
    # /usr/share/doc/pkg/changelog.Debian.gz
-   # install_data(filesToInstall_changeLogCompressed, rename : 'changelog.Debian.gz', install_dir : installSubDir_doc)
+   install_data(join_paths(meson.current_build_dir(), filesToInstall_changeLogCompressed),
+                rename : 'changelog.Debian.gz',
+                install_dir : installSubDir_doc)
 else
    #----------- Windows and Mac -----------
    install_data(filesToInstall_copyright, install_dir : installSubDir_doc)
@@ -916,16 +932,11 @@ if meson.get_compiler('cpp').get_id() == 'gcc'
    # -fno-pie   } https://stackoverflow.com/questions/52583544/boost-stack-trace-not-showing-function-names-and-line-numbers
    # -rdynamic  } But, for some reason, gcc on Windows does not accept -rdynamic
    #
-   #
-   #
    add_global_arguments(['-g3',
                          '-O2',
                          '-z', 'noexecstack', # NB Not '-z noexecstack' as otherwise will be passed to gcc in quotes!
                          '-no-pie',
                          '-fno-pie'], language : 'cpp')
-##   add_global_arguments(['-Wno-long-long',
-##                         '-no-pie',
-##                         '-fno-pie'], language : 'cpp')
    if host_machine.system() != 'windows'
       add_global_arguments('-rdynamic', language : 'cpp')
    endif

packaging/generateCompressedChangeLog.sh

@@ -0,0 +1,130 @@
+#!/bin/bash
+#----------------------------------------------------------------------------------------------------------------------
+# packaging/generateCompressedChangeLog.sh is part of Brewken, and is copyright the following authors 2022:
+#   • Matt Young <mfsy@yahoo.com>
+#
+# Brewken is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# Brewken is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied
+# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with this program.  If not, see
+# <http://www.gnu.org/licenses/>.
+#----------------------------------------------------------------------------------------------------------------------
+
+#----------------------------------------------------------------------------------------------------------------------
+# NB: This script is intended to be invoked from the Meson build (see ../meson.build) with the following environment
+# variables set:
+#    build_projectName           - Same as projectName in meson.build
+#    build_changeLogUncompressed - Same as filesToInstall_changeLogUncompressed in meson.build
+#    build_changeLogCompressed   - Same as filesToInstall_changeLogCompressed in meson.build
+#    build_packageMaintainer     - Name and email of a project maintainer conforming to
+#                                  https://www.debian.org/doc/debian-policy/ch-binary.html#s-maintainer
+#
+# We assume that none of these variables contains single or double quotes (so we can save ourselves having to escape
+# the values when we use them below).
+#
+# First thing we do is check that all these variables are set to something.
+#----------------------------------------------------------------------------------------------------------------------
+for var in build_projectName build_changeLogUncompressed build_changeLogCompressed build_packageMaintainer
+do
+   if [ -z "${!var}" ]
+   then
+      echo "ERROR $var is unset or blank" >&2
+      exit 1
+   fi
+done
+
+echo "Parsing ${build_changeLogUncompressed}"
+
+#
+# The rest of this script creates a compressed changelog in a Debian-friendly format
+#
+# Our change log (CHANGES.markdown) uses markdown format, with the following raw structure:
+#    ## v1.2.3
+#
+#    Optional one-line description of the release.
+#
+#    ### New Features
+#
+#    * Blah blah blah
+#    * etc
+#
+#    ### Bug Fixes
+#
+#    * Blah blah blah
+#    * etc
+#
+#    ### Incompatibilities
+#
+#    None
+#
+#    ### Release Timestamp
+#    Sun, 06 Feb 2022 12:02:58 +0100
+#
+# However, per https://www.debian.org/doc/debian-policy/ch-source.html#debian-changelog-debian-changelog, Debian change
+# logs need to be in the following format:
+#    package (version) distribution(s); urgency=urgency
+#      [optional blank line(s), stripped]
+#      * change details
+#      more change details
+#      [blank line(s), included in output of dpkg-parsechangelog]
+#      * even more change details
+#      [optional blank line(s), stripped]
+#     -- maintainer name <email address>[two spaces]  date
+#
+# We are being a bit fast-and-loose in hard-coding the same maintainer name for each release, but I don't thing it's a
+# huge issue.
+#
+# Note that, to keep us on our toes, Debian change log lines are not supposed to be more than 80 characters long.  This
+# is non-trivial, but the ghastly bit of awk below gets us most of the way there.
+#
+cat "${build_changeLogUncompressed}" |
+   # Skip over the introductory headings and paragraphs of CHANGES.markdown until we get to the first version line
+   sed -n '/^## v/,$p' |
+   # We want to change the release timestamp to maintainer + timestamp, but we don't want to create too long a line
+   # before we do the fold command below, so use "÷÷maintainer÷÷" as a placeholder for
+   # " -- ${build_packageMaintainer}  "
+   sed -z "s/\\n### Release Timestamp\\n\\([^\\n]*\\)\\n/\\n÷÷maintainer÷÷\\1\\n/g" |
+   # Join continued lines in bullet lists
+   sed -z "s/\\n  / /g" |
+   # Change the version to package (version) etc.  Stick a '÷' on the front of the line to protect it from
+   # modification below
+   sed "s/^## v\\(.*\\)$/÷${build_projectName} (\\1-1) unstable\; urgency=low/" |
+   # Change bullets to sub-bullets
+   sed "s/^\\* /    - /" |
+   # Change headings to bullets
+   sed "s/^### /  * /" |
+   # Change any lines that don't start with space OR a ÷ character to be bullets
+   sed "s/^\\([^ ÷]\\)/  * \\1/" |
+   # Split any long lines.  Make the width less than 80 so we've got a margin go insert spaces at the start of
+   # bullet continuation lines.
+   fold -s --width=72 |
+   # With a lot of help from awk, reindent the lines that were split off from a long bullet line so that they align
+   # with that previous line.
+   awk "BEGIN { inBullet=0 }
+   {
+      if (!inBullet) {
+         inBullet=match(\$0, \"^( +)[^ ] \", spaces);
+         print;
+      } else {
+         bulletContinues=match(\$0, \"^[^ ]\");
+         if (!bulletContinues) {
+            inBullet=match(\$0, \"^( +)[^ ] \", spaces);
+            print;
+         } else {
+            print spaces[1] \"  \" \$0;
+         }
+      }
+   }" |
+   # Fix the "÷÷maintainer÷÷" placeholders
+   sed "s/÷÷maintainer÷÷/ -- ${build_packageMaintainer}  /" |
+   # Remove the protective "÷" from the start of any other lines
+   sed "s/^÷//" |
+   gzip --best -n --to-stdout > "${build_changeLogCompressed}"
+
+echo "Wrote to ${build_changeLogCompressed}"
+exit 0

src/WaterDialog.cpp

@@ -347,8 +347,7 @@ void WaterDialog::newTotals() {
       }
       btDigit_ph->setText( calculateMashpH(), 2 );
 
-   }
-   else {
+   } else {
       for (int i = 0; i < static_cast<int>(Water::Ions::numIons); ++i ) {
          Water::Ions ion = static_cast<Water::Ions>(i);
          m_ppm_digits[i]->setText( m_salt_table_model->total(ion) / allTheWaters, 0 );
@@ -364,6 +363,7 @@ void WaterDialog::removeSalts() {
       deadSalts.append( i.row() );
    }
    m_salt_table_model->removeSalts(deadSalts);
+   return;
 }
 
 //! \brief Calcuates the residual alkalinity of the mash water.
@@ -410,8 +410,7 @@ double WaterDialog::calculateSaltpH() {
 }
 
 //! \brief Calculates the pH delta caused by any salt additions.
-double WaterDialog::calculateAddedSaltpH()
-{
+double WaterDialog::calculateAddedSaltpH() {
 
    // We need the value from the salt table model, because we need all the
    // added salts, but not the base.

src/database/ObjectStoreWrapper.h

@@ -101,9 +101,12 @@ namespace ObjectStoreWrapper {
       if (id > 0 && objectStore.contains(id)) {
          return objectStore.getById(id);
       }
-      qDebug() <<
+      // If the object isn't stored in the DB then we can create a shared pointer for it, but this is dangerous as there
+      // might already be another shared pointer to it.  At minimum we should log a warning.  In the long run we should
+      // Q_ASSERT(false) here.
+      qWarning() <<
          Q_FUNC_INFO << "Creating new shared_ptr for unstored" << ne->metaObject()->className() << "#" << id << " :" <<
-         ne->name();
+         ne->name() << ".  This may be a bug - eg if a shared_ptr already exists for this object!";
       return std::shared_ptr<NE>{ne};
    }
 

src/tableModels/BtTableModel.h

@@ -89,7 +89,7 @@ class BtTableModelData {
     * \brief Return the \c i-th row in the model.
     *        Returns \c nullptr on failure.
     */
-   std::shared_ptr<NE> getRow(int ii) {
+   std::shared_ptr<NE> getRow(int ii) const {
       if (!(this->rows.isEmpty())) {
          if (ii >= 0 && ii < this->rows.size()) {
             return this->rows[ii];
@@ -118,12 +118,37 @@ class BtTableModelData {
       return tmp;
    }
 
+   /**
+    * \brief Given a raw pointer, find the index of the corresponding shared pointer in \c this->rows
+    *
+    *        This is useful because the Qt signals and slots framework allows the slot receiving a signal to get a raw
+    *        pointer to the object that sent the signal, and we often want to find the corresponding shared pointer in
+    *        our list.
+    *
+    *        Note that using this function is a lot safer than, say, calling ObjectStoreWrapper::getSharedFromRaw(), as
+    *        that only works for objects that are already stored in the database, something which is not guaranteed to
+    *        be the case with our rows.  (Eg in SaltTableModel, new Salts are only stored in the DB when the window is
+    *        closed with OK.)
+    *
+    *        Function name is for consistency with \c QList::indexOf
+    *
+    * \param object  what to search for
+    * \return index of object in this->rows or -1 if it's not found
+    */
+   int findIndexOf(NE const * object) const {
+      for (int index = 0; index < this->rows.size(); ++index) {
+         if (this->rows.at(index).get() == object) {
+            return index;
+         }
+      }
+      return -1;
+   }
+
 protected:
    virtual std::shared_ptr<NamedEntity> getRowAsNamedEntity(int ii) {
       return std::static_pointer_cast<NamedEntity>(this->getRow(ii));
    }
 
-
    QList< std::shared_ptr<NE> > rows;
 };
 

src/tableModels/FermentableTableModel.cpp

@@ -236,14 +236,13 @@ void FermentableTableModel::changedInventory(int invKey, BtStringConst const & p
    return;
 }
 
-void FermentableTableModel::changed(QMetaProperty prop, QVariant /*val*/) {
+void FermentableTableModel::changed(QMetaProperty prop, [[maybe_unused]] QVariant val) {
    qDebug() << Q_FUNC_INFO << prop.name();
 
    // Is sender one of our fermentables?
    Fermentable* fermSender = qobject_cast<Fermentable*>(sender());
    if (fermSender) {
-      auto spFermSender = ObjectStoreWrapper::getSharedFromRaw(fermSender);
-      int ii = this->rows.indexOf(spFermSender);
+      int ii = this->findIndexOf(fermSender);
       if (ii < 0) {
          return;
       }

src/tableModels/HopTableModel.cpp

@@ -223,13 +223,12 @@ void HopTableModel::changedInventory(int invKey, BtStringConst const & propertyN
    return;
 }
 
-void HopTableModel::changed(QMetaProperty prop, QVariant /*val*/) {
+void HopTableModel::changed(QMetaProperty prop, [[maybe_unused]] QVariant val) {
 
    // Find the notifier in the list
    Hop * hopSender = qobject_cast<Hop *>(sender());
    if (hopSender) {
-      auto spHopSender = ObjectStoreWrapper::getSharedFromRaw(hopSender);
-      int ii = this->rows.indexOf(spHopSender);
+      int ii = this->findIndexOf(hopSender);
       if (ii < 0) {
          return;
       }

src/tableModels/MashStepTableModel.cpp

@@ -204,9 +204,8 @@ void MashStepTableModel::mashStepChanged(QMetaProperty prop,
                                          [[maybe_unused]] QVariant val) {
    qDebug() << Q_FUNC_INFO;
 
-   MashStep* stepSenderRaw = qobject_cast<MashStep*>(sender());
-   if (stepSenderRaw) {
-      auto stepSender = ObjectStoreWrapper::getSharedFromRaw(stepSenderRaw);
+   MashStep* stepSender = qobject_cast<MashStep*>(sender());
+   if (stepSender) {
       if (stepSender->getMashId() != this->mashObs->key()) {
          // It really shouldn't happen that we get a notification for a MashStep that's not in the Mash we're watching,
          // but, if we do, then stop trying to process the update.
@@ -217,10 +216,10 @@ void MashStepTableModel::mashStepChanged(QMetaProperty prop,
          return;
       }
 
-      int ii = this->rows.indexOf(stepSender);
+      int ii = this->findIndexOf(stepSender);
       if (ii >= 0) {
          if (prop.name() == PropertyNames::MashStep::stepNumber) {
-            this->reorderMashStep(stepSender, ii);
+            this->reorderMashStep(this->rows.at(ii), ii);
          }
 
          emit dataChanged( QAbstractItemModel::createIndex(ii, 0),

src/tableModels/MiscTableModel.cpp

@@ -405,17 +405,14 @@ void MiscTableModel::changedInventory(int invKey, BtStringConst const & property
    return;
 }
 
-void MiscTableModel::changed(QMetaProperty prop, QVariant /*val*/) {
+void MiscTableModel::changed(QMetaProperty prop, [[maybe_unused]] QVariant val) {
    Misc * miscSender = qobject_cast<Misc*>(sender());
    if (miscSender) {
-      auto spMiscSender = ObjectStoreWrapper::getSharedFromRaw(miscSender);
-      int i = this->rows.indexOf(spMiscSender);
-      if (i < 0) {
-         return;
+      int ii = this->findIndexOf(miscSender);
+      if (ii >= 0) {
+         emit dataChanged( QAbstractItemModel::createIndex(ii, 0),
+                           QAbstractItemModel::createIndex(ii, MISCNUMCOLS-1) );
       }
-
-      emit dataChanged( QAbstractItemModel::createIndex(i, 0),
-                        QAbstractItemModel::createIndex(i, MISCNUMCOLS-1) );
       return;
    }