]> git.lyx.org Git - features.git/commitdiff
Attempt to fix bug 9158 using updateBuffer.
authorRichard Heck <rgheck@lyx.org>
Sun, 5 Nov 2017 01:23:25 +0000 (21:23 -0400)
committerRichard Heck <rgheck@lyx.org>
Sun, 15 Apr 2018 04:14:32 +0000 (00:14 -0400)
Along the lines suggested by JMarc, we now collect the list of bibfiles
in use in the updateBuffer routines. This actually does simplify the code
quite a bit. See the discussion there for reasons to go this way.

(cherry picked from commit 8b9d1b860187338e06e10261b391886d50423239)

src/Buffer.cpp
src/Buffer.h
src/BufferView.cpp
src/insets/InsetBibtex.cpp
src/insets/InsetBibtex.h
src/insets/InsetInclude.cpp
src/insets/InsetInclude.h

index 6e523d9098520569300ebff6e097ef0873f52c36..1597e898729e6241a3f1aa2dd1b914228da55ea9 100644 (file)
@@ -289,8 +289,6 @@ public:
        mutable BiblioInfo bibinfo_;
        /// whether the bibinfo cache is valid
        mutable bool bibinfo_cache_valid_;
-       /// whether the bibfile cache is valid
-       mutable bool bibfile_cache_valid_;
        /// Cache of timestamps of .bib files
        map<FileName, time_t> bibfile_status_;
        /// Indicates whether the bibinfo has changed since the last time
@@ -343,10 +341,8 @@ public:
                if (!cloned_buffer_ && parent_buffer && pb)
                        LYXERR0("Warning: a buffer should not have two parents!");
                parent_buffer = pb;
-               if (!cloned_buffer_ && parent_buffer) {
-                       parent_buffer->invalidateBibfileCache();
+               if (!cloned_buffer_ && parent_buffer)
                        parent_buffer->invalidateBibinfoCache();
-               }
        }
 
        /// If non zero, this buffer is a clone of existing buffer \p cloned_buffer_
