Bug Hunting Session
Bug 60698 - kill pointless one-file library ...
Summary: kill pointless one-file library ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.0.0.3 release
Hardware: Other All
: medium normal
Assignee: Marcos Souza
URL:
Whiteboard: target:4.2.0 target:4.3.0
Keywords: difficultyInteresting, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2013-02-11 21:34 UTC by Michael Meeks
Modified: 2016-02-18 16:37 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2013-02-11 21:34:03 UTC
The fpicker/ Library_fpicker.mk contains only a single file and there is no need for it to live inside fpicker - it produces a ridiculously small standalone library that could easily live ~anywhere else.

It should be moved into vcl/ inside vcl/source/components

NB. please use 'git mv' and then git commit immediately to retain history.

We should re-work:

SvtMiscOptions().UseSystemFileDialog()

to use a direct configmgr access via sberg's nice new compiled-in API of that single setting;

officecfg/registry/data/org/openoffice/Setup.xcu:                <it>/org.openoffice.Office.Common/Misc/UseSystemFileDialog</it>

via.

bool officecfg::Office::Common::Misc::UseSystemFileDialog::get()

etc.

And we should cleanup all the horror RTL_CONSTASCII_USTRINGPARAM() mess at the same time - kill the fpicker library, and move the component file across:

cat fpicker/source/generic/fpicker.component

and merge it into the VCL component file.

Thanks ! :-)
Comment 1 Björn Michaelsen 2013-10-04 18:47:46 UTC
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 2 Marcos Souza 2013-10-14 14:33:20 UTC
Maybe fileaccess module is another candidate for this bug?

marcos@jedi:~/gitroot/core$ ls -R fileaccess/
fileaccess/:
Library_fileacc.mk  Makefile  Module_fileaccess.mk  README  source

fileaccess/source:
fileacc.component  FileAccess.cxx

This have just one file for this module,
Comment 3 Michael Meeks 2013-10-14 20:57:49 UTC
yes; sounds like a good plan :-) reducing that madness would be wonderful.
Comment 4 Marcos Souza 2013-10-17 02:38:22 UTC
I'm in the right direction here?

https://gerrit.libreoffice.org/#/c/6288/
Comment 5 Commit Notification 2013-10-18 12:35:54 UTC
Marcos Paulo de Souza committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=b538fc543ecfbafef94b15ef068890c22d7f2218

fdo#60698: Move fileaccess module to ucb



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 6 Robinson Tryon (qubit) 2013-10-23 16:48:39 UTC
Removing comma from whiteboard (please use a space to delimit values in this field)
https://wiki.documentfoundation.org/QA/Bugzilla/Fields/Whiteboard#Getting_Started
Comment 7 Marcos Souza 2013-10-25 12:23:41 UTC
Fpicker uses XComponentContext when creating the instance, while inside svtools ::cppu::createOneInstanceFactory it needs a XMultiServiceFactory...

What can I do in this case?
Comment 8 Stephan Bergmann 2013-10-25 13:25:00 UTC
(In reply to comment #7)
> Fpicker uses XComponentContext when creating the instance, while inside
> svtools ::cppu::createOneInstanceFactory it needs a XMultiServiceFactory...

Note that svt_component_getFactory (svtools/source/uno/miscservices.cxx) already has an else branch that uses cppu::component_getFactoryHelper on a cppu::ImplementationEntry array s_aServiceEntries, so you can merge g_entries from fpicker/source/generic/fpicker.cxx into it.
Comment 9 Marcos Souza 2013-10-25 16:08:50 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > Fpicker uses XComponentContext when creating the instance, while inside
> > svtools ::cppu::createOneInstanceFactory it needs a XMultiServiceFactory...
> 
> Note that svt_component_getFactory (svtools/source/uno/miscservices.cxx)
> already has an else branch that uses cppu::component_getFactoryHelper on a
> cppu::ImplementationEntry array s_aServiceEntries, so you can merge
> g_entries from fpicker/source/generic/fpicker.cxx into it.

Thanks Stephan!

https://gerrit.libreoffice.org/#/c/6436/

At least it don't gave me any error when linking, strange don't you think?

But, there is another libs that we could merge?
Comment 10 Marcos Souza 2013-10-30 22:08:12 UTC
I saw a lot of very small filters inside filter module:

-rwxrwxr-x  1 marcos marcos  168K Oct 30 13:06 libitglo.so
-rwxrwxr-x  1 marcos marcos  158K Oct 30 13:06 libipxlo.so
-rwxrwxr-x  1 marcos marcos  156K Oct 30 13:06 libiralo.so
-rwxrwxr-x  1 marcos marcos  155K Oct 30 13:06 libipblo.so

And a lot of others inside filter module.

But, it seems that this is used inside VCL:
ImpFilterLibCacheEntry::GetImportFunction

Do you think this can be unified into an bugger library, of all these small filters? As they're so small, and it seems they're used inside the same library.
Comment 11 Michael Meeks 2013-11-01 18:19:24 UTC
I imagine the idea is to save memory, but on any modern machine I suspect the shlib overhead, and the linking cost of that makes that not worthwhile. Of course - that is -assuming- that running 'ldd' on each of those shows that they have the same shared library dependencies.

Clearly it needs a lot of testing - I -hope- we have at least one (trivial) test image per file-format in git, and a unit test  for loading it - if not, that is something we should add here before re-factoring that I think.

Anyhow - thanks for poking at this ! :-)
Comment 12 Commit Notification 2013-11-04 12:22:37 UTC
Marcos Paulo de Souza committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=b40ffd288baf6dc5989cc09c5b5007eee97dd1bf

fdo#60698: Move generic fpicker to svtools



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2013-11-05 07:09:44 UTC
Marcos Paulo de Souza committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3a009c427fc04c0a1a100f5f04516cedd1f6f118

fdo#60698: Merge all libs of io...



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 14 Michael Meeks 2013-11-05 09:34:11 UTC
Nice work ! it all helps :-) having said that - I guess really we want to move to enable --enable-merge-libs=yes by default I suppose which then compiles a lot of these into a single library anyway :-)

But - either way, all these tiny fragments (if they don't break dependencies) waste linking time and RAM to load/link them - so thanks !
Comment 15 Marcos Souza 2013-11-05 09:37:13 UTC
The io module was a suggestion by Stephan!

And yes, I'll try to look for more cases like this :)

Thanks a lot for helping!
Comment 16 Marcos Souza 2013-11-06 17:22:53 UTC
It seems that in scripting module we have 6 libs that are used in the same conditions inside Repository.mk. They just link to some libs in some cases, but most of the depencies are equal.

I'll try to make some prototyping and see what happens :)
Comment 17 Commit Notification 2013-11-21 10:07:12 UTC
Marcos Paulo de Souza committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=668d6ada160438c396849a8309864f5fd33f33ac

fdo#60698: Unify spl and spl_unx



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 18 Commit Notification 2013-12-02 11:16:28 UTC
Marcos Paulo de Souza committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1c3d51fd6826fbc7d447243d40fcd351ad47ae84

fdo#60698: Merge tvhlp1 into ucpchelp1



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 19 Commit Notification 2013-12-04 10:48:58 UTC
Marcos Paulo de Souza committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=35c1d6f7617087c18b94e03ec9281c995e6d9e55

fdo#60698: Merge hatchwindowfactory into svt



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 20 Commit Notification 2013-12-11 20:46:17 UTC
Marcos Paulo de Souza committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=eebc1e9a8a5c7fd04b795c62791a70eed65995e4

fdo#60698: Merge fastsax and sax_shared into expwrap



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 21 Marcos Souza 2013-12-18 15:06:03 UTC
Hi guys!

After sending my patch about animcore, I saw that evtatt(eventattacher) just have one file... 

What's the best library to hold this implementation? Maybe basic?

By removing all these small libs and dirs, maybe some newcomers will don't be scaned of the size of libreoffice project :)
Comment 22 Commit Notification 2013-12-24 09:19:17 UTC
Marcos Paulo de Souza committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=692c724f5e71f685c269085983828e7539274ecb

fdo#60698: Merge odbcbase into odbc



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 23 Marcos Souza 2013-12-24 16:38:25 UTC
And about xmlreader library?

It just have 3 files... where we can put this?

And this module don't have a component file, so this would be easy to merge...
Comment 24 Stephan Bergmann 2014-01-06 10:16:32 UTC
(In reply to comment #23)
> And about xmlreader library?
> 
> It just have 3 files... where we can put this?

xmlreader is used by both the URE and LO layers, but is not part of the URE stable interface, so there is no obvious other library to merge it into.
Comment 25 Michael Meeks 2014-07-26 22:00:14 UTC
The original single file library is now fixed -> closing the Easy Hack =)
Thanks for the great work !
Comment 26 Robinson Tryon (qubit) 2015-12-16 00:37:13 UTC
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyInteresting SkillCpp TopicCleanup )
[NinjaEdit]
Comment 27 Robinson Tryon (qubit) 2016-02-18 16:37:26 UTC
Remove LibreOffice Dev List from CC on EasyHacks
(curtailing excessive email to list)
[NinjaEdit]