473236 - Remove executable bit from files that don't need it

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: list of all executable files in mozilla-central

Currently, there are 3881 files in mozilla-central that have their executable bit set.  The vast majority of those files are clearly not intended to be executed.

I'm filing this bug on un-setting the executable bit for the files that clearly don't need it.

Comment 1

[Daniel Holbert [:dholbert]]

Note:  I tried to fix this for a bunch of files last July, as
   http://hg.mozilla.org/mozilla-central/rev/8a99db3f8680
but the change was accidentally reverted in someone's merge a day or so later.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: list of file extensions which shouldn't be executable

Here's an alphabetical list of file extensions whose files I'm pretty sure shouldn't be executable.  Next to each extension, I've included the count of offending executable files in mozilla-central.

You can use this list to clear the executable bits by running the following command (on Ubuntu Linux, and probably on Mac as well):
   for x in `cat ~/Desktop/extension_counts.txt | cut -f1 -d' '`; do  \
     find . -type f -name "*\.$x" -exec chmod -x {} \; ; done

where "~/Desktop/extension_counts.txt" is the name of this saved attachment.

That command removes the executable bits from 3344 files.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: list of filenames which shouldn't be executable

And here's a list of two no-extension filenames which shouldn't be executable -- "README" and "Makefile" -- and the corresponding counts of offending files.

This command will remove the executable bits from all these files:
   for x in `cat ~/Desktop/filename_counts.txt | cut -f1 -d' '`; do  \
     find . -type f -name "$x" -exec chmod -x {} \; ; done

That gets rid of 77 executable files, leaving a total of 460.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: patch v1: Remove executable bits from files matching previous two attachments.

Here's a patch resulting from running the commands in my previous two comments.

Since this fix changes file modes, the "patch" command doesn't understand it -- you need to apply it with "hg import", like this:
   hg import --no-commit ~/Desktop/executable_fix.patch

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: [ignore -- same as previous list of file extensions]

I noticed that the ".ex" extension actually was for files that expect to be executed ("hdiutil-expect.ex" and "installdmg-expect.ex").

I'm removing that one from the list of file extensions, and reposting the list & patch.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: [ignore -- same as previous patch]

Here's an updated patch that doesn't strip executable-status from ".ex" files, per my previous comment.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: list of file extensions which shouldn't be executable v2a

d'oh, I somehow didn't actually remove the "ex" extension from my previous 2 attachments... here's the actual updated list of extensions.

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: patch v2a: allow *.ex to be executable

Comment 9

[Daniel Holbert [:dholbert]]

Comment on attachment 356619 [details] [diff] [review]
patch v2a: allow *.ex to be executable

bsmedberg, would you be an appropriate reviewer for this?

(I'd recommend mostly looking at attachment 356618 [details] and attachment 356605 [details] -- the patch is long, but it's just generated from those shorter attachments via the "for x in" commands listed above)

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: patch v2b: updated to current mozilla-central

Here's the patch, regenerated using up-do-date mozilla-central.  Carrying forward bsmedberg's r+.

Comment 11

[Daniel Holbert [:dholbert]]

Patch v2b pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/0d6969b2a955

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: potential false-positives, non-NSS

At ted's suggestion, I ran through the files tweaked by patch v2b, to see if any of them have "#!" in the first line (and hence maybe expect to be executable).

Here's the list, for non-NSS.  (I'm separating NSS out because (a) that just gets periodically dumped from CVS and (b) the potential false-positives there are all pretty uniform -- they're all Makefiles.)

Comment 13

[Daniel Holbert [:dholbert]]

Attachment: potential false-positives, NSS

here's the list for NSS.

The command I used to generate these is (newlines added for clarity):
for x in `cat /home/dholbert/Desktop/executable_fix_new.patch
    | grep "diff \-\-git"
    | cut -f3 -d' '`; do
  head -n1 $x | grep "\#\!";
  if [[ $? = 0 ]]; then 
    echo $x; 
  fi;
done

Comment 14

[Daniel Holbert [:dholbert]]

(In reply to comment #13)
> The command I used to generate these is (newlines added for clarity):
(Also -- before running that, I created a symlink called "a" created that points to my mozilla-central source, and ran the command in a's parent directory.  That just meant I didn't have to strip the initial "a" off of the filenames taken from the patch. :) )

