Closed Bug 647404 Opened 13 years ago Closed 13 years ago

automation.py: fix extractZip() and installExtension() issues from bug 573263, affecting bug 647394

Categories

(Testing :: General, defect)

defect
Not set
major

Tracking

(firefox5-, status2.0 wontfix)

VERIFIED FIXED
mozilla6
Tracking Status
firefox5 - ---
status2.0 --- wontfix

People

(Reporter: sgautherie, Assigned: sgautherie)

References

()

Details

Attachments

(3 files, 2 obsolete files)

      No description provided.
Assignee: nobody → sgautherie.bz
status2.0: --- → ?
Flags: in-testsuite-
shutil.rmtree(): tell me if you want it in a try+finally.

'/': "unexpected" issue, but unless it's my Windows 2000 etc only...
Attachment #523754 - Flags: review?(jmaher)
Comment on attachment 523761 [details] [diff] [review]
(Bv1) Rewrite installExtension() to copy xpi file without extracting it, Remove not needed anymore extractZip()

This is cleaner, faster and works better:
using --install-extension= with a relative path+filename instead of a filename only does work now ;->
(See bug 647394 for such a case.)
Comment on attachment 523754 [details] [diff] [review]
(Av1) s|os.sep|'/'|, Add shutil.rmtree() call, Fix some code nits
[Checked in: Comment 14]

