Review Parrot's 'make help'

[jkeenan]

When a Parrot developer invokes make help, slightly over 100 lines are printed to STDOUT. Most of those lines describe various make targets. However, it has been a long time since these targets were reviewed for the following qualities:

• Existence: Does a particular target listed in help actually still exist in Makefile?
• Functionality: Does a particular target still work as documented?
• Accuracy: Does the help message really describe what the target does?
• Absence: Are there targets in Makefile which are not listed in help but which ought to be?

We need a report which evaluates Parrot's make help with respect to the qualities listed above -- and any other relevant qualities. The report should appear in a plain-text file attached to this ticket.

The report may be accompanied as appropriate by patches to config/gen/makefiles/root.in and other files in Parrot's configuration system used to generate the Makefile.

This ticket will be offered as a Google Code-In task.

Thank you very much.

kid51

[vmax]

Doing a GCI task ( http://www.google-melange.com/gci/task/show/google/gci2010/parrot_perl_foundations/t129385113378). Sorry for my English :)

[cotto]

copy/pasting because I'm too lazy to be bothered to look at an attachment

1) make without arguments is supposed to build parrot and documentation, but it also builds parrot_utils (except parrot-prove), so we may think that all and world targets are the same. Maybe, we can remove parrot_utils from make help or make 'world' the default target

2) Targets not listed in make help: miniparrot cagecritic some pasm examples (cat,fact,trace) <-- add them to 'Examples' parrot-prove, ops2c (in parrot_utils) configure_tests <-- may be usable for Parrot devs

3)make archclean doesn't clean some *.o files, but object files are arch-dependant, aren't they?

4)make 'distclean' and 'realclean' targets do the same thing.

5)make installable can be described better in make help

*6)circular dependencies: docs-clean <-> clean docs <-> docs-dummy html <-> html-dummy htmlhelp <-> htmlhelp-dummy

*7)html-clean,pdf depend on themselves

8)Fix 6),7) and add htmlhelp-clean and pdf-clean to docs-clean *]Maybe, these are features, not bugs :). I've run configure.pl on Ubuntu 10.04 , Perl v5.10.1, perldoc --version reports "Perldoc v3.14_04, under perl v5.010001 for linux"

[jkeenan]

As I noted in  a comment on google-melange, I'm not entirely satisfied with the report submitted, so I'm going to keep the Trac ticket open.

[jkeenan]

commit 01a3028: 'make parrot_utils' must explicitly depend on 'all'.

[jkeenan]

Do we need to maintain make install-dev going forward?

	@echo "Installation:"
	@echo "  install:           Install under '$(PREFIX)' on Unix systems."
	@echo "  install-dev:       Same as 'install'."

[jkeenan]

When you configure with a --prefix directory, then build and proceed to install documentation ...

perl Configure.pl --prefix=/path/to/parrot
make
make install-doc

... you will get documentation installed under

/path/to/parrot/share/doc/

So far so good.

If you then call make docs, you generate documentation for about 14 files. But this documentation is placed under your build directory, not under your install directory:

./docs/doc-prep
./docs/ops/bit.pod
./docs/ops/cmp.pod
./docs/ops/core.pod
./docs/ops/experimental.pod
./docs/ops/io.pod
./docs/ops/math.pod
./docs/ops/object.pod
./docs/ops/pmc.pod
./docs/ops/set.pod
./docs/ops/string.pod
./docs/ops/sys.pod
./docs/ops/var.pod
./docs/packfile-c.pod

Is this what we expect? Is anyone familiar with the history of make docs enough to venture an opinion?

Thank you very much.

kid51

[jkeenan]

commit f935437: Remove instance of new_item() for deleted program 'tools/dev/list_unjitted.pl'.

[darbelo]

Replying to jkeenan:

Do we need to maintain make install-dev going forward?

It was left in the makefile for backwards compatibility with some HLL install scripts, in particular Rakudo, which have most likely been updated since. At this point it's been long enough since the unification with 'make install' that it should be safe to remove.

