When there are multiple competing configuration settings for the same
configuration layer (e.g., in xcu files of two different extensions from the
same extension layer), then the setting that is read last always won, even if
any of the settings read earlier is marked as finalized. (The reason for
originally doing it that way was that it kept the code logic somewhat simple.)
However, esp. for a scenario of multiple extensions in one extension layer
(bundled, shared, or user), it can be unexpected by a user that a non-finalized
setting (that comes from the extension that happens to be read last) can win
over a finalized one.
Therefore, change the logic accordingly. Now, if any of the competing settings
are finalized, the first finalized one that is read wins.
Change-Id: I22aeade543a5b26d95d49cfcb561f974cd7a5081
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177737
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
I don't think anyone is getting around to changing this after 13
years, and an int worth of config layers looks safe enough to me.
Change-Id: Iecf00a4d342e60d8f56a27978c40fa8d861ecd7e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177428
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
... otherwise the unotools/source/config/configitem.cxx
ConfigItem::impl_packLocalizedProperties() checking for
(getValueTypeName() == "com.sun.star.uno.XInterface") would not
hit and thus not pack the properties, returning just the useless
XWeak pointer as property.
This in ScUnoAddInCollection::ReadConfiguration() lead to English
names not being extracted from configured Add-Ins' DisplayName or
CompatibilityName, nor any CompatibilityName added to the list.
Fallout from
commit bc70f674c2
CommitDate: Sun Jun 18 08:41:27 2023 +0200
Use getXWeak in configmgr
Change-Id: Ibaab8c08a06113b27ceffe1c45ef548c1d6fa225
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172476
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
...instead of joining and later re-spawning one. This helps with the current
Emscripten setup (see 5b1df7709d "Document the
Emscripten threads issue"), and probably doesn't matter much either way on other
platforms (so just get it in unconditionally). (Renaming the delay_ member
variable to delayOrTerminate_, to make its overall role more clear.)
(I ran into this when trying to turn the Emscripten build from starting up with
a Writer document to starting up with the start center, and then manually
opening a new Writer document. The resulting different usage scheme happened to
exhaust our current -sPTHREAD_POOL_SIZE=4 pool quickly, so this is one of
multiple commits to address that.)
Change-Id: I1bd28604b4640d2afad982bfd82b1acee8200e26
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170993
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
Tested-by: Jenkins
That code had been added with 9fa4eff9be
"profilesafe: Enhancements to BackupFileHelper", but it looks dubious:
For one, I cannot find proof for the claim that MSVC would run destructors of
static objects upon _exit. Building a simple C++ test program
> #include <iostream>
> #include <stdlib.h>
> struct S { ~S() { std::cerr << "hello\n"; } };
> S s;
> void f() { _exit(1); }
> int main() { f(); }
with contemporary MSVC 17.10 and executing it shows no "hello" stderr output
(see
<https://gcc.godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,selection:(endColumn:13,endLineNumber:5,positionColumn:13,positionLineNumber:5,selectionStartColumn:13,selectionStartLineNumber:5,startColumn:13,startLineNumber:5),source:'%23include+%3Ciostream%3E%0A%23include+%3Cstdlib.h%3E%0Astruct+S+%7B+~S()+%7B+std::cerr+%3C%3C+%22hello%5Cn%22%3B+%7D+%7D%3B%0AS+s%3B%0Avoid+f()+%7B+_exit(1)%3B+%7D%0Aint+main()+%7B+f()%3B+%7D%0A'),l:'5',n:'1',o:'C%2B%2B+source+%231',t:'0')),k:32.19670284753087,l:'4',m:100,n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:vcpp_v19_40_VS17_10_x64,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'0',intel:'0',libraryCode:'0',trim:'1',verboseDemangling:'0'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,libs:!(),options:'',overrides:!(),selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+x64+msvc+v19.40+VS17.10+(Editor+%231)',t:'0'),(h:output,i:(compilerName:'x86-64+clang+18.1.0',editorid:1,fontScale:14,fontUsePx:'0',j:1,wrap:'1'),l:'5',n:'0',o:'Output+of+x64+msvc+v19.40+VS17.10+(Compiler+%231)',t:'0')),k:67.80329715246913,l:'4',m:100.00000000000001,n:'0',o:'',s:1,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4>).
If there indeed were issues with MSVC and non-standard _exit, a solution might
be to use standard _Exit instead (in FatalError in desktop/source/app/app.cxx).
And for another, when bExitWasCalled was true in
configmgr::Components::~Components, it would still have called
writeThread_->join() (i.e., it would potentially still have called
writeModFile from within configmgr::Components::WriteThread::execute()), but (a)
only after waiting out its full
> delay_.wait(std::chrono::seconds(1));
(because it missed to call configmgr::Components::WriteThread::flush, which
calls delay_.set()), and (b) potentially running into a deadlock because it
called writeThread_->join() with *lock_ locked, which
configmgr::Components::WriteThread::execute wants to lock too.
(I came across this because it got into my way when trying to adapt the
lifecycle of configmgr::Components::WriteThread to the needs of the Emscripten
build.)
Change-Id: Icb4ebf00d1d316b80c29f7bd91b86614ef4ac430
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170940
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
This reverts commit 154b204010, which is known
broken, as (unlike for boost::unordered_map) there is no guarantee that
std::unordered_map works for incomplete value types, and indeed there is build
environments that started to fail now with
> In file included from /home/libo/src/core/configmgr/source/partial.cxx:23:
> In file included from /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/set:60:
> In file included from /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_tree.h:63:
> In file included from /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_algobase.h:64:
> /usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_pair.h:218:11: error: field has incomplete type 'configmgr::Partial::Node'
> _T2 second; ///< The second member
> ^
[...]
Change-Id: Ie8d041bcf3d3b1b6aeede73adbaf14676250c4bf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170467
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
we are never actually going to need a set, so just iterate over
the Sequence
Change-Id: I0de6ff9e0661227a69b7fbe6cccc5268f9eba58f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167430
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
std::multiset seems to be pretty slow to construct and
destruct, even if we ~never put anything into these listener
lists, so rather use o3tl::sorted_vector, which has more
predictable performance.
Shaves 10% off startup time for me.
Change-Id: I13afcbe3cb63522e4efe61c64f773e1ffbec9683
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167101
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The unused but valid child elements of <info> such as <author> may be
used by exentions. This fails with:
warn:configmgr:15104:10916:configmgr/source/components.cxx:660: error reading "file:///instdir/program/../share/uno_packages/cache/uno_packages/....xcs" com.sun.star.uno.RuntimeException message: "bad member <component> in ....xcs at configmgr/source/xcsparser.cxx:289"
Because ending the first such element sets bIsParsingInfo_ to false.
This fix just concatenates all the characters in all the children,
should work well enough for extensions.
(regression from commit db3078bd8c)
Change-Id: I17a3fb7014cd34c1d546701036550085365432a4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/165143
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
...and clean up the most gross casting offenses
Change-Id: If0d646fb3e73e71a9a2735569395034973563a1f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164602
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
...which each pertain to all the preceding code that extracts numbers from
strings, and where 0a5d4dc25c "elide some OString
temporaries" had changed that preceding code from stretching over a single to
stretching over multiple statements each
Change-Id: I315187465d64f620b1ddea8a0cc74ed5f8dc113b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163063
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
...to reflect whether or not the corresponding item can be modified. Which
matches what the READONLY flag is actually used for today. (And the original,
somewhat arbitrary semantics had been assigned without much thought, IIRC, and
it should be OK to change them.)
Change-Id: I8185e47519a5cb4aff6e8f260326845276b0c092
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160957
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
In preparation for proper editing support for the different types.
Change-Id: I7044ff100c9bfcca5fa8894ff4525a1aac692796
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158028
Tested-by: Jenkins
Reviewed-by: Samuel Mehrbrodt <samuel.mehrbrodt@allotropia.de>
we can return by const& from getNode
Change-Id: If93c43fd2e910e2fb69d9bd0a9e3dc587133dfa8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158729
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
no need to construct a vector when iterating over children
Change-Id: I717e92be3c576a6e5d877f4333264a5bed9daadf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158728
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
The problem appeared when in a configuration listener's changesOccurred,
a configuration value was read using officecfg machinery, which could
return the old value of the configuration, or an updated one, at random.
This was caused by use of a cached value in
comphelper::detail::ConfigurationWrapper::getPropertyValue, which is
cleaned in ConfigurationChangesListener::changesOccurred; but the order
in which the listeners' changesOccurred methods were called depended on
the implementation detail of configmgr::Components::roots_, which was
previously a std::set of pointers, and now is a sorted vector. This
made the pointers sorted in order of the pointers' addresses (basically
random), and when a broadcaster's common list of change listeners was
prepared in Components::initGlobalBroadcaster, ConfigurationWrapper's
listener could arrive last. This meant, that the cache could be cleaned
too late, after its obsolete content was already used in a previous
listener.
The problem could be partially mitigated by clearing the cache in the
comphelper::detail::ConfigurationWrapper::setPropertyValue, but that
would only handle cases of config modifications using comphelper.
Instead, take into account that ConfigurationWrapper listens on the
root of configuration tree; and introduce a separate container in
configmgr::Broadcaster for root listeners. They would be triggered
first, before all other listeners.
Still, this would not guarantee the proper order, if another listener
is registered on root. To handle all cases, a special listener category
could be used, which could be filled using a dedicated internal API, so
comphelper could use it to register its privileged listener close to
the heart of the broadcaster :) This might be implemented later.
Change-Id: I956b7989b3927dca2683f63cf92b0dda04db9bdf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154561
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
found with a lightly tweaked version of the loplugin:stringadd
and some hand-holding.
Change-Id: I146aadcaf665e98fea89a9cad2df4dc3935622f4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152275
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
...after abd630e81b "Allow all hierarchical path
segments to be ['...'] quoted", which failed as the checked
MimeTypeClassIDRelations group is conditional on install:module="reportbuilder".
(See the comments at
<https://gerrit.libreoffice.org/c/core/+/151673/2#message-f029dd1877f8a946647456ed73a7b60f6dfc0e8f>
"Allow all hierarchical path segments to be ['...'] quoted".)
It looks like there is no other, non-conditional property with a slash in its
name, so the test now just tests the ['...'] syntax on an otherwise
unproblematic property name. (Alternatively, we could have extended the
configuration data used by unit tests with such a problematicaly named
property.)
Change-Id: I820004a5b824326d86e67c158994f13e71ba135e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152184
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
...and not just set elements. <https://gerrit.libreoffice.org/c/core/+/151660>
"tdf#104005 Don't allow changing finalized properties" found that some existing
property names include "/", which makes them unusable with e.g.
getPropertyByHierarchicalName.
The most obvious solution is to allow ['...'] quoting (without a leading
template name, though) for all kinds of hierarchical path segments. (In theory,
that would cause backwards incompatibility, as e.g. a property named ['foo']
could no longer be referenced by a ['foo'] path segment (it would need to be
referenced by a ['['foo']'] path segment instead). But in practice,
that path segment ['foo'] would have been rejected in the past anyway, as it did
not reference a set element.)
To make this work, the meaning of the setElement out-parameter of
configmgr::Data::parseSegment is changed to only be true if the segment uses
['...'] notation including a leading (non-empty) template name.
What this change does not (yet) address is writing out such quoted (group or
set) names in the hierarchical paths written out in configmgr::writeModFile,
where necessary. (It never writes out names of properties as parts of
hierarchical names, so this wouldn't be an issue for the existing problematic
properties containing "/" in their names, anyway.)
Change-Id: I635d823c7bbb6b8ac5869c7e0130ba8cfd329599
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151673
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Instead of doing two lookups, we can just call insert and
then update the mapped-to value if the insert actually succeeded.
Change-Id: I3df3b98f0debe6bf74c739dd5850e7714d9c2789
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151030
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
after my patch to merge the bufferadd loplugin into stringadd
Change-Id: I66f4ce2fd87c1e12eefb14406e0e17212f68ceff
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149497
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
At least building on Fedora 37 with dconf-devel-0.40.0-7.fc37.x86_64 against
recent LLVM 17 trunk libc++ started to fail now with
> In file included from configmgr/source/dconf.cxx:25:
> In file included from /usr/include/dconf/dconf.h:23:
> In file included from /usr/include/dconf/common/dconf-enums.h:23:
> In file included from /usr/include/glib-2.0/glib.h:34:
> In file included from /usr/include/glib-2.0/glib/gasyncqueue.h:34:
> In file included from /usr/include/glib-2.0/glib/gthread.h:34:
> In file included from /usr/include/glib-2.0/glib/gatomic.h:30:
> In file included from /usr/include/glib-2.0/glib/glib-typeof.h:44:
> In file included from ~/llvm/inst/bin/../include/c++/v1/type_traits:430:
> ~/llvm/inst/bin/../include/c++/v1/__type_traits/aligned_union.h:23:1: error: templates must have C++ linkage
> template <size_t _I0, size_t ..._In>
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> configmgr/source/dconf.cxx:19:1: note: extern "C" language linkage specification begins here
> extern "C" {
> ^
because the missing extern "C" has meanwhile been added to upstream dconf with
<f860ddcc6a>
"common: Add missing G_BEGIN/END_DECLS" towards tag 0.40.0.
So the easiest fix here appears to be to bump our requirements to that dconf
version 0.40.0: The one case I'm aware of that explicitly wants dconf support
is downstream Fedora and RHEL builds, which already have dconf 0.40.0
(cf. most recent
<a9987b0abe/f/dconf.spec (_4)>
for f37 and most recent
<1bb6b46da2/dconf.spec (L4)>
for e9s). On the other hand, the TDF Linux release builds are done with an
explicit --disable-dconf in distro-configs/LibreOfficeLinux.conf (since
ecc617e797 "configmgr: support reading from a
dconf layer (WIP)") and the Flatpak builds on Flathub are done against an
org.freedesktop.Sdk//22.08 where no dconf support is detected at all (for the
most recent LO 7.5.1 build on Flathub see
> checking for DCONF... no
> checking whether to enable dconf... no
at <https://buildbot.flathub.org/#/builders/5/builds/3184/steps/8/logs/stdio>
for aarch64 and at
<https://buildbot.flathub.org/#/builders/5/builds/3184/steps/8/logs/stdio> for
x86_64).
Change-Id: I559d2ac8712dbe2b40c9b881314b88d1cc8eaf43
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/148887
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Since the code that can run during flushModifications might manage to
call back into Service..
regression from
commit a61951a3a2
Author: Noel Grandin <noel.grandin@collabora.co.uk>
Date: Mon Feb 20 20:08:30 2023 +0200
BaseMutex->std::mutex in configmgr::configuration_provider::Service
Change-Id: I0070e7589cdea38905de6c68adefd8081b122152
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147500
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
...after b52117c0be "Try an override locale as
first fallback"
Change-Id: I23d8459fff4d76fd9d542d098953bf97cf5397a1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147219
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
