Closed Bug 1261643 Opened 4 years ago Closed 3 years ago

IOError: [Errno 2] No such file or directory: 'c:\\builds\\moz2_slave\\tb-c-cen-w32-d-000000000000000\\build\\objdir-tb\\dist\\thunderbird\\firefox-plugin-container.exe'

Categories

(Thunderbird :: Build Config, defect)

All
Windows
defect
Not set

Tracking

(Not tracked)

RESOLVED INVALID
Thunderbird 48.0

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Bug 1114647 renamed the plugin-container.exe to firefox-webcontent.exe and firefox-plugin-container.exe. firefox-webcontent.exe is pakaged normally but firefox-plugin-container.exe not.
Attached patch packFix.patch (obsolete) — Splinter Review
Add firefox-plugin-container.exe to package-manifest

Is this the way we could go? Try works with this patch https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8ce7786a72a1
Attachment #8737550 - Flags: review?(aleth)
I'm not sure, I think we should wait and see what happens in bug 1261416 and bug 1114647 first.
Blocks: e10s-rename
Depends on: 1261416
Comment on attachment 8737550 [details] [diff] [review]
packFix.patch

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

I think in this case we could land this (it'll be at least a day or two until the m-c fix is merged anyhow), but in general that's a harder question. 

For the package-manifest.in changes in general finishing bug 1158018 would be time very well spent for comm-central.
Attachment #8737550 - Flags: review?(aleth) → review+
Flags: needinfo?(mkmelin+mozilla)
Landed *temporary* fix:
https://hg.mozilla.org/comm-central/rev/ccb6b71cf7a5
Keywords: leave-open
Whiteboard: [temporary fix, remove later]
(In reply to Magnus Melin from comment #4)
I disagree with the r+ here - I don't see a similar line in browser/installer/package-manifest.in. It may make the build error go away, but I have no idea if it actually works. Maybe I am missing something? 

Temporary fixes should still be fixes ;)
(In reply to aleth [:aleth] from comment #6)
> (In reply to Magnus Melin from comment #4)
> I disagree with the r+ here
This was approved as a temporary measure.

> Temporary fixes should still be fixes ;)
We'll see whether it fixes the problem. According to the try run, it does.
If you have a hole in a tube, you put some sticky tape a hose clamp until you can fix it properly.
Flags: needinfo?(rkent)
It fixed the build, thanks, Richard!

We can now enjoy the look of duct tape and hose clamps and wait for the plumber to arrive in our dry living room that would otherwise be flooded ;-)
Attached patch packFix-2.patchSplinter Review
Sorry to come this late but I found now how FX is doing this packaging. This patch makes the similar. This patch is to apply on tip.

Try created the installer without failure: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=973a97f3afd2
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8737605 - Flags: review?(mkmelin+mozilla)
Attachment #8737550 - Flags: review-
Comment on attachment 8737605 [details] [diff] [review]
packFix-2.patch

This looks much better, thanks.
Attachment #8737605 - Flags: review?(mkmelin+mozilla) → review+
(In reply to Richard Marti (:Paenglab) from comment #9)
> This patch is to apply on tip.
The other patch has already landed.

So we back out the other one and land this one instead, right?
Comment on attachment 8737605 [details] [diff] [review]
packFix-2.patch

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

I was too quick. Looking at https://hg.mozilla.org/mozilla-central/rev/8f159c5d671e, it seems the windows installer also needs corresponding changes.
Attachment #8737605 - Flags: review+ → review-
https://hg.mozilla.org/comm-central/rev/dd20636a9759111a1de643c0f0f9bac9c74245a9
Backed out changeset ccb6b71cf7a5 (bug 1261643). a=aleth CLOSED TREE DONTBUILD
(In reply to Jorg K (GMT+2) from comment #7)
> > Temporary fixes should still be fixes ;)
> We'll see whether it fixes the problem. According to the try run, it does.

I don't think a try run is always enough, you have to actually look at the patch and understand it. Just because it builds doesn't mean it works.
(In reply to aleth [:aleth] from comment #12)
> Comment on attachment 8737605 [details] [diff] [review]
> packFix-2.patch
> 
> Review of attachment 8737605 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I was too quick. Looking at
> https://hg.mozilla.org/mozilla-central/rev/8f159c5d671e, it seems the
> windows installer also needs corresponding changes.

For clarity, I'm referring to the registry changes in browser/installer/windows/nsis/installer.nsi and uninstaller.nsi.
Hmm, it looks like TB doesn't have https://dxr.mozilla.org/mozilla-central/source/browser/installer/windows/nsis/installer.nsi#477. Is that an oversight or is there a good reason for it?
Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
https://hg.mozilla.org/comm-central/rev/79b497d14c8af8bea463d60b4b0edd1e23d4cbb7
Bug 1261643 - Port bug 1114647: add firefox-plugin-container.exe the same way to the package as FX. r=aleth a=bustage fix CLOSED TREE
Comment on attachment 8737605 [details] [diff] [review]
packFix-2.patch

Since the mediaplayer registry values were missing from TB so far, let's add them (or not) in a separate patch.
Attachment #8737605 - Flags: review- → review+
Attachment #8737550 - Attachment is obsolete: true
Yes they are not in installer.nsi nor in uninstaller.nsi. The question is, are this container used in TB?
Flags: needinfo?(richard.marti)
(In reply to Richard Marti (:Paenglab) from comment #19)
> The question is, is this container used in TB?
As far as I understand it, plugin-container.exe is for e10s (Electrolysis), that is, multiprocess FF, where every tab runs in it's own process. I don't think this applies to TB.
(In reply to Jorg K (GMT+2) from comment #20)
> (In reply to Richard Marti (:Paenglab) from comment #19)
> > The question is, is this container used in TB?
> As far as I understand it, plugin-container.exe is for e10s (Electrolysis),
> that is, multiprocess FF, where every tab runs in it's own process. I don't
> think this applies to TB.

iirc it is also used for plugins like Flash even without e10s (so that a flash crash is surviveable).
(In reply to aleth [:aleth] from comment #16)
> Hmm, it looks like TB doesn't have
> https://dxr.mozilla.org/mozilla-central/source/browser/installer/windows/
> nsis/installer.nsi#477. Is that an oversight or is there a good reason for
> it?

The  mediaplayer entry probably just not useful in Thunderbird. Not sure what it does in ff though...
Flags: needinfo?(mkmelin+mozilla)
Are we done here?
Flags: needinfo?(aleth)
Keywords: leave-open
Whiteboard: [temporary fix, remove later]
Well, bug 1261416 will likely undo the whole thing so... That's the reason this was intended as a tmp fix so we don't burn for a day or week while they decide.
Agreed. But now it looks more like a (permanent) fix and we'll need to follow bug 1261416 in another bug. Aleth, what's your plan?
Bug 1261416 needs no change on our files. I think it's safe to close this bug.
Given comment 22 I think this is done as well.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: needinfo?(aleth)
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 48.0
Aleth, should this patch now also be backed-out like the FX patches?
Flags: needinfo?(aleth)
https://hg.mozilla.org/comm-central/rev/ba8da2d7a902fae1a9ca0637fce64dc7e164ed71
Backed out changeset 79b497d14c8a (Bug 1261643) to match m-c. rs=bustage-fix CLOSED TREE
(In reply to Richard Marti (:Paenglab) from comment #30)
> Aleth, should this patch now also be backed-out like the FX patches?

Yes, thanks.
Status: RESOLVED → REOPENED
Flags: needinfo?(aleth)
Resolution: FIXED → ---
https://hg.mozilla.org/comm-central/rev/b76b3ff1b8303294858f3b8c3fc56f183dc920e8
Backed out changeset 79b497d14c8a (Bug 1261643) to match m-c, package-manifest part. rs=bustage-fix CLOSED TREE
we went with firefox.exe for web content, and kept plugin-container for plugins. I don't think there's any follow up work to do here.
Status: REOPENED → RESOLVED
Closed: 4 years ago3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.