[jkeenan]

commit 57dbc6b: Eliminate 'testp' target, which is declared exactly the same as the older 'testr' target.

[jkeenan]

make coretest and make smolder_coretest are currently dysfunctional. They give failures here in the same contexts in which make test and make smolder_test give PASSes.

Test Summary Report
-------------------
t/pmc/packfile.t                  (Wstat: 0 Tests: 45 Failed: 14)
  Failed tests:  12-21, 28-30, 43
t/pmc/packfileannotations.t       (Wstat: 0 Tests: 17 Failed: 1)
  Failed test:  2
t/pmc/packfileconstanttable.t     (Wstat: 0 Tests: 13 Failed: 3)
  Failed tests:  1-3
  Parse errors: Bad plan.  You planned 15 tests but ran 13.
t/pmc/packfiledirectory.t         (Wstat: 0 Tests: 20 Failed: 5)
  Failed tests:  5-6, 18-20
  Parse errors: Bad plan.  You planned 17 tests but ran 20.
t/pmc/packfilerawsegment.t        (Wstat: 0 Tests: 7 Failed: 3)
  Failed tests:  1-2, 5
Files=222, Tests=7497, 80 wallclock secs ( 0.95 usr  0.55 sys +
 46.64 cusr 12.62 csys = 60.76 CPU)
Result: FAIL

IIRC, there has been on-and-off discussion in recent months as to where we draw the boundary between 'Parrot core' and 'Parrot non-core'. This question then spills over into the boundary between make coretest and make test. These t/pmc/packfile*.t tests are apparently falling on the 'wrong' side of the boundary at the moment.

This should probably be a separate TT or post to parrot-dev, but I'm logging the problem here as part of the make help review.

kid51

[jkeenan]

Replying to jkeenan:

make coretest and make smolder_coretest are currently dysfunctional.

Correction to the above (I posted before make coretest had completed -- bad.)

I'm getting the failures shown in make smolder_coretest but not in make coretest. This ought to make the problem more easily fixable.

[jkeenan]

Replying to jkeenan:

I'm getting the failures shown in make smolder_coretest but not in make coretest. This ought to make the problem more easily fixable.

Fixed.

commit 24c6f3a9: Correct 'smolder_coretest' target to include 'pbctestfiles', just as we did for 'coretest'.

[jkeenan]

Currently, the make help output for make distclean reads:

Removes also anything built, in theory.

But this is false. Currently,

distclean : realclean

So it's just an alias. I have corrected the help message accordingly. But can anyone come up with code for distclean that would do something else?

commit b0ceaaa: Correct description of what 'distclean' currently does.

[jkeenan]

Am attaching the relevant part of the output of make apilist, make exportlist and make malloclist.

Because there are warnings thrown in each list with respect to src/nci/libffi.c, I am opening a separate ticket for those.

kid51

[jkeenan]

make tags-vi and make tags-emacs are two targets with which I am not familiar. Can anyone describe their purpose?

In any event, I got inconsistent results. Both appeared to work on Linux; both appeared to fail on Darwin. Attaching outputs of each target.

Thank you very much.

kid51

[coke]

On Sun, Jan 9, 2011 at 5:12 PM, Parrot <parrot-tickets@lists.parrot.org> wrote:
> #1922: Review Parrot's 'make help'
> ---------------------+------------------------------------------------------
>  Reporter:  jkeenan  |       Owner:  jkeenan
>     Type:  cage     |      Status:  assigned
>  Priority:  normal   |   Milestone:
> Component:  docs     |     Version:  2.10.0
>  Severity:  low      |    Keywords:
>     Lang:           |       Patch:
>  Platform:           |
> ---------------------+------------------------------------------------------
>
> Comment(by jkeenan):
>
>  `make tags-vi` and `make tags-emacs` are two targets with which I am not
>  familiar.  Can anyone describe their purpose?
>
>  In any event, I got inconsistent results.  Both appeared to work on Linux;
>  both appeared to fail on Darwin.  Attaching outputs of each target.
>
>  Thank you very much.
>
>  kid51

