Discussion:
[GSoC] "Modularize Automake" Final Report
Matthias Paulmier
2018-08-13 23:37:48 UTC
Permalink
Hello everyone,

As you may know, I worked on Automake this summer as part of the GSoC.
Here is my final report on this project. I hope you will find it
informative.

__________________________________

[GSOC] REFACTORING: FINAL REPORT
__________________________________


1 Introduction
==============

As the program comes to an end, it is time to report on the work that
has been done during this period. This is my final report for the
GSoC project "Modularize Automake to improve the test-suite". In this
report, I will attempt to explain the work that has been done during
this summer. As I expected with this project, there was a lot of work
to do and there is still a fair bit left to do to totally complete the
project.

The work I have done can be found on the [Automake git repo] on the
branch [experimental/gsoc/refactoring]. The commits I made go from
`be2458dd8` to `2d7d3fd31`.


[Automake git repo] http://git.savannah.gnu.org/cgit/automake.git

[experimental/gsoc/refactoring]
http://git.savannah.gnu.org/cgit/automake.git/log/?h=experimental/gsoc/refactoring


2 The Exporter module
=====================

This is probably not a very crucial point but we now have a
homogeneous usage of the Exporter module. Before that we had
different uses depending on the module. Some of them used the
following:
,----
| use Exporter;
| use vars qw (@ISA @EXPORT);
| @ISA = qw (Exporter);
| @EXPORT = qw ([...]);
`----

While others used:
,----
| require Exporter;
| [...]
`----

All of those modules declared themselves as sub classes of the
Exporter module. I didn't know which of `use' or `require' provided
the best option or if there was a difference in terms of performance
at all. So I went and asked more experienced Perl devs about that and
was advised to use the following form:
,----
| use Exporter 'import';
| use vars qw (@EXPORT);
| @EXPORT = qw ([...]);
`----