NB: I'm not sure why this code does extract2temp+copy2dest rather than extract2dest, but this first patch is just to make it work as is.
(In reply to comment #3)
> using --install-extension= with a relative path+filename instead of a filename
> only does work now ;->

Not too sure how it works as is, but let's do it right.
Attachment #523834 - Flags: review?(jmaher)
Blocks: 647414
Comment on attachment 523754 [details] [diff] [review]
(Av1) s|os.sep|'/'|, Add shutil.rmtree() call, Fix some code nits
[Checked in: Comment 14]

thanks for the cleanup!
Attachment #523754 - Flags: review?(jmaher) → review+
Comment on attachment 523761 [details] [diff] [review]
(Bv1) Rewrite installExtension() to copy xpi file without extracting it, Remove not needed anymore extractZip()

>+      # Copy extension tree into its own directory.
>+      shutil.copytree(extensionSource, os.path.join(extnsdir, extensionID))

here we are assuming that extensionSource is a directory.  Can we verify that it exists before copytree?
Attachment #523761 - Flags: review?(jmaher) → review-
Comment on attachment 523834 [details] [diff] [review]
(Cv1) Explicitly support an optional path in --install-extension

>       abspath = self.getFullPath(f)
>-      extensionID = f[:f.rfind(".")]
>+      extensionID = abspath[abspath.rfind(os.sep) + 1 : abspath.rfind(".")]
>       self.automation.installExtension(abspath, profileDir, extensionID)

does os.sep work correctly in msys (our shell for windows)?  


>       abspath = self.getFullPath(f)
>-      extensionID = f[:f.rfind(".")]
>+      extensionID = abspath[abspath.rfind(os.sep) + 1 : abspath.rfind(".")]
>       self.automation.installExtension(abspath, options.profilePath, extensionID)

same as above
(In reply to comment #8)
> does os.sep work correctly in msys (our shell for windows)?  

Yes, that's how I'm testing it: Windows 2000 + msys.
Keywords: checkin-needed
Whiteboard: [c-n: patch Av1]
Attachment #523834 - Flags: review?(jmaher) → review+
Whiteboard: [c-n: patch Av1] → [c-n: patch Av1, Cv1]
Please renom for landing once all of the patches are reviewed.
Keywords: checkin-needed
Whiteboard: [c-n: patch Av1, Cv1]
Attachment #524044 - Flags: review?(jmaher) → review+
Keywords: checkin-needed
Whiteboard: [c-n: patch Av1, Bv2, Cv1]
Keywords: checkin-needed
Cv1, unbloated.


(In reply to comment #5)
> Not too sure how it works as is, but let's do it right.

It works with patch B because extensionID isn't used anymore :->
Attachment #523834 - Attachment is obsolete: true
Attachment #524670 - Flags: review?(jmaher)
(In reply to comment #11)
> Please renom for landing once all of the patches are reviewed.

Still not changing my position here!  ;-)
Whiteboard: [c-n: patch Av1, Bv2, Cv1]
Comment on attachment 523754 [details] [diff] [review]
(Av1) s|os.sep|'/'|, Add shutil.rmtree() call, Fix some code nits
[Checked in: Comment 14]

http://hg.mozilla.org/mozilla-central/rev/66656b3d818f
Attachment #523754 - Attachment description: (Av1) s|os.sep|'/'|, Add shutil.rmtree() call, Fix some code nits → (Av1) s|os.sep|'/'|, Add shutil.rmtree() call, Fix some code nits [Checked in: Comment 14]
Comment on attachment 524044 [details] [diff] [review]
(Bv2) Rewrite installExtension() to copy xpi file without extracting it, Remove now unused extractZip()
[Checked in: Comment 15]

http://hg.mozilla.org/mozilla-central/rev/a6afe12a2896
Attachment #524044 - Attachment description: (Bv2) Rewrite installExtension() to copy xpi file without extracting it, Remove now unused extractZip() → (Bv2) Rewrite installExtension() to copy xpi file without extracting it, Remove now unused extractZip() [Checked in: Comment 15]
Attachment #524670 - Flags: review?(jmaher) → review+
Comment on attachment 524670 [details] [diff] [review]
(Cv2) Explicitly support an optional path in --install-extension
[Checked in: Comment 16]

http://hg.mozilla.org/mozilla-central/rev/3afa19c50a85
Attachment #524670 - Attachment description: (Cv2) Explicitly support an optional path in --install-extension → (Cv2) Explicitly support an optional path in --install-extension [Checked in: Comment 16]
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n to m-2.1: patch Av1, Bv2, Cv2]
Target Milestone: --- → mozilla6
Whiteboard: [c-n to m-2.1: patch Av1, Bv2, Cv2] → [c-n to m-2.1 and m-2.0: patch Av1, Bv2, Cv2]
Any reason why these patches are needed on mozilla-2.1?  IINM the only product being released from that component is Fennec.
(In reply to comment #17)

> Any reason why these patches are needed on mozilla-2.1?

No other reason (that I know of) than m-2.1 is in-between m-c and m-2.0, where SeaMonkey 2.1 needs this.

> IINM the only product being released from that component is Fennec.

I was told so once.
Ftr, I'm still trying to recover from what happened with m-2.0, let alone trying to figure out m-2.1 story :-|
So, here's what's happening with the 2.1 repo.  Mobile guys will release dot-released to Fennec 4 off of that repo.  As far as I know, no other project is going to use it.  Hence, I don't think we need to land this on 2.1.
(In reply to comment #19)
> Hence, I don't think we need to land this on 2.1.

Let me say in other words what I'm looking for:
I understand about m-c and m-2.0 (and m-1.9.*).
I'm still very unclear about all that is happening "in-between" (or aside of!?) m-c and m-2.0, as in m-2.1, m-aurora, etc, or the various branch code names, as in "birsh", "cedar", etc, or where FF5 actually lives now.
My goal is to have my patches checked in to m-2.0 (so SeaMonkey 2.1 can have it), and to any repository that future SeaMonkey versions might be released off.
Hence, this should be checked in to all "in-between" repositories that apply (if any).

NB: I'm talking about SeaMonkey, but this bug may be useful to other applications, like testing Firefox+Firebug for example (if that's still wanted).
Whiteboard: [c-n to m-2.1 and m-2.0: patch Av1, Bv2, Cv2] → [c-n (down) to m-2.0: patch Av1, Bv2, Cv2]
(In reply to comment #20)
> My goal is to have my patches checked in to m-2.0 (so SeaMonkey 2.1 can have
> it), and to any repository that future SeaMonkey versions might be released
> off.

Hum, let me try in yet other words:
I was even more surprised by the jump to Gecko versions 5 and 6...
But maybe that's just the point, iiuc.
I checked in to m-c which is now Gecko 6.
I need this in m-2.0 which is Gecko 4.
Then maybe there's just 1 "in-between" repo to care about, the one for Gecko 5 ... which currently is what, m-aurora!?
We don't need this for FF5 and don't believe there are plans to release SM out of FF5....tracking -.

(note it is still an open question if it will be taken on releases/mozilla-2.0)
(we will also be adding the bug process and tree rules there shortly as well)
(In reply to comment #22)
> (note it is still an open question if it will be taken on releases/mozilla-2.0)

Any progress?
This is, for example, blocking bug 647414 which blocks bug 647394...
V.Fixed, per bug 647414 comment 14.
Status: RESOLVED → VERIFIED
(In reply to comment #22)
> We don't need this for FF5 and don't believe there are plans to release SM
> out of FF5....tracking -.

SeaMonkey is (now that we have discussed) planning to jump directly onto the train, and release off Firefox 5...

> (note it is still an open question if it will be taken on
> releases/mozilla-2.0)

As a SeaMonkey Driver, I do not feel taking this on m-2.0 (or even aurora for Firefox 5) is worth the efforts/risk. This is needed for test runs, and nothing else.
Keywords: checkin-needed
Whiteboard: [c-n (down) to m-2.0: patch Av1, Bv2, Cv2]
(In reply to comment #23)
> You may find this document helpful:
> <http://mozilla.github.com/process-releases/draft/development_specifics/>

Yeah, I had already tried to read this draft once or twice.

Now that things have settled a little, iiuc, after having had an overly long FF4 cycle (including tree "freeze"), we're now just having a release-and-forget mozilla2 branch, a Fennec only mozilla2.1 branch and a very quick frozen-open-frozen mozilla5 branch.
This explains why I had a hard time figuring out where/when to land non Firefox patches: the answer seems to mostly be to jump from 2.0pre to 6.0pre :-<
So be it.
You need to log in before you can comment on or make changes to this bug.