692625 - Flatten directories in the tree that only contain a single subdirectory and no files

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Ed Morley [:emorley]]

Broken out from bug 689884 comment 12.

There exists in the tree a number of directories whose parent, only contains that directory and nothing else - and so could be flattened into the parent.

Comment 1

[Ed Morley [:emorley]]

Attachment: Script to find such directories

Comment 2

[Ed Morley [:emorley]]

Attachment: Raw results

Output of the script. Obviously contains a number of directories that will not presumably not want to be flattened (eg db/sqlite/ even though it's the only folder in db/), and will need to go through manually.

Comment 3

[Ed Morley [:emorley]]

Dao, what are your thoughts about the results attachment? This bug was created on your suggestion (like you, I agree with it's intent), however I can see this turning into a nightmare of chasing module owners up and then people getting cross for weeks after, since none of their patch queues will then apply. 

Do you have any suggestions as to how best to go about this / roughly which of the types (eg tests folders that only contain one type of tests and so on) in the results file you see as worth being moved?

Comment 4

[(not currently active) Ted Mielczarek]

An alternative solution is to change the DIRS line for parent dirs. If you have foo/bar, which just has "DIRS = xyz", you could change foo/Makefile.in to read "DIRS = bar/xyz", remove foo/bar/Makefile.in, and not have to move any files around.

Comment 5

[Ed Morley [:emorley]]

(In reply to Ted Mielczarek [:ted, :luser] from comment #4)
> An alternative solution is to change the DIRS line for parent dirs

That's what I did in bug 689884 (all instances should now be changed, unless anyone's added any since), however in that bug Dao suggested that I should have collapsed the directories instead (bug 689884 comment 12), which was why I filed this. 

However now that I've scanned the tree I'm somewhat indifferent, given the sheer number of directories (only some were as a result of bug 689884), and given that for many of them their existence is justified or else perhaps going to cause more upset than it's worth to move.

Comment 6

[Ed Morley [:emorley]]

errr... and by Dao I of course meant Gavin throughout, whoops.

Sorry for the spam Dao.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

I think these should be fixed:
browser/components/sessionstore/test
browser/components/sidebar
browser/components/test
browser/devtools/styleinspector/test
browser/devtools/webconsole/test
toolkit/components/aboutmemory/tests
toolkit/components/startup/tests

I don't really care about the others.

Comment 8

[Ed Morley [:emorley]]

Attachment: A: browser/components/sessionstore/test

Comment 9

[Ed Morley [:emorley]]

Attachment: B: browser/components/sidebar

Comment 10

[Ed Morley [:emorley]]

Attachment: C: browser/components/test

Comment 11

[Ed Morley [:emorley]]

Attachment: D: browser/devtools/styleinspector/test

Rob, moves browser/devtools/styleinspector/test/browser/ to browser/devtools/styleinspector/test/ since the parent directory is otherwise empty.

Comment 12

[Ed Morley [:emorley]]

Attachment: E: browser/devtools/webconsole/test

Moves browser/devtools/webconsole/test/browser/ to browser/devtools/webconsole/test/

Comment 13

[Ed Morley [:emorley]]

Attachment: F: toolkit/components/aboutmemory/tests

Moves toolkit/components/aboutmemory/tests/chrome/ to toolkit/components/aboutmemory/tests/

Comment 14

[Ed Morley [:emorley]]

Attachment: G: toolkit/components/startup/tests

Moves toolkit/components/startup/tests/browser/ to toolkit/components/startup/tests/

Comment 15

[Ed Morley [:emorley]]

Attachment: H: Update *makefiles.sh to reflect new paths

(standing rs=build, so not requesting review)

Comment 16

[Ed Morley [:emorley]]

Meant to add: Everything passes try at the moment, and when reviews are complete, I'll be landing over a weekend whilst it's quiet, so I can tryserver again + land straight after, before other things can break it. (Given how unsuccessfully mercurial backs out renames, I want to make 110% sure this sticks).

Comment 17

[Nicholas Nethercote [inactive]]

Comment on attachment 578873 [details] [diff] [review]
F: toolkit/components/aboutmemory/tests

Review of attachment 578873 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like gavin already r+'d, but this seems fine to me -- I just copied the structure from somewhere else so I'm not greatly attached to it.

Comment 18

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 578871 [details] [diff] [review]
D: browser/devtools/styleinspector/test

looks ok.

Comment 19

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 578872 [details] [diff] [review]
E: browser/devtools/webconsole/test

in:

--- a/browser/devtools/webconsole/test/browser/browser_warn_user_about_replaced_api.js
+++ b/browser/devtools/webconsole/test/browser_warn_user_about_replaced_api.js
@@ -32,17 +32,17 @@
  * use your version of this file under the terms of the MPL, indicate your
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
-const TEST_REPLACED_API_URI = "http://example.com/browser/browser/devtools/webconsole/test//browser/test-console-replaced-api.html";
+const TEST_REPLACED_API_URI = "http://example.com/browser/browser/devtools/webconsole/test//test-console-replaced-api.html";

Extra '/' character in that URL. Should probably fix that.

another one of these in:

+++ b/browser/devtools/webconsole/test/browser_webconsole_basic_net_logging.js

I'm wondering if our previous move script was broken because all of these URI consts seem to have them.

Since they were like that when you got them, I'll R+ this with a follow-up to clean up the URIs.

Thanks for doing this!

Comment 20

[Rob Campbell [:rc] (:robcee)]

filed bug 707650 to clean up the extra characters in the webconsole tests.

Comment 21

[Dave Townsend [:mossop]]

Comment on attachment 578874 [details] [diff] [review]
G: toolkit/components/startup/tests

Review of attachment 578874 [details] [diff] [review]:
-----------------------------------------------------------------

What is the benefit of this move?

Comment 22

[Ed Morley [:emorley]]

(In reply to Dave Townsend (:Mossop) from comment #21)
> What is the benefit of this move?

Reducing unnecessary hierarchy depth in the trees + Dao asked that I do it in bug 689884 comment 12. Happy for you and Dao to come to an agreement either way and obsolete this part's patch if decided easiest.

Comment 23

[Ed Morley [:emorley]]

I've done it again!
s/Dao/Gavin/

(also remembering to add to CC this time)

Comment 24

[Ed Morley [:emorley]]

Slight rebase + including the move of a few more tests added in those directories since:
https://tbpl.mozilla.org/?tree=Try&rev=e26d522933e4

Comment 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d24c34dbf1c8
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e4c03e4c37b
https://hg.mozilla.org/integration/mozilla-inbound/rev/379a992ed77c
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3807e5362da
https://hg.mozilla.org/integration/mozilla-inbound/rev/524e771b72db
https://hg.mozilla.org/integration/mozilla-inbound/rev/c981969df93b
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ba5e09d2dbe
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f46911a6c6e

Comment 26

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d24c34dbf1c8
https://hg.mozilla.org/mozilla-central/rev/4e4c03e4c37b
https://hg.mozilla.org/mozilla-central/rev/379a992ed77c
https://hg.mozilla.org/mozilla-central/rev/b3807e5362da
https://hg.mozilla.org/mozilla-central/rev/524e771b72db
https://hg.mozilla.org/mozilla-central/rev/c981969df93b
https://hg.mozilla.org/mozilla-central/rev/5ba5e09d2dbe
https://hg.mozilla.org/mozilla-central/rev/8f46911a6c6e