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

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Jonathan Griffin (:jgriffin)]

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.

Comment 1

[Jonathan Griffin (:jgriffin)]

Attachment: mirror changes from m-c

The PRAGMA change resolves a conflict between github and m-c in favor of m-c.

Comment 2

[Jeff Hammel]

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 3

[Andrew Halberstadt [:ahal]]

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.

Comment 4

[Jonathan Griffin (:jgriffin)]

https://github.com/mozilla/mozbase/commit/260721f9ffcdd845b7a293f3d8132270395b7a9e

Comment 5

[Jonathan Griffin (:jgriffin)]

(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?

Comment 6

[Jonathan Griffin (:jgriffin)]

This commit broke a mozprofile test:

https://travis-ci.org/mozilla/mozbase/builds/6224698

Comment 7

[Jonathan Griffin (:jgriffin)]

Attachment: test fix

According to https://bugzilla.mozilla.org/show_bug.cgi?id=784875#c1, this should have been PRAGMA user_version rather than PRAGMA schema_version.

Comment 8

[Jonathan Griffin (:jgriffin)]

https://github.com/mozilla/mozbase/commit/463df5f0844a177ea7ae2eb5e302e286b9172aa2

Comment 9

[Jeff Hammel]

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 ...

Comment 10

[Jonathan Griffin (:jgriffin)]

Attachment: Add backwards compatibility

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).

Comment 11

[Jeff Hammel]

Comment on attachment 736056 [details] [diff] [review]
Add backwards compatibility

lgtm, i think :) thanks!

Comment 12

[Jeff Hammel]

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.

Comment 13

[Jeff Hammel]

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

Comment 14

[Jeff Hammel]

Attachment: fix backwards compatability, v2

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).

Comment 15

[Jonathan Griffin (:jgriffin)]

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 16

[Andrew Halberstadt [:ahal]]

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.

Comment 17

[Jonathan Griffin (:jgriffin)]

Attachment: fix backwards compatibility v3

What I mean is that we should fix the test, not the way we're constructing the profile, see attached.

Comment 18

[Jeff Hammel]

Comment on attachment 737975 [details] [diff] [review]
fix backwards compatibility  v3

Thanks, Jonathan!

Comment 19

[Jonathan Griffin (:jgriffin)]

https://github.com/mozilla/mozbase/commit/335f0fb11f003019a20092f50e0403642bf27685

Comment 20

[Jonathan Griffin (:jgriffin)]

Since jeff is doing the mirror as part of bug 860091, I'll close this.