? Success

User tests: Successful: Unsuccessful:

avatar drmmr763
drmmr763
15 Jun 2014

Remove index.html files from the joomla-cms. Used from the CLI:
find . -name 'index.html' -type f -delete

avatar drmmr763 drmmr763 - open - 15 Jun 2014
avatar wilsonge
wilsonge - comment - 15 Jun 2014

How do build scripts now work e.g. index references here https://github.com/joomla/joomla-cms/blob/staging/build/build.php#L86

Plus I guess we can remove this script here https://github.com/joomla/joomla-cms/blob/staging/build/indexmaker.php

avatar drmmr763
drmmr763 - comment - 15 Jun 2014

I'm wondering if we should provide a way for the user to generate these files in case their host doesn't support another way to prevent this sort of directory browsing. In which case - Ian's script might be useful there.

avatar mbabker
mbabker - comment - 15 Jun 2014

The build script "tags" files in each top level folder to ensure they're always in the packages. The index.html files were used because they were constant, and some of the top level folders don't have files within them, just more folders.

avatar brianteeman brianteeman - change - 21 Aug 2014
Status New Pending
avatar nicksavov nicksavov - change - 21 Aug 2014
Labels Removed: ?
avatar brianteeman brianteeman - change - 23 Sep 2014
Category Code style
avatar wilsonge
wilsonge - comment - 7 Oct 2014

