Bug 66754 - Remove own implementations of various Perl functions in installer
Summary: Remove own implementations of various Perl functions in installer
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyBeginner, easyHack, skillPerl, topicCleanup
Depends on:
Blocks:
 
Reported: 2013-07-10 06:32 UTC by Jan Holesovsky
Modified: 2017-02-14 08:57 UTC (History)
4 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 2013-07-10 06:32:43 UTC
Some Perl knowledge is needed for this Easy Hack.

solenv/bin/modules/installer/systemactions.pm implements for example create_directory() that can be exchanged with standard Perl's make_path() [http://perldoc.perl.org/File/Path.html].  Similarly other functions there, for which standard (or widely used) implementations exist.

What needs to be done (in the create_directory case; but similarly for more functions there):

Find all Perl scripts that use create_directory:

git grep '\<create_directory\>'

Edit all those that are Perl (end with .pl or .pm) so that you add use File::Path qw(make_path); at the top and then instead of create_directory() calls, you use make_path().

Then, remove the create_directory and create_directory_with_privileges declarations / definitions.

As the last step, make sure that everything builds & packages still :-)
Comment 1 Björn Michaelsen 2013-10-04 18:46:38 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 Robinson Tryon (qubit) 2013-10-23 17:24:21 UTC Comment hidden (obsolete)
Comment 3 bavincen 2014-09-23 06:26:58 UTC
(In reply to comment #0)
> Some Perl knowledge is needed for this Easy Hack.
> 
> solenv/bin/modules/installer/systemactions.pm implements for example
> create_directory() that can be exchanged with standard Perl's make_path()
> [http://perldoc.perl.org/File/Path.html].  Similarly other functions there,
> for which standard (or widely used) implementations exist.
> 
> What needs to be done (in the create_directory case; but similarly for more
> functions there):
> 
> Find all Perl scripts that use create_directory:
> 
> git grep '\<create_directory\>'
> 
> Edit all those that are Perl (end with .pl or .pm) so that you add use
> File::Path qw(make_path); at the top and then instead of create_directory()
> calls, you use make_path().
> 
> Then, remove the create_directory and create_directory_with_privileges
> declarations / definitions.
> 
> As the last step, make sure that everything builds & packages still :-)

Hi jan
can we keep that  create_directory and implement make path in it.
and remove  create_directory_with_privileges ,

need a breif explanation :) thanks
Comment 4 Taylor Lee 2015-06-10 07:12:11 UTC
bavincen,

I think your solution is a step in the right direction.  I find 'sub create_directory' in the following 4 files:

 setup_native/scripts/admin.pl
 solenv/bin/modules/installer/systemactions.pm
 solenv/bin/modules/par2script/systemactions.pm
 solenv/bin/modules/pre2par/systemactions.pm

The last 2 files don't seem to need make_path as there is no implementation of recursive directory creation; mkdir seems sufficient.

In the first 2 files, I think it makes sense to replace the contents of create_directory() with make_path and some error detection as was implemented in create_directory_with_privileges().

I argue that there are many calls to installer::systemactions::create_directory($var), so does changing all calls to "make_path($var, {mode=0775})" make sense?  Maybe having mode in a central function is a good thing?  Not sure.

> egrep -ciIR installer::systemactions::create_directory | egrep -v ':0$'
solenv/bin/modules/installer.pm:6
solenv/bin/modules/installer/parameter.pm:2
solenv/bin/modules/installer/worker.pm:6
solenv/bin/modules/installer/windows/mergemodule.pm:3
solenv/bin/modules/installer/windows/idtglobal.pm:2
solenv/bin/modules/installer/windows/strip.pm:1
solenv/bin/modules/installer/windows/update.pm:1
solenv/bin/modules/installer/windows/msp.pm:7
solenv/bin/modules/installer/windows/admin.pm:2
solenv/bin/modules/installer/strip.pm:1
solenv/bin/modules/installer/profiles.pm:1
solenv/bin/modules/installer/simplepackage.pm:9
solenv/bin/modules/installer/archivefiles.pm:2
solenv/bin/modules/installer/scpzipfiles.pm:2
solenv/bin/modules/installer/download.pm:1
solenv/bin/modules/installer/copyproject.pm:1
Comment 5 Robinson Tryon (qubit) 2015-12-14 07:10:42 UTC Comment hidden (obsolete)
Comment 6 Josh Baldwin 2016-01-26 02:18:18 UTC
Hello, I would like to take up this task as my first LO commit.  I will start with just one file though, to make sure I understand the process.  Then I will change all create_directory() to make_path().  After that, I can research more functions we can move into the core and leave those findings here for more new developers.

Question though for Taylor:
>I argue that there are many calls to >installer::systemactions::create_directory($var), so does changing all calls to >"make_path($var, {mode=0775})" make sense?  Maybe having mode in a central >function is a good thing?  Not sure.

I do follow your questions.  Can you explain your thought process in more detail?  Thanks!
Comment 7 Jan Holesovsky 2016-01-26 08:01:23 UTC
Josh: Great to see you hacking on this!  Let me CC JanI who will be able to help you with pushing your patches etc. :-)
Comment 8 jani 2016-01-28 08:00:51 UTC
(In reply to Jan Holesovsky from comment #7)
> Josh: Great to see you hacking on this!  Let me CC JanI who will be able to
> help you with pushing your patches etc. :-)

Yes I am here to help non-committers getting their first patch submitted.

if you have problems/questions please feel free to email me directly 
jani@documentfoundation.org

Of also using IRC and our mailing list is a good idea.

We have created a step by step guide, to help you over the first steps:
https://wiki.documentfoundation.org/Development/GetInvolved/DeveloperStepByStep

have fun
Comment 9 Josh Baldwin 2016-01-31 14:03:24 UTC
I am having email issues, so I'll post this here:

I made the change to /setup_native/scripts/admin.pl.  I did

egrep -ir "admin.pl" .

to see where in libreoffice admin.pl used.  However, it does not appear to be called anywhere.  Any ideas on how this file is used? Is it manually called and not from any other part of libreoffice?  I wanted to know so I can properly test the code before submitting.  Any insight will be greatly appreciated.
Comment 10 Christian Lohmaier 2016-02-01 09:53:37 UTC
just read the file and you'll see that's it can be used to do a special kind of installation using the created msi installer packages. It is not used during building of LO.
Comment 11 jani 2016-04-18 07:30:38 UTC
A polite ping, still working on this issue?
Comment 12 jani 2016-06-03 06:02:57 UTC
Unassigning due to lack of work, if you still work on the issue, please assign it again.