Closed Bug 1336501 Opened 3 years ago Closed 3 years ago

Sync shield-recipe-client from GitHub (commit ad0f45e)

Categories

(Firefox :: Normandy Client, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: mythmon, Assigned: mythmon)

References

Details

Attachments

(1 file)

PRs since last sync:

ad0f45e Merge pull request #474 from mythmon/addon-distribution-id - recipe-client-addon: Add distribution ID to filter expressions and driver
e29e03c Merge pull request #458 from mythmon/fix-bad-import - recipe-client-addon: Fix missing import of LogManager in bootstrap::shutdown
cada912 Merge pull request #453 from mythmon/addon-sync-counts - Add sync device counts to driver.
Assignee: nobody → mcooper
Comment on attachment 8833358 [details]
Bug 1336501 - Sync shield-recipe-client from GitHub (ad0f45e)

Approval Request Comment
[Feature/Bug causing the regression]: Improvements to Shield self-repair driver
[User impact if declined]: Incomplete self-repair driver
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Small features needed for proper functioning of self-repair.
[String changes made/needed]: None
Attachment #8833358 - Flags: approval-mozilla-aurora?
Comment on attachment 8833358 [details]
Bug 1336501 - Sync shield-recipe-client from GitHub (ad0f45e)

https://reviewboard.mozilla.org/r/109606/#review110866

r=me on the assumption this does what it says on the tin.
Attachment #8833358 - Flags: review?(gijskruitbosch+bugs) → review+
For the uplift, has the previous sync of the add-on been uplifted, too? If not, that probably needs its own bug and/or we should provide a branch patch that includes the other changes.
Flags: needinfo?(mcooper)
I believe that the previous sync happened before the current aurora branch was forked, and the one bug that has affected this code has also been uplifted.

I also ran |hg diff -r aurora:central -- browser/extensions/shield-recipe-client|, which reported no differences. Is that a valid way to verify this?
Flags: needinfo?(mcooper)
(In reply to Mike Cooper [:mythmon] from comment #6)
> I believe that the previous sync happened before the current aurora branch
> was forked, and the one bug that has affected this code has also been
> uplifted.
> 
> I also ran |hg diff -r aurora:central --
> browser/extensions/shield-recipe-client|, which reported no differences. Is
> that a valid way to verify this?

What do the 2 --'s do there? RyanVM recently referenced https://hg.mozilla.org/mozilla-central/rev/acd9236e8f36 , and if I run:

hg diff -r aurora:central browser/extensions/shield-recipe-client

I see the diff that seems to correspond to that change, which wasn't filed as a bug and, as far as I can tell, not uplifted to 53. I could be missing something, though...?
Flags: needinfo?(mcooper)
The --'s there are a git-ism: they explicitly separate file names from non-filenames. I think they do the same thing in mercurial, but they aren't necessary either way, and don't affect the command.

My mercurial isn't very good, but I ran through this again. I made sure to have up-to-date versions of aurora and central that match what I see on hg.mozilla.org (changeset 514a5268030f and e677ba018b22 respectively). I now see one difference between the two, and it is a change that was included in this patch (Adding a missing Cu to CleanupManager.jsm)

I'm not sure that acd9236e8f36 is included in aurora; I haven't found out how to do that in hg yet. But the diff seems safe to me. I think this change was added to central before aurora was branched off. If you are still getting different results, can you tell me what changeset ids you have for aurora and central? Maybe I've missed something.
Flags: needinfo?(mcooper)
(In reply to Mike Cooper [:mythmon] from comment #8)
> The --'s there are a git-ism: they explicitly separate file names from
> non-filenames. I think they do the same thing in mercurial, but they aren't
> necessary either way, and don't affect the command.
> 
> My mercurial isn't very good, but I ran through this again. I made sure to
> have up-to-date versions of aurora and central that match what I see on
> hg.mozilla.org (changeset 514a5268030f and e677ba018b22 respectively). I now
> see one difference between the two, and it is a change that was included in
> this patch (Adding a missing Cu to CleanupManager.jsm)
> 
> I'm not sure that acd9236e8f36 is included in aurora; I haven't found out
> how to do that in hg yet. But the diff seems safe to me. I think this change
> was added to central before aurora was branched off. If you are still
> getting different results, can you tell me what changeset ids you have for
> aurora and central? Maybe I've missed something.

I had old csets. Sorry, I should have checked.

It looks like Ryan landed https://hg.mozilla.org/releases/mozilla-aurora/rev/c02020dd7e6e for bug 1325409 (rather than landing only the specific aurora patch on that bug) and thereby pulled in all the other changes. Full steam ahead, then!
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aa6dfc22160f
Sync shield-recipe-client from GitHub (ad0f45e) r=Gijs
(In reply to :Gijs from comment #9)
> It looks like Ryan landed
> https://hg.mozilla.org/releases/mozilla-aurora/rev/c02020dd7e6e for bug
> 1325409 (rather than landing only the specific aurora patch on that bug) and
> thereby pulled in all the other changes. Full steam ahead, then!

Yeah, there were conflicts on the uplift due the aforementioned upstream update. I had figured that the branch patch would end up being that anyway so I went ahead and folded it in. Sorry if it caused any confusion.
Duplicate of this bug: 1337926
https://hg.mozilla.org/mozilla-central/rev/aa6dfc22160f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Modified from Uplift Dashboard.
Comment on attachment 8833358 [details]
Bug 1336501 - Sync shield-recipe-client from GitHub (ad0f45e)

Improvements to Shield sound good to me.
Attachment #8833358 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Blocks: 1349348
Product: Shield → Firefox
You need to log in before you can comment on or make changes to this bug.