@@ -432,7 +428,7 @@ Buffer::Impl::Impl(Buffer * owner, FileName const & file, bool readonly_,
          file_fully_loaded(false), file_format(LYX_FORMAT), need_format_backup(false),
          ignore_parent(false),  toc_backend(owner), macro_lock(false),
          checksum_(0), wa_(0),  gui_(0), undo_(*owner), bibinfo_cache_valid_(false),
-         bibfile_cache_valid_(false), cite_labels_valid_(false), preview_error_(false),
+         cite_labels_valid_(false), preview_error_(false),
          inset(0), preview_loader_(0), cloned_buffer_(cloned_buffer),
          clone_list_(0), doing_export(false),
          tracked_changes_present_(0), externally_modified_(false), parent_buffer(0),
@@ -452,7 +448,6 @@ Buffer::Impl::Impl(Buffer * owner, FileName const & file, bool readonly_,
        bibfiles_cache_ = cloned_buffer_->d->bibfiles_cache_;
        bibinfo_ = cloned_buffer_->d->bibinfo_;
        bibinfo_cache_valid_ = cloned_buffer_->d->bibinfo_cache_valid_;
-       bibfile_cache_valid_ = cloned_buffer_->d->bibfile_cache_valid_;
        bibfile_status_ = cloned_buffer_->d->bibfile_status_;
        cite_labels_valid_ = cloned_buffer_->d->cite_labels_valid_;
        unnamed = cloned_buffer_->d->unnamed;
@@ -1904,7 +1899,7 @@ void Buffer::writeLaTeXSource(otexstream & os,
                // Biblatex bibliographies are loaded here
                if (params().useBiblatex()) {
                        vector<docstring> const bibfiles =
-                               prepareBibFilePaths(runparams, getBibfilesCache(), true);
+                               prepareBibFilePaths(runparams, getBibfiles(), true);
                        for (docstring const & file: bibfiles)
                                os << "\\addbibresource{" << file << "}\n";
                }
@@ -2322,47 +2317,11 @@ void Buffer::getLabelList(vector<docstring> & list) const
 }
 
 
-void Buffer::updateBibfilesCache(UpdateScope scope) const
-{
-       // FIXME This is probably unnecssary, given where we call this.
-       // If this is a child document, use the parent's cache instead.
-       if (parent() && scope != UpdateChildOnly) {
-               masterBuffer()->updateBibfilesCache();
-               return;
-       }
-
-       d->bibfiles_cache_.clear();
-       for (InsetIterator it = inset_iterator_begin(inset()); it; ++it) {
-               if (it->lyxCode() == BIBTEX_CODE) {
-                       InsetBibtex const & inset = static_cast<InsetBibtex const &>(*it);
-                       support::FileNamePairList const bibfiles = inset.getBibFiles();
-                       d->bibfiles_cache_.insert(d->bibfiles_cache_.end(),
-                               bibfiles.begin(),
-                               bibfiles.end());
-               } else if (it->lyxCode() == INCLUDE_CODE) {
-                       InsetInclude & inset = static_cast<InsetInclude &>(*it);
-                       Buffer const * const incbuf = inset.getChildBuffer();
-                       if (!incbuf)
-                               continue;
-                       support::FileNamePairList const & bibfiles =
-                                       incbuf->getBibfilesCache(UpdateChildOnly);
-                       if (!bibfiles.empty()) {
-                               d->bibfiles_cache_.insert(d->bibfiles_cache_.end(),
-                                       bibfiles.begin(),
-                                       bibfiles.end());
-                       }
-               }
-       }
-       d->bibfile_cache_valid_ = true;
-       d->bibinfo_cache_valid_ = false;
-       d->cite_labels_valid_ = false;
-}
-
-
 void Buffer::invalidateBibinfoCache() const
 {
        d->bibinfo_cache_valid_ = false;
        d->cite_labels_valid_ = false;
+       removeBiblioTempFiles();
        // also invalidate the cache for the parent buffer
        Buffer const * const pbuf = d->parent();
        if (pbuf)
@@ -2370,29 +2329,13 @@ void Buffer::invalidateBibinfoCache() const
 }
 
 
-void Buffer::invalidateBibfileCache() const
-{
-       d->bibfile_cache_valid_ = false;
-       d->bibinfo_cache_valid_ = false;
-       d->cite_labels_valid_ = false;
-       // also invalidate the cache for the parent buffer
-       Buffer const * const pbuf = d->parent();
-       if (pbuf)
-               pbuf->invalidateBibfileCache();
-}
-
-
-support::FileNamePairList const & Buffer::getBibfilesCache(UpdateScope scope) const
+FileNamePairList const & Buffer::getBibfiles(UpdateScope scope) const
 {
        // FIXME This is probably unnecessary, given where we call this.
-       // If this is a child document, use the master's cache instead.
+       // If this is a child document, use the master instead.
        Buffer const * const pbuf = masterBuffer();
        if (pbuf != this && scope != UpdateChildOnly)
-               return pbuf->getBibfilesCache();
-
-       if (!d->bibfile_cache_valid_)
-               this->updateBibfilesCache(scope);
-
+               return pbuf->getBibfiles();
        return d->bibfiles_cache_;
 }
 
@@ -2412,6 +2355,20 @@ BiblioInfo const & Buffer::bibInfo() const
 }
 
 
+void Buffer::registerBibfiles(FileNamePairList const & bf) const {
+       Buffer const * const tmp = masterBuffer();
+       if (tmp != this)
+               return tmp->registerBibfiles(bf);
+
+       for (auto const & p : bf) {
+               FileNamePairList::const_iterator tmp =
+                       find(d->bibfiles_cache_.begin(), d->bibfiles_cache_.end(), p);
+               if (tmp == d->bibfiles_cache_.end())
+                       d->bibfiles_cache_.push_back(p);
+       }
+}
+
+
 void Buffer::checkIfBibInfoCacheIsValid() const
 {
        // use the master's cache
@@ -2421,8 +2378,13 @@ void Buffer::checkIfBibInfoCacheIsValid() const
                return;
        }
 
+       // if we already know the cache is invalid, no need to check
+       // the timestamps
+       if (!d->bibinfo_cache_valid_)
+               return;
+
        // compare the cached timestamps with the actual ones.
-       FileNamePairList const & bibfiles_cache = getBibfilesCache();
+       FileNamePairList const & bibfiles_cache = getBibfiles();
        FileNamePairList::const_iterator ei = bibfiles_cache.begin();
        FileNamePairList::const_iterator en = bibfiles_cache.end();
        for (; ei != en; ++ ei) {
@@ -4753,10 +4715,16 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
        Buffer const * const master = masterBuffer();
        DocumentClass const & textclass = master->params().documentClass();
 
+       FileNamePairList old_bibfiles;
        // do this only if we are the top-level Buffer
        if (master == this) {
                textclass.counters().reset(from_ascii("bibitem"));
                reloadBibInfoCache();
+               // we will re-read this cache as we go through, but we need
+               // to know whether it's changed to know whether we need to
+               // update the bibinfo cache.
+               old_bibfiles = d->bibfiles_cache_;
+               d->bibfiles_cache_.clear();
        }
 
        // keep the buffers to be children in this set. If the call from the
@@ -4802,14 +4770,30 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
        ParIterator parit = cbuf.par_iterator_begin();
        updateBuffer(parit, utype);
 
+       // If this document has siblings, then update the TocBackend later. The
+       // reason is to ensure that later siblings are up to date when e.g. the
+       // broken or not status of references is computed. The update is called
+       // in InsetInclude::addToToc.
        if (master != this)
-               // If this document has siblings, then update the TocBackend later. The
-               // reason is to ensure that later siblings are up to date when e.g. the
-               // broken or not status of references is computed. The update is called
-               // in InsetInclude::addToToc.
                return;
 
-       d->bibinfo_cache_valid_ = true;
+       // if the bibfiles changed, the cache of bibinfo is invalid
+       sort(d->bibfiles_cache_.begin(), d->bibfiles_cache_.end());
+       // the old one should already be sorted
+       if (old_bibfiles != d->bibfiles_cache_) {
+               invalidateBibinfoCache();
+               reloadBibInfoCache();
+               // We relied upon the bibinfo cache when recalculating labels. But that
+               // cache was invalid, although we didn't find that out until now. So we
+               // have to do it all again.
+               // That said, the only thing we really need to do is update the citation
+               // labels. Nothing else will have changed. So we could create a new 
+               // UpdateType that would signal that fact, if we needed to do so.
+               parit = cbuf.par_iterator_begin();
+               updateBuffer(parit, utype);
+       }
+       else
+               d->bibinfo_cache_valid_ = true;
        d->cite_labels_valid_ = true;
        /// FIXME: Perf
        cbuf.tocBackend().update(true, utype);
index ae1d67064ae3fbb4c79f3f86f2028230696afedb..0e01dbc286716650e3f7273f6a425293a4015a96 100644 (file)
@@ -493,25 +493,13 @@ public:
        */
        void validate(LaTeXFeatures &) const;
 
-       /// Reference information is cached in the Buffer, so we do not
+       /// Bibliography information is cached in the Buffer, so we do not
        /// have to check or read things over and over.
-       ///
-       /// There are two caches.
-       ///
-       /// One is a cache of the BibTeX files from which reference info is
-       /// being gathered. This cache is PER BUFFER, and the cache for the
-       /// master essentially includes the cache for its children. This gets
-       /// invalidated when an InsetBibtex is created, deleted, or modified.
-       ///
-       /// The other is a cache of the reference information itself. This
-       /// exists only in the master buffer, and when it needs to be updated,
+       /// The cache exists only in the master buffer. When it is updated,
        /// the children add their information to the master's cache.
-
        /// Calling this method invalidates the cache and so requires a
        /// re-read.
        void invalidateBibinfoCache() const;
-       /// This invalidates the cache of files we need to check.
-       void invalidateBibfileCache() const;
        /// Updates the cached bibliography information, checking first to see
        /// whether the cache is valid. If so, we do nothing. If not, then we
        /// reload all the BibTeX info.
@@ -777,6 +765,8 @@ public:
        void setChangesPresent(bool) const;
        bool areChangesPresent() const;
        void updateChangesPresent() const;
+       ///
+       void registerBibfiles(support::FileNamePairList const & bf) const;
 
 private:
        friend class MarkAsExporting;
@@ -793,13 +783,10 @@ private:
        /// last time we loaded the cache. Note that this does NOT update the
        /// cached information.
        void checkIfBibInfoCacheIsValid() const;
-       /// Update the list of all bibfiles in use (including bibfiles
-       /// of loaded child documents).
-       void updateBibfilesCache(UpdateScope scope = UpdateMaster) const;
        /// Return the list with all bibfiles in use (including bibfiles
        /// of loaded child documents).
        support::FileNamePairList const &
-               getBibfilesCache(UpdateScope scope = UpdateMaster) const;
+               getBibfiles(UpdateScope scope = UpdateMaster) const;
        ///
        void collectChildren(ListOfBuffers & children, bool grand_children) const;
 
index 7788f76d3db747c40b250e702704d6179f25ccfb..f1b5c378b631230779e0f1aa2a5aba5e84ecad1f 100644 (file)
@@ -1637,10 +1637,8 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
                InsetBibtex * inset = getInsetByCode<InsetBibtex>(tmpcur,
                                                BIBTEX_CODE);
                if (inset) {
-                       if (inset->addDatabase(cmd.argument())) {
-                               buffer_.invalidateBibfileCache();
+                       if (inset->addDatabase(cmd.argument()))
                                dr.forceBufferUpdate();
-                       }
                }
                break;
        }
@@ -1651,10 +1649,8 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
                InsetBibtex * inset = getInsetByCode<InsetBibtex>(tmpcur,
                                                BIBTEX_CODE);
                if (inset) {
-                       if (inset->delDatabase(cmd.argument())) {
-                               buffer_.invalidateBibfileCache();
+                       if (inset->delDatabase(cmd.argument()))
                                dr.forceBufferUpdate();
-                       }
                }
                break;
        }