"tags-vi" generates a tags file for vi. (so you can do "vi -t
<function name>" and have vi open the correct file of the definition
and start you on that line. (I don't use vi, but assume the other does
the same.)

Presumably (since you don't show the failure) you just don't have tags
installed on darwin. (works here on darwin)



-- 
Will "Coke" Coleda

[jkeenan]

Replying to jkeenan:

Am attaching the relevant part of the output of make apilist, make exportlist and make malloclist. Because there are warnings thrown in each list with respect to src/nci/libffi.c, I am opening a separate ticket for those.

See: TT #1942

[jkeenan]

Replying to jkeenan:

If you then call make docs, you generate documentation for about 14 files. But this documentation is placed under your build directory, not under your install directory:

Moved this issue to its own ticket: TT #1954.

[jkeenan]

Replying to cotto:

*6)circular dependencies: docs-clean <-> clean docs <-> docs-dummy html <-> html-dummy htmlhelp <-> htmlhelp-dummy *7)html-clean,pdf depend on themselves

I think this analysis is incorrect. Take, for instance, this code in config/gen/makefiles/root.in:

pdf:
    $(MAKE) docs pdf

pdf-clean:
    $(MAKE) docs pdf-clean

In config/inter/make.pm, we have this assignment:

$conf->data->set( make_c => "$prog -C" );

... where make option -C means "change to the directory found after -C before proceeding". This leads to this at the top of the Makefile:

MAKE = @make_c@

... which means that the pdf target translates to: "Change to the docs subdirectory and build the pdf target found in config/gen/makefiles/docs.in. And similarly for pdf-clean.

So I don't see any circularity here. Does that make sense?

kid51

[jkeenan]

In separate tickets, nwellnhof++ and NotFound++ took care of some warnings that appeared when we run make apilist, make exportlist and make malloclist.

But as I worked on those issues I realized that the way these targets are currently defined in config/gen/makefiles/root.in implicitly requires that ./parrot already have been built.

apilist: src/core_pmcs.c
    $(HEADERIZER) --macro=PARROT_API $(HEADERIZER_O_FILES)

exportlist: src/core_pmcs.c
    $(HEADERIZER) --macro=PARROT_EXPORT $(HEADERIZER_O_FILES)

malloclist: src/core_pmcs.c
    $(HEADERIZER) --macro=PARROT_MALLOC $(HEADERIZER_O_FILES)

src/core_pmcs.c is a Configure-generated file. But the headerizer has a dependency which is not fulfilled until src/extend_vtable.c gets generated at some point during make:

headerizer : src/core_pmcs.c src/extend_vtable.c
    $(HEADERIZER) $(HEADERIZER_O_FILES) compilers/imcc/imcc.y

So when I call make apitest, I am prone to get this:

couldn't read 'src/extend_vtable.c': 
  No such file or directory at 
    lib/Parrot/Headerizer/Functions.pm line 124.
make: *** [apilist] Error 2

IMHO, once I've configured I feel I should be able to call any make target and have make build as needed until my target is built. In other words, I feel that if I really want make apilist, as much of Parrot as needs to get built to get to that target ought to be built.

I therefore propose the patch attached, which adds src/extend_vtable.c to those three make *list targets.

Thank you very much.

kid51

[jkeenan]

Correct dependencies for make apilist, exportlist and malloclist

[jkeenan]

Replying to jkeenan:

IMHO, once I've configured I feel I should be able to call any make target and have make build as needed until my target is built. In other words, I feel that if I really want make apilist, as much of Parrot as needs to get built to get to that target ought to be built. I therefore propose the patch attached, which adds src/extend_vtable.c to those three make *list targets.

Tonight I have a presentation "Making Sense of 'make'" at Perl Seminar NY. As the final exercise, we applied and tested that patch.

kid51

[jkeenan]

Replying to cotto:

2) Targets not listed in make help: miniparrot cagecritic some pasm examples (cat,fact,trace) <-- add them to 'Examples' parrot-prove, ops2c (in parrot_utils) configure_tests <-- may be usable for Parrot devs 8)Fix 6),7) and add htmlhelp-clean and pdf-clean to docs-clean *]Maybe, these are features, not bugs :). I've run configure.pl on Ubuntu 10.04 , Perl v5.10.1, perldoc --version reports "Perldoc v3.14_04, under perl v5.010001 for linux"

Some of these are more "internal targets" that we have created to enable greater precision within the Makefile. They're not really intended to be called on the command-line, though they could be. I'm thinking about including make cagecritic and make miniparrot in make help, but the others I think can safely be left out.

kid51

[cotto]

It'd be nice if we differentiated internal-only targets that aren't intended to be used directly so that we don't get confused while spelunking through the Makefile. Something as simple as a comment would be fine by me.

[jkeenan]

Replying to cotto:

It'd be nice if we differentiated internal-only targets that aren't intended to be used directly so that we don't get confused while spelunking through the Makefile. Something as simple as a comment would be fine by me.

I don't necessarily disagree with that -- but that would be scope-creep with respect to this TT. In this TT (which started out as a GCI task), we're focusing on those targets that are listed in make help. What you are looking for would be a much broader review of the Makefile. If you want that, I would recommend opening a new ticket. (But, frankly, I think that would be a low-priority item and I myself would not assume the task anytime soon.)

Thank you very much.

kid51

[cotto]

kid51, you're quite correct. Is this ticket otherwise ok to close?

[jkeenan]

Replying to cotto:

kid51, you're quite correct. Is this ticket otherwise ok to close?

I'm going to review all of the above comments, then close.

[jkeenan]

Here's a case where I bump up against my limited understanding of make. Reviewing the description in make help for miniparrot, I grepped:

$ grep -in miniparrot Makefile
532:MINIPARROT          = ./miniparrot$(EXE)
859:    runtime/parrot/include/datatypes.pasm $(MINIPARROT)
860:    $(MINIPARROT) -Iruntime/parrot/include config_lib.pir > $@
864:    $(MINIPARROT)
889:$(MINIPARROT) : frontend/parrot/main$(O) 
       include/parrot/api.h include/parrot/longopt.h $(LIBPARROT) \
9295:    $(MINIPARROT) \
9343:    $(MINIPARROT) \

Note that while there is an assignment to macro $(MINIPARROT) (line 532), there is no entry in the conventional format for targets for miniparrot, i.e., nothing like this:

miniparrot: <pre-requisites>
<TAB>action

Nonetheless, when you start from completed configuration, make miniparrot works and builds a ./miniparrot executable.

$ ls -l ./miniparrot 
-rwxr-xr-x 1 jimk jimk 32851 Jan 20 06:30 ./miniparrot

Moreover, this works for make parrot as well!

$ grep -nE '^parrot(:| )' Makefile
# no output, but ...
$ make parrot
# builds parrot executable

So what is it about make or the Makefile that enables us to build a target without defining it?

Thank you very much. kid51

[coke]

>  889:$(MINIPARROT) : frontend/parrot/main$(O)

There's the build rule.


-- 
Will "Coke" Coleda

[jkeenan]

Replying to coke:

 889:$(MINIPARROT) : frontend/parrot/main$(O)

There's the build rule.

Coke++, that was the last piece in the puzzle for me.

Follow-up:

1. I'm tightening up the formatting of make help a bit, eliminating about 4 lines.

2. My attention having been called to make cagecritic, I've started to play with it. If we had known about it during GCI, we could have created 1,224 more tasks! :-)

3. Discussion with cotto and coke suggested that, these days at least, miniparrot is more of an internal target than something someone would call on the command-line. Hence, am not including it in make help.

[master 71005d0] Trim some whitespace for more compact output. Provide description of 'make cagecritic'.

And with that I put this ticket to rest!

Thanks to vmax, cotto, coke, nwellnhof, NotFound and anyone I overlooked!

kid51