So now, none of our modules inherit from the Exporter class and they
all use the same form. What could be investigated now is whether
`@EXPORT' is really what we want or rather use the `@EXPORT_OK'
variable and let the "clients" (even though here we only have
`automake' and `aclocal') choose which function/method they need. In
theory, the second option seems to be a more reasonable approach,
however I didn't investigate this option much.


3 New modules
=============

Some new modules have been created during this period. Of course,
there hasn't been new features added, but the code is now better
organized and functions are grouped coherently together.

The following modules have been created:

Module name Role
---------------------------------------------------------------------------------
CondStack Checks on the conditional stack
ConfVars Define new configure variables
Errors Print errors for ac and am files
File Read the contents of a file
Global Holds global variable/constant declarations
HandleConfigure Handle configure files from automake's point of view
LangHandling Helper functions for defining different languages
Requires Functions for requiring files in 'Makefile.am's
Standards Functions for checking the conformity with GNU/GNITS standards
Texi Functions for handling texinfo files
Utils Miscellaneous functions
VarAppend Helper functions for the Variable module

Although I said in the previous report that the goal is to move all
the functions away from `bin/automake[.in]', I think letting some of
them in it might better alternative (maybe Utils.pm). For instance
`usage', `version' and `parse_arguments' seem to fit in the main
script as they are strongly bound to it.


4 Tests
=======

I have added some tests under the `t/pm/' directory as part of the
testsuite. I still need to work on my testing skills as I have some
issues coming up with good tests for non trivial functions.

I have also removed a "hack" in the test suite for which I am pretty
pleased. Some Perl tests are testing that a misuse of the library
effectively creates a fatal error. So far, we just had each of those
test cases in separate files that were expected to fail as they ended
in a fatal error. I have been able to overcome this problem with the
Perl `eval' function that is able to capture the errors of a block of
code without interrupting the program. Now Perl tests are properly
grouped by module (some modules are tested twice: once without the use
of the `threads' lib and once with it) and all PASS as expected.


5 What more could be done
=========================

5.1 Tests
~~~~~~~~~

I am not an expert on tests but I am sure we could come up with more
test cases for certain modules. For example, I tried testing the
CondStack module which takes care of verifying that the successive
calls to `if', `else' and `endif' are in the correct order.
Unfortunately I later realized that my tests and assumptions on how
the module worked were somewhat wrong and I ended up removing the
tests from the repo as I couldn't come up with better tests.


5.2 Documentation
~~~~~~~~~~~~~~~~~

Although all the methods are properly documented, they don't all use
the same documentation conventions. Some use the [pod] format while
others use comments as documentation.


[pod] http://perldoc.perl.org/perlglossary.html#pod


5.3 More modules
~~~~~~~~~~~~~~~~

There are still some functions in the main script that should be put
in modules. As easy as it is to create new modules and dump those
functions inside them, my goal was to create modules with good
cohesion and keep coupling low. So it took a lot of time to come up
with modules or integrate functions to existing ones. If you followed
the repo, you may have noticed that I "reset" the branch somewhat
regularly. That's often because I realized, after creating some
modules, that they were poorly made or that their methods belonged
elsewhere. Even though the work seemed (and was mostly) repetitive, I
had to think a lot about that and it slowed me down quite a lot in
this task.


6 Communication
===============

At the beginning of the program, I said that I would communicate about
my progress regularly on this mailing list. As you may have noticed,
I didn't really do that. This is because we changed the way we were
originally supposed to communicate with Mathieu. We were originally
going to have an non formal vocal meeting every week and I would write
a report to the list every time it was necessary. After a few
meetings, we decided that we would change that to a weekly written
report to him. This was better for me for two reasons:
- No time constraint (I could write the report whenever I want during
the week).
- I could write it in french, which makes it faster for me.


7 Conclusion
============

This summer has overall been a great experience for me as I have
learned a lot about Perl and tests in general. This was a big
challenge for me as I have never worked on such a large code base (it
may not be much for others but it was quite a significant one for me).
As we have seen above, the project still needs some work as I have
said previously.

As you may know if you follow this list, Mathieu is stepping down as
maintainer of Automake. The code produced this summer will therefore
not be integrated into the project until someone is taking on that
role and is willing to review the changes.

I am not aware of anyone taking on the role of maintainer of Automake
so far but, when this person is there, I am willing to work with them
to be able to integrate my code into Automake as well as improving or
expanding it (while I will not be as available as this summer i am
still willing to contribute). Be it partially or in its entirety. I
think some parts could be very interesting for the project.
Especially the fix of the Perl `XFAIL' tests and the use of the
`Export' module inside Automake.


--
Matthias Paulmier
Mathieu Lirzin
2018-08-14 08:15:18 UTC
Permalink
Hello Matthias,
Post by Matthias Paulmier
This summer has overall been a great experience for me as I have
learned a lot about Perl and tests in general. This was a big
challenge for me as I have never worked on such a large code base (it
may not be much for others but it was quite a significant one for me).
As we have seen above, the project still needs some work as I have
said previously.
As you may know if you follow this list, Mathieu is stepping down as
maintainer of Automake. The code produced this summer will therefore
not be integrated into the project until someone is taking on that
role and is willing to review the changes.
I am not aware of anyone taking on the role of maintainer of Automake
so far but, when this person is there, I am willing to work with them
to be able to integrate my code into Automake as well as improving or
expanding it (while I will not be as available as this summer i am
still willing to contribute). Be it partially or in its entirety. I
think some parts could be very interesting for the project.
Especially the fix of the Perl `XFAIL' tests and the use of the
`Export' module inside Automake.
I really enjoyed reading your detailed and honest report, and even if I
have not been able to follow the second half of the coding period in
details, I am pleased with the work you have done.

Even if I stepped down I am still a commiter, so there is nothing
besides time which prevents me from integrating your work in particular
if those changes are safe. In theory, refactoring work can easily be
reviewed if for example you create one patch for each module extracted
with its corresponding test. I recommend to submit those patches
incrementally to <automake-***@gnu.org>.

Thank you for your participation in the GSoC!
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Matthias Paulmier
2018-08-21 17:38:13 UTC
Permalink
(This email was sent yesterday but I forgot to cc the mailing list)

Hello Mathieu,
Post by Mathieu Lirzin
Hello Matthias,
Post by Matthias Paulmier
This summer has overall been a great experience for me as I have
learned a lot about Perl and tests in general. This was a big
challenge for me as I have never worked on such a large code base (it
may not be much for others but it was quite a significant one for me).
As we have seen above, the project still needs some work as I have
said previously.
As you may know if you follow this list, Mathieu is stepping down as
maintainer of Automake. The code produced this summer will therefore
not be integrated into the project until someone is taking on that
role and is willing to review the changes.
I am not aware of anyone taking on the role of maintainer of Automake
so far but, when this person is there, I am willing to work with them
to be able to integrate my code into Automake as well as improving or
expanding it (while I will not be as available as this summer i am
still willing to contribute). Be it partially or in its entirety. I
think some parts could be very interesting for the project.
Especially the fix of the Perl `XFAIL' tests and the use of the
`Export' module inside Automake.
I really enjoyed reading your detailed and honest report, and even if I
have not been able to follow the second half of the coding period in
details, I am pleased with the work you have done.
Even if I stepped down I am still a commiter, so there is nothing
besides time which prevents me from integrating your work in particular
if those changes are safe. In theory, refactoring work can easily be
reviewed if for example you create one patch for each module extracted
with its corresponding test. I recommend to submit those patches
Cool, I didn't realize that (and that there are also other committers).
I'll try submitting them in the coming days, I just need to figure out a
good way of making these patches. I think I will be submitting the
changes to the test suite first, then the individual modules with their
corresponding tests.

Sorry for the late response, I was taking a small break after submitting
my final report.

--
Matthias Paulmier

Loading...