662796 - Remove installchrome.pl and installcfunc.pl

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Joey Armstrong [:joey]]

1) A make driven unit test should be written for the script to verify existing functionality.

2) Convert script into a .pm module rather than calling "return(1);" to simulate the behavior.  This is likely legacy code used for importing modules with require, foo.pm + "use module" is the newer form.  installcfunc.pl can remain but it should become a very thin wrapper.

3) Add "use strict; use warnings;" (-vs- perl -w)

4) Replace shell globbing "<$inPath${gPathDelimiter}>" with opendir() or one of the newer 5.10.x modules with builtin support for shell pattern globbing.  Forking a shell to gather files can fail when build machines are under a heavy load or are unable to spawn new processes.

5) String & regex comparisons could be replaced with hashes for optimization and to greatly reduce the code footprint.  A unit test will need to be setup before this is attempted.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I think this is dead code.  installcfunc.pl appears to only be used from installchrome.pl, which appears to be unused.  This was probably replaced with python many moons ago.

Comment 2

[Benjamin Smedberg]

In fact, I have never heard of this script, and I've been in charge of the chrome install scripts since 2003. Rip it out!

Comment 3

[Joey Armstrong [:joey]]

(In reply to comment #2)
> In fact, I have never heard of this script, and I've been in charge of the
> chrome install scripts since 2003. Rip it out!

This sounds like a relatively safe edit then :)

Comment 4

[(not currently active) Ted Mielczarek]

I think his point was that we'd just like a patch to "hg rm" the files since they're unused. No point in cleaning up the trash.

Comment 5

[Joey Armstrong [:joey]]

(In reply to comment #4)
> I think his point was that we'd just like a patch to "hg rm" the files since
> they're unused. No point in cleaning up the trash.

Yes understood, removal cannot be much "safer" than when the script has atrophied to the point that a maintainer is not familiar with the code.

Comment 6

[Joey Armstrong [:joey]]

Attachment: Remove installchome.pl and installcfunc.pl

Comment 7

[Benjamin Smedberg]

Comment on attachment 538520 [details] [diff] [review]
Remove installchome.pl and installcfunc.pl

Only one review is necessary.

Comment 8

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/3a59f48381ad

Comment 9

[Mounir Lamouri (:mounir)]

Merged:
http://hg.mozilla.org/mozilla-central/rev/3a59f48381ad

Comment 10

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0

I checked that the files were removed from the repo:
http://hg.mozilla.org/mozilla-central/file/cc1e08803869/config

Is there a test case to verify this manually or is this enough to mark this as Verified Fixed?

Thanks!