Closed Bug 1525654 Opened 7 months ago Closed 7 months ago

[remote-dbg-next] Cannot use build dates to check if remote runtime is too recent

Categories

(DevTools :: about:debugging, enhancement, P1)

enhancement

Tracking

(firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

While porting the compatibility warning for the new about:debugging, I noticed that our current logic to detect if the remote runtime was too recent was flawed:

https://searchfox.org/mozilla-central/rev/e00ea598e52bbb35f8c45abf9c2eade17962bb5e/devtools/shared/client/debugger-client.js#221-276

We are using build dates at the moment, and this is great as long as you compare two runtimes that have the same version. But for different release channels the build dates cannot be compared.

For instance I connected from Firefox Release 65 to a Fennec Nightly 67. So I expected to see the warning. But this Firefox Release was built on Jan 08 2019, while my Fennec Nightly was build on Jan 14 2019 (and we allow 1 week of "too recent" runtimes, but that's not the real issue here). So no warning here.

We should update the logic to check first the version numbers, and then if the version numbers are the same, compare the build dates.

Alex, do you remember the motivation behind using build dates instead of versions when we want to detect runtimes that are too recent? I am tempted to only use versions, and show the "too recent" warning as soon as the major remote version is greater than local one.

I understand build dates can be useful if someone is trying to connect two builds of nightly together. But if that's the only scenario where build dates are useful, I am in favor of dropping it.

Flags: needinfo?(poirot.alex)

(In reply to Julian Descottes [:jdescottes] from comment #1)

Alex, do you remember the motivation behind using build dates instead of versions when we want to detect runtimes that are too recent? I am tempted to only use versions, and show the "too recent" warning as soon as the major remote version is greater than local one.

I understand build dates can be useful if someone is trying to connect two builds of nightly together. But if that's the only scenario where build dates are useful, I am in favor of dropping it.

That's actually a handy check for gecko developers to have.
It is useful when we introduce any breaking change.
But this is clearly super-broken for the case you mentioned!
What about checking for versions and if the versions are the same, fallback on build dates?

Flags: needinfo?(poirot.alex)

What about checking for versions and if the versions are the same, fallback on build dates?

Sure that works for me, I just wanted to see if I could simplify the whole thing. Worth a shot :)

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: -- → P1
Attachment #9042038 - Attachment is obsolete: true
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/23fa2fb048b9
Move version compatibility check to dedicated module and add unit-tests;r=ochameau
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.