I hate to ask but are we gonna need to add each of these files into the script.php :(

avatar beat
beat - comment - 7 Oct 2014

btw, heads-up: git doesn't store at all completely empty folders, as it stores only files, so with this PR folders with only index.html will disappear ;-) .

avatar mbabker
mbabker - comment - 7 Oct 2014

I hate to ask but are we gonna need to add each of these files into the script.php :(

I wouldn't worry about it, if they're present on existing sites then so be it.

avatar zero-24
zero-24 - comment - 7 Oct 2014

btw, heads-up: git doesn't store at all completely empty folders, as it stores only files, so with this PR folders with only index.html will disappear ;-) .

Examples (Override and Cache Folder in Backend and Frontend):
https://github.com/joomla/joomla-cms/tree/staging/administrator/cache
https://github.com/joomla/joomla-cms/tree/staging/administrator/language/overrides
https://github.com/joomla/joomla-cms/tree/staging/cache
https://github.com/joomla/joomla-cms/tree/staging/language/overrides

avatar zero-24
zero-24 - comment - 7 Oct 2014
avatar infograf768
infograf768 - comment - 8 Oct 2014

and also /logs

avatar Hackwar
Hackwar - comment - 12 Oct 2014

Could we make this simple? index.html stays in empty folders and everything else gets removed. I created a PR against @drmmr763 s repo to do that. Lets get this approved. :smile:

avatar drmmr763
drmmr763 - comment - 13 Oct 2014

Merged in @Hackwar's changes, fixed the silly com_config change that shouldn't have been there.

avatar nicksavov
nicksavov - comment - 13 Oct 2014

@beat good catch! :)

avatar nicksavov nicksavov - change - 16 Oct 2014
Labels Added: ?
avatar compojoom
compojoom - comment - 17 Oct 2014

Okay, just tested and all index.html are gone, except for cache/index.php and administrator/cache/index.php, but this is expected...

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3788.

avatar compojoom compojoom - test_item - 17 Oct 2014 - Tested successfully
avatar wilsonge
wilsonge - comment - 17 Oct 2014

umm shouldn't logs/index.html exist as well?

avatar Hackwar
Hackwar - comment - 17 Oct 2014

You are right...

avatar elkuku
elkuku - comment - 17 Oct 2014

What if we place a .gitignore file inside those empty folders (that might give the additional benefit of git-ignoring the contents)?

Oh, and a BIG :+1: for doing this :wink:

avatar Bakual
Bakual - comment - 17 Oct 2014

What if we place a .gitignore file inside those empty folders (that might give the additional benefit of git-ignoring the contents)?

I would keep it as simple as possible for now. Removing the index.html from all non-empty folders will not generate any problems and doesn't need additonal code and can be merged based on review.

avatar Hackwar
Hackwar - comment - 17 Oct 2014

@elkuku .gitignore is as good as an index.html file...

avatar Hackwar
Hackwar - comment - 23 Nov 2014

Hi folks, this PR has been around quite a bit now... Could we please merge this into 3.4? There is nothing to test and nothing to do, except checking that it is indeed only index.html files that are deleted...

avatar roland-d
roland-d - comment - 23 Nov 2014

Just something we spoke about on the JBS chat today. Looking at this, are you also removing the index.html files from CodeMirror and TinyMce? If so, I believe they should remain as they are part of a 3rd party package.

avatar Hackwar
Hackwar - comment - 23 Nov 2014

The index.html files in CodeMirror should not be part of this PR yet, since they have been added 2 months ago, the PR is 4 months old. The index.html files in TinyMCE have been added by us ourselfs.

avatar gwsdesk
gwsdesk - comment - 23 Nov 2014

We should have a 'hands-off' policy on all 3rd party as discussed on the chat. So if we have those in Tiny do they have a valid function or can we do that different?

avatar Hackwar
Hackwar - comment - 23 Nov 2014

No, we should not have a hands-off policy for 3rd party libraries, because there is no reason to ship the complete documentation of phpmailer for example with Joomla. If people need that documentation, they can look at it at the phpmailer dev site. There is no reason for us to add a Megabyte of static docs to our repository from a third party. I would remove the index.html files in codemirror, too, because they, too, are simply documentation for a developer when they want to implement this feature in their system and thus unnecessary for a normal enduser. But I honestly couldn't care less about those files. leave them in if you want.

Regarding the TinyMCE index.html files: WE added those files into TinyMCE according to our rule at that time that all folders should have such an index.html. They are not originaly from TinyMCE, they are not necessary for the functioning of TinyMCE and they are completely useless otherwise as well.

avatar gwsdesk
gwsdesk - comment - 23 Nov 2014

I partly agree but as Roland mentioned, they will be back in codemirror
or wherever as soon as an upgrade arrives. Understand me right please I
am all in favor to get rid of all those files as long as we do not break
things
On 11/23/2014 6:06 PM, Hannes Papenberg wrote:

No, we should not have a hands-off policy for 3rd party libraries,
because there is no reason to ship the complete documentation of
phpmailer for example with Joomla. If people need that documentation,
they can look at it at the phpmailer dev site. There is no reason for
us to add a Megabyte of static docs to our repository from a third
party. I would remove the index.html files in codemirror, too, because
they, too, are simply documentation for a developer when they want to
implement this feature in their system and thus unnecessary for a
normal enduser. But I honestly couldn't care less about those files.
leave them in if you want.

Regarding the TinyMCE index.html files: WE added those files into
TinyMCE according to our rule at that time that all folders should
have such an index.html. They are not originaly from TinyMCE, they are
not necessary for the functioning of TinyMCE and they are completely
useless otherwise as well.


Reply to this email directly or view it on GitHub
#3788 (comment).

avatar Hackwar
Hackwar - comment - 23 Nov 2014

As well as the documentation for phpmailer would be back if we copied it verbatim from the source. The job of the next guy updating codemirror would be to remove those again before commiting it to our repo. But since its simply useless bits and bytes that we are lugging around, its totally irrelevant if we have it or don't have it or if it gets added back in later on. Right now its identical to driving a rock around in your trunk. Doesn't hurt. Doesn't help either.

avatar gwsdesk
gwsdesk - comment - 23 Nov 2014

It costs bandwidth so it does hurts as would in your example the
increase of weight takes more petrol so your remark is very valid "The
job of the next guy updating codemirror would be to remove those again
before committing it to our repo." So we could start by removing those
from this 3.4 release?
On 11/23/2014 6:18 PM, Hannes Papenberg wrote:

As well as the documentation for phpmailer would be back if we copied
it verbatim from the source. The job of the next guy updating
codemirror would be to remove those again before commiting it to our
repo. But since its simply useless bits and bytes that we are lugging
around, its totally irrelevant if we have it or don't have it or if it
gets added back in later on. Right now its identical to driving a rock
around in your trunk. Doesn't hurt. Doesn't help either.


Reply to this email directly or view it on GitHub
#3788 (comment).

avatar roland-d
roland-d - comment - 23 Nov 2014

Hannes does make a valid point, that the documentation can be looked up at the website of the 3rd party plugin. As long a removing documentation from a 3rd party plugin doesn't interfere with it's workings I am fine with it. I mean, if there is a help button inside the editor that breaks because we remove the index.html file, one can wonder if we should remove it.

This should indeed be the responsibility of the maintainer but I don't know if we have set maintainers or it is just someone seeing it needs an update and creates a PR. This person may not be aware of what has been done in the past. We should document this somewhere.

avatar Bakual
Bakual - comment - 23 Nov 2014

For the libraries we pull in using composer, we added the doc and testing files to our .gitignore. See lines starting at https://github.com/joomla/joomla-cms/blob/staging/.gitignore#L46
This way we can pull in the whole library repo using composer, and only what we really need gets pushed into our own repo.
But this gets offtopic here a bit :smile:

avatar Bakual
Bakual - comment - 23 Nov 2014

I find this actually very hard to review since GitHub doesn't show me the whole diff. There are 1283 changed files, but it only shows me the first 300. Thus I can't review it online. I have to download the diff into my IDE and see what it shows. PhpStorm calculates that since about 20 minutes, so I doubt that's gonna work well :smile:

I wonder how you guys reviewed/tested that :smile:

avatar dgt41
dgt41 - comment - 23 Nov 2014

@Bakual Why don’t you apply this and just search in terminal for index.php? It sounds easier...

avatar Bakual
Bakual - comment - 23 Nov 2014

@dgt41 Don't tell me there is still a terminal in the year 2014. I prefer a GUI wherever I can use one.

avatar mbabker
mbabker - comment - 23 Nov 2014

GUI's are restrictive :stuck_out_tongue_closed_eyes:

Command line interfaces expose the full toolset of whatever you're working with

/me steps off soapbox :wink:

avatar Bakual
Bakual - comment - 23 Nov 2014

I used to think the same back in the days of DOS :wink:

avatar dgt41
dgt41 - comment - 23 Nov 2014

DOS 6.22 Those were the days…

avatar brianteeman
brianteeman - comment - 23 Nov 2014

Pah -youngsters
On 23 Nov 2014 16:53, "Dimitris Grammatiko" notifications@github.com
wrote:

DOS 6.22 Those were the days…

avatar infograf768
infograf768 - comment - 24 Nov 2014

Did not we say that logs/index.html should stay?

avatar wilsonge
wilsonge - comment - 24 Nov 2014

Yes

avatar okonomiyaki3000
okonomiyaki3000 - comment - 10 Dec 2014

I could be wrong but I reckon that 1283 is a few too many. By my count there should be 1239 deleted files. I arrived at this number by the following methodology so please let me know if it's wrong.

  • checkout a new branch based on staging
  • run the following shell commands to remove all index files then put them back in empty folders
find . -type f -name index.html | xargs rm
find . -type d -empty | xargs git checkout
  • commit the changes then run diff --name-status against staging and count the lines
avatar Hackwar
Hackwar - comment - 10 Dec 2014

Since this PR is unfortunately not behaving like a good cheese and thus gets better with ageing, I'm thinking if it would be better to do this in batches instead. Like, removing all index.html from /components in one PR and then all from /modules in another. In the hopes that it can be reviewed by a CMS maintainer more easily...

avatar Bakual
Bakual - comment - 10 Dec 2014

I fully agree with Hannes. This PR can't easily reviewed due to the sheer amount of files.
GitHub only shows the first 300 changed files.

If someone wants to do smaller PRs with max 300 removed files, then I can review and merge them fast.

Make sure you don't remove index.html files from empty folders as that would remove the folder in Git as well.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 10 Dec 2014

I can do it soon.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 10 Dec 2014
avatar Hackwar
Hackwar - comment - 10 Dec 2014

@okonomiyaki3000 THANK YOU! :smile:

avatar okonomiyaki3000
okonomiyaki3000 - comment - 10 Dec 2014

@Hackwar No problem. It's really easy to do something like this if you use the command line, right @mbabker ?

avatar zero-24
zero-24 - comment - 10 Dec 2014

@Bakual here is the PR to remove also the indexmaker script: #5383

avatar Bakual
Bakual - comment - 10 Dec 2014

Going to close this PR in favor of the smaller ones.
Thanks all!

avatar Bakual Bakual - close - 10 Dec 2014
avatar Bakual Bakual - change - 10 Dec 2014
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2014-12-10 10:35:23

Add a Comment

Login with GitHub to post a comment