Ticket #524 (closed cage: fixed)

Opened 6 years ago

Last modified 6 years ago

[CAGE] duplicate variables in configuration

Reported by: fperrad Owned by: jkeenan
Priority: normal Milestone:
Component: configure Version:
Severity: medium Keywords: configure
Cc: jkeen@… Language:
Patch status: applied Platform: all

Description (last modified by jkeenan) (diff)

Some variables are duplicated (with the same value) in configuration :

  • bin_dir & bindir
  • include_dir & includedir
  • lib_dir & libdir

The version with underscore is preferred.

Attachments

dir_simplify.patch Download (1.4 KB) - added by jkeenan 6 years ago.
Diff between trunk and dir_simplify branch, eliminating 3 deprecated attributes in init::install

Change History

  Changed 6 years ago by doughera

Why is the version with the underscore preferred? The versions with the underscore have been marked 'deprecated' in config/init/install.pm for years. The versions without the underscore match the Configure.pl command-line options for --bindir, --libdir, and --includedir. It seems to me that the versions without the underscore are the ones that should be preferred.

  Changed 6 years ago by fperrad

Ok, for without underscore (I don't know this deprecation)

So the following variables must be rename :

  • doc_dir
  • icu_dir

and perhaps :

  • blib_dir
  • build_dir

in reply to: ↑ description   Changed 6 years ago by jkeenan

  • cc jkeen@… added
  • description modified (diff)

Replying to fperrad:

Some specifics ...

$ grep -n "dir'" lib/Parrot/Config/Generated.pm 
127:             'bin_dir' => '/usr/local/bin',
128:             'bindir' => '/usr/local/bin',
130:             'blib_dir' => 'blib/lib',
131:             'build_dir' => '/homedir/work/parrot',
156:             'datadir' => '/usr/local/share',
158:             'doc_dir' => '/usr/local/share/doc',
249:             'i_sysdir' => 'define',
259:             'i_sysndir' => undef,
288:             'icu_dir' => '/usr',
291:             'include_dir' => '/usr/local/include',
292:             'includedir' => '/usr/local/include',
293:             'infodir' => '/usr/local/info',
318:             'lib_dir' => '/usr/local/lib',
319:             'libdir' => '/usr/local/lib',
320:             'libexecdir' => '/usr/local/libexec',
334:             'localstatedir' => '/usr/local/var',
341:             'mandir' => '/usr/local/man',
352:             'oldincludedir' => '/usr/include',
381:             'sbindir' => '/usr/local/sbin',
384:             'sharedstatedir' => '/usr/local/com',
387:             'srcdir' => '/usr/local/src',
390:             'sysconfdir' => '/usr/local/etc',
396:             'versiondir' => '/parrot/1.0.0-devel',

follow-up: ↓ 5   Changed 6 years ago by jkeenan

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

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

  • keywords configure added
  • platform set to all

I have created the dir_simplify branch in SVN to work on this ticket.

I propose to work in several small steps. I will first remove the three config attributes cited by fperrad which are obviously duplicates and which have long borne the comment # deprecated in config/init/install.pm.

Unless Coke or somebody tells me otherwise, I am going to assume that these are immediately removable (assuming nothing breaks) without being added to DEPRECATED.pod on the grounds that they are not part of Parrot's interface to language developers.

Once I've done that merge and assuming there are no complaints, I'll remove other "dirs with underscores" such as doc_dir one step at a time. The most likely candidates to be left unchanged would be blib_dir and (for Darwin) ports_include_dir and fink_include_dir.

Files which will need to be reviewed by others include:
-- docs/pdds/draft/pdd30_install.pod (around lines 189, 202)
-- tools/dev/mk_language_shell.pl (around lines 493, 494)

Changed 6 years ago by jkeenan

Diff between trunk and dir_simplify branch, eliminating 3 deprecated attributes in init::install

follow-up: ↓ 8   Changed 6 years ago by jkeenan

See patch just attached for elimination of bin_dir, lib_dir, include_dir. make and make test passed. tools/dev/mk_language_shell.pl appears to be okay, but I don't have an installed parrot which is a prerequisite for that.

Comments?

  Changed 6 years ago by jkeenan

  • patch set to new

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

  • patch changed from new to applied

Replying to jkeenan:

See patch just attached for elimination of bin_dir, lib_dir, include_dir. make and make test passed. tools/dev/mk_language_shell.pl appears to be okay, but I don't have an installed parrot which is a prerequisite for that.

No adverse comments, so bin_dir, lib_dir and include_dir bit the dust in r37861. dir_simplify branch has been merged and deleted.

I'll keep the ticket open until I/we reach a final decision on the other _dir instances.

Thank you very much.
kid51

follow-up: ↓ 10   Changed 6 years ago by doughera

I haven't looked at the list carefully, but here's what my overall thought was:

If there is a variable that can be controlled by a Configure.pl command-line option, it probably makes sense to have the variable name match the command-line option.

in reply to: ↑ 9   Changed 6 years ago by jkeenan

Replying to doughera:

I haven't looked at the list carefully, but here's what my overall thought was: If there is a variable that can be controlled by a Configure.pl command-line option, it probably makes sense to have the variable name match the command-line option.

Looks like we're already set there. In lib/Parrot/Configure/Options/Conf/Shared.pm, there are no options spelled _dir.

in reply to: ↑ 8   Changed 6 years ago by jkeenan

Replying to jkeenan:

No adverse comments, so bin_dir, lib_dir and include_dir bit the dust in r37861. dir_simplify branch has been merged and deleted.

Caught two usages I missed earlier in config/gen/makefiles/parrot_pc.in. (I suspect ack didn't recognize that as a source code file.)

Fixed in r37863.

  Changed 6 years ago by jkeenan

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

On second thought, since fperrad's original post was aimed at duplicate configuration attributes, and since we've eliminated them, I'm going to close this ticket.

We can take care of _dir odds and ends as time goes by.

Thank you very much.
kid51

Note: See TracTickets for help on using tickets.