Closed Bug 860110 Opened 11 years ago Closed 11 years ago

Mirror in-tree mozprofile changes to github and back again...

Categories

(Testing :: Mozbase, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jgriffin, Unassigned)

References

Details

(Whiteboard: [mozbase])

Attachments

(2 files, 3 obsolete files)

There have been two direct commits to mozprofile in m-c recently:

http://hg.mozilla.org/mozilla-central/rev/961258980236
http://hg.mozilla.org/mozilla-central/rev/4e1840fca4f3

The lack of these in pypi is breaking desktop B2G mochitests (which aren't currently run in TBPL).

We need to mirror these back to mozbase, then versionbump, post to pypi, and re-mirror to m-c, in some order or other.

I think this qualifies as an argument for moving the canonical location of mozbase to m-c, although granted this doesn't happen often.
The PRAGMA change resolves a conflict between github and m-c in favor of m-c.
Attachment #735502 - Flags: review?(ahalberstadt)
Whiteboard: [mozbase]
Summary: Mirror to port in-tree mozprofile changes to github and back again... → Mirror in-tree mozprofile changes to github and back again...
Note also we should bump mozdevice and mirror that and test.py to m-c: https://bugzilla.mozilla.org/show_bug.cgi?id=860091  We may (or may not) want to coordinate with this.

> I think this qualifies as an argument for moving the canonical
> location of mozbase to m-c, although granted this doesn't happen
> often.

If there was enough desire to keep mozbase development separate, we could engineer solutions for here as has been discussed before.
Comment on attachment 735502 [details] [diff] [review]
mirror changes from m-c

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

It hasn't happened much to date mostly because the first major piece of automation to use mozbase is b2g mochitests which only happened <2 weeks ago. This will become more and more frequent as we convert more and more pieces to use mozbase.
Attachment #735502 - Flags: review?(ahalberstadt) → review+
https://github.com/mozilla/mozbase/commit/260721f9ffcdd845b7a293f3d8132270395b7a9e
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to Jeff Hammel [:jhammel] from comment #2)
> Note also we should bump mozdevice and mirror that and test.py to m-c:
> https://bugzilla.mozilla.org/show_bug.cgi?id=860091  We may (or may not)
> want to coordinate with this.
> 
That makes sense; do you want to bump and mirror both at the same time?  Do you want me to file a separate bug for it?
This commit broke a mozprofile test:

https://travis-ci.org/mozilla/mozbase/builds/6224698
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch test fix (obsolete) — Splinter Review
According to https://bugzilla.mozilla.org/show_bug.cgi?id=784875#c1, this should have been PRAGMA user_version rather than PRAGMA schema_version.
Attachment #735868 - Flags: review?(jhammel)
Attachment #735868 - Flags: review?(jhammel) → review+
https://github.com/mozilla/mozbase/commit/463df5f0844a177ea7ae2eb5e302e286b9172aa2
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
We still need the pypi versionbump and mirroring back to m-c maybe with bug 860091.

My question is: is this a backwards breaking change?  That is, will mozprofile, say, 0.7 not work with firefox older than a few days ago?  I don't know, but it seems like?

As long as we're using instances wrt the mozbase in m-c issue, not knowing things like this and having these changes being made to (in this case) mozbase without respect to development practices or notifying the caretakers of the software ...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Add backwards compatibility (obsolete) — Splinter Review
This adds backwards compatibility wrt moz_hosts user_versions.  It has tests, which pass, and it passes the real-world use case as well (b2g desktop mochitests).
Attachment #736056 - Flags: review?(jhammel)
Comment on attachment 736056 [details] [diff] [review]
Add backwards compatibility

lgtm, i think :) thanks!
Attachment #736056 - Flags: review?(jhammel) → review+
Comment on attachment 736056 [details] [diff] [review]
Add backwards compatibility

This causes mozprofile/tests/bug785146.py to fail:

test_remove_directory (test.TestRemoveTree) ... ok

======================================================================
FAIL: test_schema_version (bug785146.PermissionsTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jhammel/mozbase/src/mozbase/mozprofile/tests/bug785146.py", line 52, in test_schema_version
    self.assertEqual(schema_version, 3)
AssertionError: 2 != 3

----------------------------------------------------------------------
Ran 105 tests in 124.613s

FAILED (failures=1)

I don't see anything obvious at a quick glance.  I'll look again when I can (today or tomorrow, if nothing erupts), but will r- for now.
Attachment #736056 - Flags: review+ → review-
Okay, duh:

-           expireTime INTEGER,
-           appId INTEGER,
-           isInBrowserElement INTEGER)""")
+           expireTime INTEGER)""")

We now create the table with the version 2 schema.

+        # when creating a DB we should default to user_version==2
+        cur.execute('PRAGMA user_version')
+        entries = cur.fetchall()
+        self.assertEqual(entries[0][0], 2)

Guessing we'll want to make that 3
Attached patch fix backwards compatability, v2 (obsolete) — Splinter Review
This makes the version=3, not 2, by default, and corrects the permissions.py test appropriately.  All tests pass.

I'm not sure why v=2 was the default.  If we shouldn't take this, let me know.  In any case, it'd be nice to have a record of what is default why.

Giving to :ahal as I believe :jgriffin is busy this week, but feel free to take the review (or you, :wlach, etc).
Attachment #737724 - Flags: review?(ahalberstadt)
The reason we are defaulting to v2 is that the browser will auto-upgrade a v2 db to v3, but it won't do the reverse.  So, if we create a v3 db and try to use that profile with an older browser, it will be a version that the browser doesn't know how to handle, and all the INSERT statements performed by the browser wil fail due to incorrect number of columns.

So, if we want true backwards compatibility, I believe we should create v2 databases rather than v3.
Comment on attachment 737724 [details] [diff] [review]
fix backwards compatability, v2

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

I'm slightly confused what jgriffin's comment means in terms of taking this patch. Though from my point of view, lgtm.
Attachment #737724 - Flags: review?(ahalberstadt) → review+
What I mean is that we should fix the test, not the way we're constructing the profile, see attached.
Attachment #736056 - Attachment is obsolete: true
Attachment #737975 - Flags: review?(jhammel)
Comment on attachment 737975 [details] [diff] [review]
fix backwards compatibility  v3

Thanks, Jonathan!
Attachment #737975 - Flags: review?(jhammel) → review+
Attachment #737724 - Attachment is obsolete: true
Attachment #735868 - Attachment is obsolete: true
Since jeff is doing the mirror as part of bug 860091, I'll close this.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: