Bug 98602 - Duplicate code in onlineupdate/
Summary: Duplicate code in onlineupdate/
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.0.1.2 release
Hardware: All All
: medium normal
Assignee: Dipankar Niranjan
URL:
Whiteboard: target:5.2.0 target:5.3.0
Keywords: difficultyInteresting, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2016-03-11 13:06 UTC by Jan Holesovsky
Modified: 2017-02-14 08:58 UTC (History)
3 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 Jan Holesovsky 2016-03-11 13:06:12 UTC
onlineupdate/source/update/src/ and onlineupdate/source/libmar/src contain the same files.

The onlineupdate/Executable_mar.mk should be split to creating a library (libmar), and the mar executable - and the libmar should be used by both updater and mar.

It is enough (maybe even preferred) if libmar is a static library.  Have a look at */*StaticLibrary* to see other static libraries - hopefully one of them can be used as an example.

Then the onlineupdate/source/update/src can be removed.
Comment 1 Commit Notification 2016-03-14 09:17:47 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98602 Duplicate code in onlineupdate/

It will be available in 5.2.0.

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 2 Jan Holesovsky 2016-03-14 09:22:27 UTC
Thanks a lot! :-)  Closing...
Comment 3 Commit Notification 2016-03-14 09:34:01 UTC
Jan Holesovsky committed a patch related to this issue.
It has been pushed to "master":

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

Revert "tdf#98602 Duplicate code in onlineupdate/"

It will be available in 5.2.0.

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 4 Jan Holesovsky 2016-03-14 09:34:19 UTC
Unfortunately turned out the code does not compile :-(

Dipakar: To test it, you need to configure with:

--enable-online-update=mar

The first thing I needed to do was

 $(eval $(call gb_Executable_Executable,updater))
 
 $(eval $(call gb_Executable_set_include,updater,\
+       -I$(SRCDIR)/onlineupdate/source/libmar/src \
        -I$(SRCDIR)/onlineupdate/source/update/inc \
        -I$(SRCDIR)/onlineupdate/source/update/common \
        -I$(SRCDIR)/onlineupdate/source/update/updater/xpcom/glue \

But even with that, there were linking errors.

Can you please have a look at that?  If you get stuck, please poke me :-)
Comment 5 Dipankar Niranjan 2016-03-14 22:07:16 UTC
Hi kendy,

I have a couple of tests today so couldn't delve into the issue deeply..
I did look into it, but couldn't gather much.. The linker error still persists..
I'll take a closer look tomorrow and let you know if I'm making progress or if I get stuck..
Comment 6 Alfredo 2016-03-20 18:22:38 UTC
Hi, can I still take this easyHack?
Comment 7 Dipankar Niranjan 2016-03-20 18:57:20 UTC
(In reply to Alfredo from comment #6)
> Hi, can I still take this easyHack?

Review pending.. :) Will be resolved after that..
https://gerrit.libreoffice.org/#/c/23312/
Comment 8 jani 2016-05-26 09:06:19 UTC
A polite ping, still working on this patch ?

In case it is only review missing, ping the reviewers.
Comment 9 Dipankar Niranjan 2016-05-26 15:01:44 UTC
(In reply to jan iversen from comment #8)
> A polite ping, still working on this patch ?
> 
> In case it is only review missing, ping the reviewers.

Well, kendy is the reviewer..
Comment 10 Commit Notification 2016-06-13 07:46:30 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98602 Duplicate code in onlineupdate/

It will be available in 5.3.0.

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 11 Jan Holesovsky 2016-06-13 07:53:41 UTC
Dipankar: Thank you for doing this - the code is much better without the copy there :-)