Tom Prince [:tomprince] Assignee
	Description • 7 years ago

This causes

ValueError: Item already in manifest: mozmill/resources/**

since Bug 1416465.

It looks like the only place this occurs is actually unused.

	Tom Prince [:tomprince] Assignee
	Comment 2 • 7 years ago
mozreview-review

Comment on attachment 8928653 [details]
Bug 1417605: Stop using multiple patterns in TEST_HARNESS_FILES pointing to the same destination.

https://reviewboard.mozilla.org/r/199888/#review205018

It looks like all the files that actually exist here are included via some other path (in [`mailnews/moz.build`](https://dxr.mozilla.org/comm-central/rev/9d2ede676b3bceaa3fbf860324ec6fd7c24463c0/mailnews/moz.build#65-91).

	Jorg K (CEST = GMT+2)
	Comment 3 • 7 years ago

I see a potential fix is already being tried. Thanks. Looking at that, we have C++ bustage:
db/mork/src/morkFile.cpp:654:37: error: ignoring return value of 'size_t fwrite(const void*, size_t, size_t, FILE*)', declared with attribute warn_unused_result [-Werror=unused-result]
Maybe platform specific since Linux and Mac flag more warnings.

Meanwhile, here's try run with bug 1416465 backed out:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a4212eb15d4f861d8e99b2bf1cffee19462fce61

	Philipp Kewisch [:Fallen] [:📆][:🧩]
	Comment 4 • 7 years ago

(In reply to Tom Prince [:tomprince] from comment #2)
> It looks like all the files that actually exist here are included via some
> other path (in
> [`mailnews/moz.build`](https://dxr.mozilla.org/comm-central/rev/
> 9d2ede676b3bceaa3fbf860324ec6fd7c24463c0/mailnews/moz.build#65-91).

Are you sure this is true? The mozmill path adds to TEST_HARNESS_FILES.mozmill.resources while the other adds to TEST_HARNESS_FILES.xpcshell.mailnews.data and TEST_HARNESS_FILES.xpcshell.mailnews.resources

Could this be a shortcoming of the m-c bug that doesn't check the context for the value?

	Tom Prince [:tomprince] Assignee
	Comment 5 • 7 years ago

The problem is that there are multiple overlapping patterns pointing at TEST_HARNESS_FILES.mozmill.resources which is potentially racy if files in multiple locations have the same name. This was supported accidentally before, and the linked bug fixed it so it isn't supported.

Some grepping suggested that these files aren't used at the paths created by these entries. If not, we'll need to fix it either by splitting up were these files are put (and adjusting the tests that use them to use the new paths) or listing files explicitly.

	Tom Prince [:tomprince] Assignee
	Comment 7 • 7 years ago
mozreview-review

Comment on attachment 8928653 [details]
Bug 1417605: Stop using multiple patterns in TEST_HARNESS_FILES pointing to the same destination.

https://reviewboard.mozilla.org/r/199888/#review205128

So, the original attempt didn't work. Fix this by being explict about the files we use (and incidentally make it slightly clearer where the files are coming from in the js code too.

	Pulsebot
	Comment 10 • 7 years ago

Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/comm-central/rev/5a4b1f1d4e46
Stop using multiple patterns in TEST_HARNESS_FILES pointing to the same destination; rs=bustage-fix

	Philipp Kewisch [:Fallen] [:📆][:🧩]
	Comment 11 • 7 years ago
mozreview-review

Comment on attachment 8928653 [details]
Bug 1417605: Stop using multiple patterns in TEST_HARNESS_FILES pointing to the same destination.

https://reviewboard.mozilla.org/r/199890/#review205276

::: mail/test/mozmill/moz.build:17
(Diff revision 4)
> -    '/%s/mailnews/test/fakeserver/**' % CONFIG['commreltopsrcdir'],
> -    '/%s/mailnews/test/resources/**' % CONFIG['commreltopsrcdir']
>  ]
> +
> +
> +def mailnews_files(files, comm=CONFIG['commreltopsrcdir']):

I like this, can we use it in othe places as well?