Ticket #677 (closed cage: fixed)

Opened 6 years ago

Last modified 4 years ago

Which tools/*/*.pl programs belong in which tools/* directory?

Reported by: jkeenan Owned by: jkeenan
Priority: minor Milestone:
Component: configure Version: 1.1.0
Severity: low Keywords: tools
Cc: Coke, allison Language:
Patch status: applied Platform:

Description

(I posed this question in  RT #41912 two months ago but got no response. I want to close that ticket because its main concerns have been addressed. So I am re-posing the question here in Trac.)

We have many Perl 5 programs stored in subdirectories of tools/. Do we have a rule as to which programs ought to be under tools/build/, which under tools/dev, and which under, say, tools/util/?

I thought the rule -- or a rule -- went like this: If the program is invoked by make, put it under tools/build. (Example: tools/build/pmc2c.pl.) If the program is one a Parrot developer uses as a one-off aid, put it under tools/dev. (Example: tools/dev/nopaste.pl.)

However, today I noticed that there are quite a few tools/dev/ programs that make their way into the top-level Makefile:

$ grep -n 'tools/dev' Makefile
116:RECONFIGURE      := $(PERL) tools/dev/reconfigure.pl
640:    @$(PERL) tools/dev/cc_flags.pl $(CUR_DIR)/CFLAGS $(CC) "" $(CFLAGS) -I$(@D) -o $@ -c $<
645:    @$(PERL) tools/dev/cc_flags.pl $(CUR_DIR)/CFLAGS $(CC) "" $(CFLAGS) -I$(@D) -o $@ -c $<
647:#UNLESS(win32)      @$(PERL) tools/dev/cc_flags.pl $(CUR_DIR)/CFLAGS $(CC) "" $(CFLAGS) -I$(@D) -o $@ -c $<
866:    @$(PERL) tools/dev/cc_flags.pl $(CUR_DIR)/CFLAGS echo $(CC) $(CFLAGS) -I$(@D) -o  xx$(O) -c xx.c
882:$(PBC_TO_EXE) : tools/dev/pbc_to_exe.pir runtime/parrot/library/config.pir $(PARROT)
883:    $(PARROT) -o pbc_to_exe.pbc tools/dev/pbc_to_exe.pir
956:    $(PERL) tools/dev/lib_deps.pl object $(O_FILES)
959:    $(PERL) tools/dev/lib_deps.pl source all_source
2851:   $(PERL) tools/dev/svnclobber.pl
2857:   $(PERL) tools/dev/manicheck.pl
2860:   $(PERL) tools/dev/opsrenumber.pl $(OPS_FILES)
2863:   $(PERL) tools/dev/pmcrenumber.pl $(SRC_DIR)/pmc/pmc.num
3183:   $(PERL) tools/dev/install_files.pl \
3196:   $(PERL) tools/dev/install_dev_files.pl \
3227:   $(PERL) tools/dev/mk_inno.pl

Of the above, tools/dev/cc_flags.pl seems to be one which really should be in tools/build/.

And I don't know what rule applies to tools/util:

$ grep -n 'tools/util' Makefile
886:$(PARROT_CONFIG) : tools/util/parrot-config.pir $(PARROT) $(PBC_TO_EXE)
887:    $(PARROT) -o parrot_config.pbc tools/util/parrot-config.pir
985:    $(PARROT) -o parrot_config.pbc tools/util/parrot-config.pir
3292:   perlcritic --profile tools/util/perlcritic.conf $(CRITIC_FILES)
3296:   perlcritic -1 --profile tools/util/perlcritic-cage.conf $(CRITIC_FILES)

For example, why should perlcritic-cage.conf be under tools/util/ rather than tools/dev/?

Thoughts?

Attachments

tt677_toolsdirs.48639.diff Download (174.8 KB) - added by jkeenan 4 years ago.
diff of tt677_toolsdir against trunk at branch point

Change History

  Changed 6 years ago by jkeenan

  • type changed from bug to cage

  Changed 6 years ago by wayland

subscribe

  Changed 6 years ago by wayland

[09:09] <wayland76> Quick question; does anyone know the policy of which files go where inside the "tools" directory? (for more details, see https://trac.parrot.org/parrot/ticket/677 ) [09:09] <Coke> wayland76: there is no policy. [09:09] <Coke> stuff just got plopped in various locations. [09:09] <wayland76> Coke: Ok, do we want a policy? [09:09] <wayland76> Or at least a guideline or something? [09:10] <Coke> guideline good.

Here's an excerpt from an IRC log, with irrelevant comments removed. The times are GMT+1000, on Friday 22nd May.

  Changed 6 years ago by wayland

Auto-wrapping bad.

[09:09] <wayland76> Quick question; does anyone know the policy of which files go where inside the "tools" directory? (for more details, see https://trac.parrot.org/parrot/ticket/677 )

[09:09] <Coke> wayland76: there is no policy.

[09:09] <Coke> stuff just got plopped in various locations.

[09:09] <wayland76> Coke: Ok, do we want a policy?

[09:09] <wayland76> Or at least a guideline or something?

[09:10] <Coke> guideline good.

follow-up: ↓ 6   Changed 6 years ago by wayland

Here's some suggested documentation until we have something better:


The tools/ directory was begun as an ad-hoc collection of random files. This guideline suggests what kind of files go where. It is hoped that, over time, the files will migrate to the places suggested in this guideline. If you really want to put a file somewhere where this guideline doesn't suggest, then that probably means that this guideline needs updating.

build Tools called by the build process

dev Tools that are called on a one-off, ad-hoc basis by developers

docs Tools that help generate the documentation from source

install Not sure about these; they're probably good for something

util No rule appears to govern these either; maybe they should be merged with "dev"


It seems to me that a number of the scripts can probably be turfed out now that we don't have all the languages stored with Parrot. In particular, it seems that tools/install/smoke_languages.pl can go, which means that the install directory contains only the one file. That seems to me to mean that that file should go somewhere else, and the install directory deleted.

in reply to: ↑ 5 ; follow-ups: ↓ 7 ↓ 9   Changed 6 years ago by jkeenan

  • status changed from new to assigned
  • owner set to jkeenan

Replying to wayland:

Here's some suggested documentation until we have something better: ----- The tools/ directory was begun as an ad-hoc collection of random files. This guideline suggests what kind of files go where. It is hoped that, over time, the files will migrate to the places suggested in this guideline. If you really want to put a file somewhere where this guideline doesn't suggest, then that probably means that this guideline needs updating. build Tools called by the build process dev Tools that are called on a one-off, ad-hoc basis by developers docs Tools that help generate the documentation from source install Not sure about these; they're probably good for something util No rule appears to govern these either; maybe they should be merged with "dev"

Wayland,

I was thinking about this overnight and came to very similar conclusions. I think tools/build should hold programs invoked by make (or, more precisely, make all). tools/install/ should hold the two install programs. tools/docs: self-explanatory. tools/dev: probably everything else, including make targets other than all, install and install-dev.

We'll need to make up a list of programs which need to be migrated from one directory to another. We should also try to determine a Parrot Design Document location in which to place whatever rule we decide upon.

Thank you very much.

kid51

in reply to: ↑ 6   Changed 6 years ago by coke

Replying to jkeenan:

We'll need to make up a list of programs which need to be migrated from one directory to another. We should also try to determine a Parrot Design Document location in which to place whatever rule we decide upon.

A simple tools/README will probably suffice.

  Changed 5 years ago by wayland

./install/smoke_languages.pl ==> Removed ./install/smoke.pl ==> dev

./dev/install_files.pl ==> install ./dev/install_dev_files.pl ==> install

./util/* ==> dev

Anything else?

in reply to: ↑ 6 ; follow-up: ↓ 10   Changed 4 years ago by jkeenan

  • cc Coke, allison added

Replying to jkeenan:

I think tools/build should hold programs invoked by make (or, more precisely, make all).

Prodded by the "close TTs", I took a look at this ticket tonight for the first time in many months.

With regard to the 11 Perl 5 programs currently found in tools/build/: 7 are clearly invoked in the course of make all. The other 4 are:

tools/build/addopstags.pl
tools/build/fixup_gen_file.pl
tools/build/headerizer.pl
tools/build/ops2c.pl

I have posted to list a question as to whether we can now eliminate ops2c.pl outright.

headerizer.pl should move to tools/dev/. Although there is a make headerizer target, that target is not a prerequisite for make all. It is clearly a Parrot developer's tool used for the purpose of getting source code files into the best condition possible.

addopstags.pl: I was unfamiliar with this program until tonight. It's invoked as part of make tags-vi, as indicated by this passage from config/gen/makefiles/root.in:

tags-vi: tags.vi.dummy
    $(RM_F) tags
    @ctags@ \
    --links=no --totals \
    -R --exclude=blib --exclude=.svn  \
    --languages=c,perl --langmap=c:+.h,c:+.pmc,c:+.ops \
    -I NOTNULL,NULLOK,ARGIN,ARGMOD,ARGOUT,ARGINOUT,ARGIN_NULLOK,
ARGOUT_NULLOK,ARGMOD_NULLOK,ARGFREE,ARGFREE_NOTNULL \
    .
    $(PERL) $(BUILD_TOOLS_DIR)/addopstags.pl $(OPS_FILES)

So, it too belongs in tools/dev rather than tools/build.

fixup_gen_file.pl: I was also unfamiliar with this program until tonight. Here is an excerpt from its documentation:

NAME
       tools/build/fixup_gen_file.pl - mark a generated file as generated

SYNOPSIS
           % perl tools/build/fixup_gen_file.pl <options> <generated_file> <dest_file>

DESCRIPTION
       This script adds special headers and footers to files generated by
       tools outside of Parrot's normal build process.  This is so that people
       do not accidentally modify these files.

So, like headerizer.pl, this is a tool which Parrot developers use to get files into better shape. I recommend that it move to tools/dev/ as well.

Any disagreements? (Note: I have not yet examined those programs already found in tools/dev/. But I would appreciate feedback on the 3 proposed directory shifts mentioned above.

Thank you very much.

kid51

in reply to: ↑ 9 ; follow-up: ↓ 11   Changed 4 years ago by coke

Replying to jkeenan:

tools/build/fixup_gen_file.pl

Checkout compilers/imcc/Rules.in - only used when --maintainer option to configure, but it is still used during the build.

in reply to: ↑ 10   Changed 4 years ago by jkeenan

Replying to coke:

Replying to jkeenan:

tools/build/fixup_gen_file.pl

Checkout compilers/imcc/Rules.in - only used when --maintainer option to configure, but it is still used during the build.

Thanks for bringing that to my attention.

I've been dipping into the Perl 5 programs in tools/dev/ today -- adding some POD here; posing some questions on list there. All the programs I've looked at are appropriately placed in that directory. That would suggest that the remaining tasks are:

1. Decide what, if any, useful purpose a separate tools/util/ directory serves.

2. Move addopstags.pl and headerizer.pl from tools/build/ to tools/dev/.

3. Write a README for each directory.

Does that seem like a correct approach?

Thank you very much.

kid51

follow-up: ↓ 15   Changed 4 years ago by jkeenan

I have now reviewed the files in tools/util/. 5 of these files are used by the Release Manager during the monthly release process:

 crow.pir
 gen_release_info.pl
 inc_ver.pir
 release.json
 templates.json

I propose that they be moved to a new directory called tools/release, that references be modified in all files and that a README be created in that directory.

The remaining 7 files in tools/util/ would then be moved to tools/dev/ and tools/util/ would be eliminated:

ncidef2pasm.pl
parrot-config.pir
perlcritic-cage.conf
perlcritic.conf
perltidy.conf
pgegrep
update_copyright.pl

These steps, plus steps 2 and 3 from my last post, constitute my proposal. Comments?

Thank you very much.

kid51

  Changed 4 years ago by jkeenan

Created tt677_toolsdir branch to work on this ticket. See attached diff.

Changed 4 years ago by jkeenan

diff of tt677_toolsdir against trunk at branch point

  Changed 4 years ago by jkeenan

  • patch set to new

in reply to: ↑ 12   Changed 4 years ago by coke

Replying to jkeenan:

These steps, plus steps 2 and 3 from my last post, constitute my proposal. Comments?

+1

  Changed 4 years ago by jkeenan

As per discussion on parrot-dev list (NotFound++, coke++ and pmichaud++) I'm separating the question of pgegrep from this issue and moving it directly to examples/tools/. See r48698.

kid51

  Changed 4 years ago by jkeenan

  • patch changed from new to applied

Branch was merged into trunk at r48706.

  Changed 4 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

No complaints in 3 days. Closing ticket.

Note: See TracTickets for help on using tickets.