From ac61e1c77b4461b1033ac8039ec41533aecdbbae Mon Sep 17 00:00:00 2001 From: Richard Kimberly Heck Date: Sat, 1 Sep 2018 21:48:48 -0400 Subject: [PATCH] Revert "Attempt to fix bug 9158 using updateBuffer." This reverts commit fe246160603786a9d299755d88eabd3d7ce5439e. --- src/Buffer.cpp | 139 ++++++++++++++++++------------------ src/Buffer.h | 23 ++++-- src/BufferView.cpp | 8 ++- src/insets/InsetBibtex.cpp | 37 ++++++---- src/insets/InsetBibtex.h | 4 +- src/insets/InsetInclude.cpp | 7 ++ src/insets/InsetInclude.h | 8 +++ 7 files changed, 136 insertions(+), 90 deletions(-) diff --git a/src/Buffer.cpp b/src/Buffer.cpp index 4d527422e3..71b26f9590 100644 --- a/src/Buffer.cpp +++ b/src/Buffer.cpp @@ -289,6 +289,8 @@ 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 bibfile_status_; /// Indicates whether the bibinfo has changed since the last time @@ -341,8 +343,10 @@ public: if (!cloned_buffer_ && parent_buffer && pb) LYXERR0("Warning: a buffer should not have two parents!"); parent_buffer = pb; - if (!cloned_buffer_ && parent_buffer) + if (!cloned_buffer_ && parent_buffer) { + parent_buffer->invalidateBibfileCache(); parent_buffer->invalidateBibinfoCache(); + } } /// If non zero, this buffer is a clone of existing buffer \p cloned_buffer_ @@ -428,7 +432,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), - cite_labels_valid_(false), preview_error_(false), + bibfile_cache_valid_(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), @@ -448,6 +452,7 @@ 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; @@ -1899,7 +1904,7 @@ void Buffer::writeLaTeXSource(otexstream & os, // Biblatex bibliographies are loaded here if (params().useBiblatex()) { vector const bibfiles = - prepareBibFilePaths(runparams, getBibfiles(), true); + prepareBibFilePaths(runparams, getBibfilesCache(), true); for (docstring const & file: bibfiles) os << "\\addbibresource{" << file << "}\n"; } @@ -2317,11 +2322,47 @@ void Buffer::getLabelList(vector & 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(*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(*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) @@ -2329,13 +2370,29 @@ void Buffer::invalidateBibinfoCache() const } -FileNamePairList const & Buffer::getBibfiles(UpdateScope scope) 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 { // FIXME This is probably unnecessary, given where we call this. - // If this is a child document, use the master instead. + // If this is a child document, use the master's cache instead. Buffer const * const pbuf = masterBuffer(); if (pbuf != this && scope != UpdateChildOnly) - return pbuf->getBibfiles(); + return pbuf->getBibfilesCache(); + + if (!d->bibfile_cache_valid_) + this->updateBibfilesCache(scope); + return d->bibfiles_cache_; } @@ -2355,20 +2412,6 @@ 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 @@ -2378,13 +2421,8 @@ 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 = getBibfiles(); + FileNamePairList const & bibfiles_cache = getBibfilesCache(); FileNamePairList::const_iterator ei = bibfiles_cache.begin(); FileNamePairList::const_iterator en = bibfiles_cache.end(); for (; ei != en; ++ ei) { @@ -4723,16 +4761,10 @@ 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 @@ -4778,45 +4810,14 @@ 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; - // if the bibfiles changed, the cache of bibinfo is invalid - FileNamePairList new_bibfiles = d->bibfiles_cache_; - // this is a trick to determine whether the two vectors have - // the same elements. - sort(new_bibfiles.begin(), new_bibfiles.end()); - sort(old_bibfiles.begin(), old_bibfiles.end()); - if (old_bibfiles != new_bibfiles) { - LYXERR(Debug::FILES, "Reloading bibinfo 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(); - // we will be re-doing the counters and references and such. - textclass.counters().reset(); - clearReferenceCache(); - // we should not need to do this again? - // updateMacros(); - setChangesPresent(false); - updateBuffer(parit, utype); - // this will already have been done by reloadBibInfoCache(); - // d->bibinfo_cache_valid_ = true; - } - else { - LYXERR(Debug::FILES, "Bibfiles unchanged."); - // this is also set to true on the other path, by reloadBibInfoCache. - d->bibinfo_cache_valid_ = true; - } + d->bibinfo_cache_valid_ = true; d->cite_labels_valid_ = true; /// FIXME: Perf cbuf.tocBackend().update(true, utype); diff --git a/src/Buffer.h b/src/Buffer.h index 048c875404..a68ad046cf 100644 --- a/src/Buffer.h +++ b/src/Buffer.h @@ -493,13 +493,25 @@ public: */ void validate(LaTeXFeatures &) const; - /// Bibliography information is cached in the Buffer, so we do not + /// Reference information is cached in the Buffer, so we do not /// have to check or read things over and over. - /// The cache exists only in the master buffer. When it is updated, + /// + /// 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 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. @@ -767,8 +779,6 @@ public: void setChangesPresent(bool) const; bool areChangesPresent() const; void updateChangesPresent() const; - /// - void registerBibfiles(support::FileNamePairList const & bf) const; private: friend class MarkAsExporting; @@ -785,10 +795,13 @@ 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 & - getBibfiles(UpdateScope scope = UpdateMaster) const; + getBibfilesCache(UpdateScope scope = UpdateMaster) const; /// void collectChildren(ListOfBuffers & children, bool grand_children) const; diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 588dea74b4..17dedce1b3 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -1663,8 +1663,10 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr) InsetBibtex * inset = getInsetByCode(tmpcur, BIBTEX_CODE); if (inset) { - if (inset->addDatabase(cmd.argument())) + if (inset->addDatabase(cmd.argument())) { + buffer_.invalidateBibfileCache(); dr.forceBufferUpdate(); + } } break; } @@ -1675,8 +1677,10 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr) InsetBibtex * inset = getInsetByCode(tmpcur, BIBTEX_CODE); if (inset) { - if (inset->delDatabase(cmd.argument())) + if (inset->delDatabase(cmd.argument())) { + buffer_.invalidateBibfileCache(); dr.forceBufferUpdate(); + } } break; } diff --git a/src/insets/InsetBibtex.cpp b/src/insets/InsetBibtex.cpp index 7ad93a0587..2d164969e7 100644 --- a/src/insets/InsetBibtex.cpp +++ b/src/insets/InsetBibtex.cpp @@ -59,7 +59,22 @@ 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 */) @@ -101,6 +116,8 @@ void InsetBibtex::doDispatch(Cursor & cur, FuncRequest & cmd) cur.recordUndo(); setParams(p); + buffer().invalidateBibfileCache(); + buffer().removeBiblioTempFiles(); cur.forceBufferUpdate(); break; } @@ -360,15 +377,15 @@ void InsetBibtex::latex(otexstream & os, OutputParams const & runparams) const } -FileNamePairList InsetBibtex::getBibFiles() const +support::FileNamePairList InsetBibtex::getBibFiles() const { FileName path(buffer().filePath()); - PathChanger p(path); + support::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. - FileNamePairList vec; + support::FileNamePairList vec; vector bibfilelist = getVectorFromString(getParam("bibfiles")); vector::const_iterator it = bibfilelist.begin(); @@ -383,6 +400,7 @@ FileNamePairList InsetBibtex::getBibFiles() const } return vec; + } namespace { @@ -655,9 +673,9 @@ void InsetBibtex::parseBibTeXFiles(FileNameList & checkedFiles) const BiblioInfo keylist; - FileNamePairList const files = getBibFiles(); - FileNamePairList::const_iterator it = files.begin(); - FileNamePairList::const_iterator en = files.end(); + support::FileNamePairList const files = getBibFiles(); + support::FileNamePairList::const_iterator it = files.begin(); + support::FileNamePairList::const_iterator en = files.end(); for (; it != en; ++ it) { FileName const bibfile = it->second; if (find(checkedFiles.begin(), checkedFiles.end(), bibfile) != checkedFiles.end()) @@ -893,11 +911,6 @@ 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 { diff --git a/src/insets/InsetBibtex.h b/src/insets/InsetBibtex.h index 882f0b8460..2d033464f0 100644 --- a/src/insets/InsetBibtex.h +++ b/src/insets/InsetBibtex.h @@ -29,6 +29,8 @@ class InsetBibtex : public InsetCommand { public: /// InsetBibtex(Buffer *, InsetCommandParams const &); + /// + ~InsetBibtex(); /// support::FileNamePairList getBibFiles() const; @@ -55,8 +57,6 @@ 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; diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp index 89f2533111..7b6f38950f 100644 --- a/src/insets/InsetInclude.cpp +++ b/src/insets/InsetInclude.cpp @@ -197,6 +197,11 @@ 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_; } @@ -349,6 +354,8 @@ void InsetInclude::setParams(InsetCommandParams const & p) if (type(params()) == INPUT) add_preview(*preview_, *this, buffer()); + + buffer().invalidateBibfileCache(); } diff --git a/src/insets/InsetInclude.h b/src/insets/InsetInclude.h index 18329fe6d9..009b49e3ac 100644 --- a/src/insets/InsetInclude.h +++ b/src/insets/InsetInclude.h @@ -59,6 +59,14 @@ 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(); ///