index 30b169acb07ff5d0d0a80c402b9e54b508668ede..4ecb5227378b13391166ddf8b616a0c71e8ee73a 100644 (file)
@@ -59,22 +59,7 @@ namespace os = support::os;
 
 InsetBibtex::InsetBibtex(Buffer * buf, InsetCommandParams const & p)
        : InsetCommand(buf, p)
-{
-       buffer().invalidateBibfileCache();
-       buffer().removeBiblioTempFiles();
-}
-
-
-InsetBibtex::~InsetBibtex()
-{
-       if (isBufferLoaded()) {
-               /* We do not use buffer() because Coverity believes that this
-                * may throw an exception. Actually this code path is not
-                * taken when buffer_ == 0 */
-               buffer_-> invalidateBibfileCache();
-               buffer_->removeBiblioTempFiles();
-       }
-}
+{}
 
 
 ParamInfo const & InsetBibtex::findInfo(string const & /* cmdName */)
@@ -116,8 +101,6 @@ void InsetBibtex::doDispatch(Cursor & cur, FuncRequest & cmd)
 
                cur.recordUndo();
                setParams(p);
-               buffer().invalidateBibfileCache();
-               buffer().removeBiblioTempFiles();
                cur.forceBufferUpdate();
                break;
        }
@@ -378,15 +361,15 @@ void InsetBibtex::latex(otexstream & os, OutputParams const & runparams) const
 }
 
 