Comment 15

[Daniel Holbert [:dholbert]]

Comment on attachment 358447 [details]
potential false-positives, non-NSS

Here are the potential false-positives, arranged in sub-groups:

>#! /bin/sh
>a/build/autoconf/config.guess
>#! /bin/sh
>a/build/autoconf/config.sub

Ted already fixed these in bug 474973.

>#!/bin/sh
>a/build/unix/mozilla-config.in
>#! /bin/sh
>a/nsprpub/build/autoconf/config.guess
>#! /bin/sh
>a/nsprpub/build/autoconf/config.sub
>#!/bin/sh
>a/nsprpub/config/nspr-config.in
>#! /bin/sh
>a/toolkit/crashreporter/google-breakpad/autotools/config.guess
>#! /bin/sh
>a/toolkit/crashreporter/google-breakpad/autotools/config.sub

Looks like the above files expect to have executable status.

>#!/usr/bin/js
>a/testing/tools/pageloader/test/chrome/content/pageloader.js
>#!/usr/bin/js
>a/testing/tools/pageloader/test/chrome/content/report.js

Apparently we run some tests-processing scripts with a command called "/usr/bin/js"? Interesting.  I should probably re-enable those, just to be safe.

>#!/usr/bin/perl
>a/toolkit/mozapps/installer/CFGParser.pm
>#!/usr/bin/perl -w
>a/config/Moz/Milestone.pm

pm = perl library = shouldn't be run directly, right? These don't need to be executable, I'd think.

>#!nmake
>a/embedding/qa/mozembed/makefile.win
>#!nmake
>a/embedding/qa/mozembed/public/makefile.win
>#!nmake
>a/tools/trace-malloc/makefile.win

nmake --> these are for Visual Studio, which is on Windows & doesn't care about exec bits.

>Binary file (standard input) matches
>a/toolkit/themes/winstripe/global/toolbar/spring.gif

And, images don't need executable status, even if they have #! in their first line. :)

Comment 16

[Daniel Holbert [:dholbert]]

Attachment: Followup Patch v3a: re-add exec status to some files, non-nss

This patch re-enables executable status for the files in the top half of comment 15.

Comment 17

[Daniel Holbert [:dholbert]]

Attachment: Followup Patch v3b: re-add exec status to some files, nss

And here's a patch re-adding executable status to Makefiles with "#! gmake" in the "nss" folder, because
 - it's conceivable that someone might depend on running them directly
 - their current executable status in mozilla-central will be blown away next time we get a snapshot of NSS anyway.

Comment 18

[(not currently active) Ted Mielczarek]

diff --git a/nsprpub/config/nspr-config.in b/nsprpub/config/nspr-config.in

Note that nsprpub/ is also imported from NSPR's CVS repo.

Comment 19

[Daniel Holbert [:dholbert]]

(In reply to comment #18)
> Note that nsprpub/ is also imported from NSPR's CVS repo.
Ah, good to know -- so any other changes I made there there will get overwritten at next import.  Oh well -- after followup v3a, nspr only had 6 files that my patch tweaked. (2 .c files, 1 each .cpp/h/.tmpl files, and a Makefile.in)  6 more unnecessarily-executable files won't be a huge deal. :)

Comment 20

[Daniel Holbert [:dholbert]]

Landed followup patches with approval from Sheriff Ted.
 Followup Patch v3a: http://hg.mozilla.org/mozilla-central/rev/bdf096d22018
 Followup Patch v3b: http://hg.mozilla.org/mozilla-central/rev/a5ceb7405ca2

Comment 21

[Daniel Holbert [:dholbert]]

Landed another batch of these for test files in /layout:
  http://hg.mozilla.org/integration/mozilla-inbound/rev/26114d0e9609

Comment 22

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

http://hg.mozilla.org/mozilla-central/rev/26114d0e9609

Comment 23

[Joe Drew (not getting mail)]

http://hg.mozilla.org/mozilla-central/rev/62df2afae1c7

Comment 24

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/623c2704b45b

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/623c2704b45b

Comment 26

[Daniel Holbert [:dholbert]]

Removed executable bit from 56 png files that were formerly marked as executable:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/23c638395987

Comment 27

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/23c638395987