Closed Bug 1417605 Opened 5 years ago Closed 5 years ago

Stop using multiple patterns in TEST_HARNESS_FILES pointing to the same destination.

Categories

(Thunderbird :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 59.0

People

(Reporter: tomprince, Assigned: tomprince)

References

Details

Attachments

(1 file)

This causes

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

since Bug 1416465.

It looks like the only place this occurs is actually unused.
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).
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
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
(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?
Flags: needinfo?(mozilla)
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.
Flags: needinfo?(mozilla)
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.
Whiteboard: [PLR]
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
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
OS: Unspecified → All
Hardware: Unspecified → All
Target Milestone: --- → Thunderbird 59.0
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?
Attachment #8928653 - Flags: review?(philipp) → review+
Whiteboard: [PLR]
You need to log in before you can comment on or make changes to this bug.