-support::FileNamePairList InsetBibtex::getBibFiles() const
+FileNamePairList InsetBibtex::getBibFiles() const
 {
        FileName path(buffer().filePath());
-       support::PathChanger p(path);
+       PathChanger p(path);
 
        // We need to store both the real FileName and the way it is entered
        // (with full path, rel path or as a single file name).
        // The latter is needed for biblatex's central bibfile macro.
-       support::FileNamePairList vec;
+       FileNamePairList vec;
 
        vector<docstring> bibfilelist = getVectorFromString(getParam("bibfiles"));
        vector<docstring>::const_iterator it = bibfilelist.begin();
@@ -401,7 +384,6 @@ support::FileNamePairList InsetBibtex::getBibFiles() const
        }
 
        return vec;
-
 }
 
 namespace {
@@ -674,9 +656,9 @@ void InsetBibtex::parseBibTeXFiles(FileNameList & checkedFiles) const
 
        BiblioInfo keylist;
 
-       support::FileNamePairList const files = getBibFiles();
-       support::FileNamePairList::const_iterator it = files.begin();
-       support::FileNamePairList::const_iterator en = files.end();
+       FileNamePairList const files = getBibFiles();
+       FileNamePairList::const_iterator it = files.begin();
+       FileNamePairList::const_iterator en = files.end();
        for (; it != en; ++ it) {
                FileName const bibfile = it->second;
                if (find(checkedFiles.begin(), checkedFiles.end(), bibfile) != checkedFiles.end())
@@ -912,6 +894,11 @@ void InsetBibtex::validate(LaTeXFeatures & features) const
 }
 
 
+void InsetBibtex::updateBuffer(ParIterator const &, UpdateType) {
+       buffer().registerBibfiles(getBibFiles());
+}
+
+
 int InsetBibtex::plaintext(odocstringstream & os,
        OutputParams const & op, size_t max_length) const
 {
index 2d033464f0a6409a0a1309d124622c26fba66d07..882f0b8460a5ee3adbf91aab2f6527538071b8ba 100644 (file)
@@ -29,8 +29,6 @@ class InsetBibtex : public InsetCommand {
 public:
        ///
        InsetBibtex(Buffer *, InsetCommandParams const &);
-       ///
-       ~InsetBibtex();
 
        ///
        support::FileNamePairList getBibFiles() const;
@@ -57,6 +55,8 @@ public:
        int plaintext(odocstringstream & ods, OutputParams const & op,
                      size_t max_length = INT_MAX) const;
        ///
+       void updateBuffer(ParIterator const &, UpdateType);
+       ///
        void collectBibKeys(InsetIterator const &, support::FileNameList &) const;
        ///
        void validate(LaTeXFeatures &) const;
index 7b6f38950f0cbc0229f4518ad04d2828a121ff89..89f2533111a99f04cc6f9551c5577750c6cf5de5 100644 (file)
@@ -197,11 +197,6 @@ InsetInclude::InsetInclude(InsetInclude const & other)
 
 InsetInclude::~InsetInclude()
 {
-       if (isBufferLoaded())
-               /* We do not use buffer() because Coverity believes that this
-                * may throw an exception. Actually this code path is not
-                * taken when buffer_ == 0 */
-               buffer_->invalidateBibfileCache();
        delete label_;
 }
 
@@ -354,8 +349,6 @@ void InsetInclude::setParams(InsetCommandParams const & p)
 
        if (type(params()) == INPUT)
                add_preview(*preview_, *this, buffer());
-
-       buffer().invalidateBibfileCache();
 }
 
 
index 009b49e3ac03c06b9ca4e55b72a677f3a41e157a..18329fe6d9117826e8f62c9eaf2e1b8990123d8f 100644 (file)
@@ -59,14 +59,6 @@ public:
         */
        void updateBibfilesCache();
 
-       /** Return the cache with all bibfiles in use of the child buffer
-        *  (including bibfiles of grandchild documents).
-        *  Return an empty vector if the child doc is not loaded.
-        *  \param buffer the Buffer containing this inset.
-        */
-       support::FileNameList const &
-               getBibfilesCache() const;
-
        ///
        void updateCommand();
        ///