1531583 - [socket process] cherry-pick and rebase non-mc patches

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P1)

Description

[Junior [inactive]]

Attachment: commits

We're suffering from merging m-c to larch project [1]

We push some commits and merge from m-c several times.
The last merge [2] is based on [3]

The goal is to cherry-pick the patches (list in the attachments) to the top of [3] in larch. [2] is a workable version. If the result of rebase is not workable, we might add a patch on top to make it as same as [2].
We might reset the larch remote repo. Just make sure we finish the rebase well.

If the rebase is too complicated, we might make a big patch which is diff of [2] and [3]. However this makes the patch hard to review. We want to land these patch back when we're ready.

[1] https://hg.mozilla.org/projects/larch
[2] https://hg.mozilla.org/projects/larch/pushloghtml?changeset=ce8802224f86
[3] https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=93c4470ee3af

Comment 1

[Honza Bambas (:mayhemer)]

I had one idea that could benefit from hg rebase and its capabilities to simplify merging patches.

Let's say that we:

• pull from mozilla-central to larch, but don't update or merge
• we end up having few blocks (sub branches) on larch, as depicted:

O -- current (or recent) mozilla-central `default` (changeset B)
|
?
|
O -- a merge commit of m-c to larch, joining the larch specific patches and m-c
|\
| \
|  O -- larch specific patch2
|  |
|  |
|  O -- larch specific patch1
| /
|/
O -- a base (parent) changeset which can be found also on m-c
|
|
V

If there are more blocks (sub branches) like these, find the oldest one. Then, I think that if you update to patch2 (or in general - the top-most patch in the sub-branch) and do hg rebase -d default, or -d <hash of B> to be sure you do the right thing, it will move the patches patch1 and patch2 (or in general - the whole sub-branch) on top of the current m-c default changeset (B) and rebase them (offering the 3-diff tool to merge conflicts it can't resolve automatically). It should then look like this:

O -- patch2, changeset B2
|
|
O -- patch1
|
|
O -- mozilla-central common default, changeset B
|
|
O
|
?  ... other sub branches specific to larch...
|
V

Then you repeat again for another oldest sub-branch, but this time you will obviously hg rebase -d <hash of B2>.

And so on.

It would be good to do this on a separate (new) clone of larch in case something goes wrong, to prevent destruction of your development tree.

Comment 2

[Kim Moir [:kmoir] ET]

I don't think this bug belongs in the hg developer services bucket. Our team initializes project repos for teams but it looks like the requirements on this bug are for a better merging strategy for your team. Is there an action here you would like us to take?

Comment 3

[Junior [inactive]]

(In reply to Kim Moir [:kmoir] ET from comment #2)

I don't think this bug belongs in the hg developer services bucket. Our team initializes project repos for teams but it looks like the requirements on this bug are for a better merging strategy for your team. Is there an action here you would like us to take?

Right, it seems pointless to reset the repo.

Comment 4

[Junior [inactive]]

Let's move to Core::Networking

Comment 5

[Junior [inactive]]

Attachment: P01: Bug 1485353 - P1 create HttpTransactionParent/Child to pretend IPC

Comment 6

[Junior [inactive]]

Attachment: P02 Bug 1485353 - P2 disable passing objects which is used by both processes

Comment 7

[Junior [inactive]]

Attachment: P03 Bug 1485353 - P3: Create PHttpTransacation.ipdl

Comment 8

[Junior [inactive]]

Attachment: P04 Bug 1485353 - P4 facilitate opt-out remote nsHttpTransaction

Comment 9

[Junior [inactive]]

Attachment: P05 Bug 1496464 - Cancel the transaction if OnStartRequest fails

As comment 1 said, when OnStart fails, we must not call OnData and propagate
the error to OnStop.

Please note this patch might not cover the whole "stream listener contract".

netwerk/protocol/http/HttpTransactionParent.cpp | 32 ++++++++++++++++++-------
1 file changed, 24 insertions(+), 8 deletions(-)

Comment 10

[Junior [inactive]]

Attachment: P06 Bug 1509406 - Add prefs to turn on the socket porcess and to perform all networking access on the socket process. r=valentin

Comment 11

[Junior [inactive]]

Attachment: P07 Bug 1485355 - implement IPC handles of nsHttpTransaction

(a) able to dispatch Reschedule/Cancel/UpdateClassOfService to socket process
(b) able to send priority to socket priority during init
(c) encapsulate the pump init logic under nsAHttpTransactionShell

Comment 12

[Junior [inactive]]

Attachment: P08 Bug 1520657 - dispatching open-channel using CallOrWaitForSocketProcess

Comment 13

[Junior [inactive]]

Attachment: P09 Bug 1520830 - Enable necessary xpcom components, r=froydnj,dragana

Comment 14

[Junior [inactive]]

Attachment: P10 Pass timings, SetDNSWasRefresh, request context, proxy info and cos to socket process

Bug 1497236 - serialize timings for socket process

Bug 1497238 - move SetDNSWasRefresh to nsAHttpTransactionShell

Bug 1497235 - P1: Pass request context id to HttpTransactionChild, r=JuniorHsu

Bug 1497235 - P2: Pass ProxyInfo to socket process, r=JuniorHsu

Bug 1497235 - P3: Pass classOfService to socket process, r=dragana

Simply pass classOfService to socket process via Init() message.

Comment 15

[Junior [inactive]]

Attachment: P11 Bug 1510691 - [socket-proc] Send OnDataAvailable directly to the content process r=dragana!

Comment 16

[Junior [inactive]]

(In reply to Junior [:junior] from comment #14)

Created attachment 9049366 [details]
P10 Pass timings, SetDNSWasRefresh, request context, proxy info and cos to socket process

Bug 1497236 - serialize timings for socket process

Bug 1497238 - move SetDNSWasRefresh to nsAHttpTransactionShell

Bug 1497235 - P1: Pass request context id to HttpTransactionChild, r=JuniorHsu

Bug 1497235 - P2: Pass ProxyInfo to socket process, r=JuniorHsu

Bug 1497235 - P3: Pass classOfService to socket process, r=dragana

Simply pass classOfService to socket process via Init() message.

Also fold Bug 1521817 into this patch

Comment 17

[Junior [inactive]]

Finish rebasing scaffold. Ask some reviews (P1, P3, P11) for the heavy-conflicted patches to make sure I'm not just going wonky-eyed

Comment 18

[Junior [inactive]]

Attachment: P12 Bug 1497237 - Pass response trailers via SendOnStopRequest, r=valentin

Comment 19

[Junior [inactive]]

Attachment: P13 Bug 1509822 - Stop using HTTP connection manager on the parent process

Bug 1509822 - P1: Decouple nsHttpConnectionMgr and AltSvcCache, r=dragana

Bug 1509822 - P2: Send observer notifications to socket process, r=dragana

Bug 1509822 - P3: Get HttpData from socket process, r=dragana

Bug 1509822 - P4: Send IPC message for DoShiftReloadConnectionCleanup, r=dragana

Bug 1509822 - P5: Not create connection manager in parent process, r=dragana

Comment 20

[Junior [inactive]]

Attachment: P14 enable socket process by default

Comment 21

[Junior [inactive]]

Finish rebasing with some warnings (request Http before socket process ready) and small crashes in Preferences.cpp

Comment 22

[Junior [inactive]]

Attachment: big_diff_patch

Want to put some basic hg trick here:

My final workflow is pretty simple: directly cherry-pick those patches we want

1. hg pull central and hg up ce8802224f86 which is the same parent with our last merge commit.
2. Look at each pushlog, compare to bug 1513057 and have a list (attachment 9047548 [details]) to rebase in order
3. For each patch: do hg graft <changeset>, solve conflict (usually ref from [2]), make it build and run a simple test
4. Put these patches to phab (no worry for the author information. They are kept in local and I'll push them handy)

hg graft and even hg rebase take care of the clang-format except the conflict, which is good.
Have a quick check to avoid this false modification:

LOG(
    ("some logging"));

Here's reason for not doing other ways:
a. to make a big diff patch from [2] and [3]
Attach is the diff file, resulted from hg up larch && hg merge --tool=internal:merge-other ce8802224f86
Sometimes it fails during the merge, we can use this to continue hg resolve -t internal:other --all

The patch consists of lots of format change, and some inconsistent changes.
More important, one patch is hard to review when we want to land back to central.

Btw, making the patch takes >4 hours in my i7 desktop.

b. hg rebase patches to the parent of next patch instead of hg graft to the top
First of all, larch and central are from remote, so patches from the larch is not rebase-able.
We need to hg phase -r <changeset> -d --force to force it to draft phase.
It will ruin the local tree for a while (which I don't like it)
Once we finish, we can hg pull again to make it back to remote phase.

Here's some analysis:
There are four merges after Honza's final rebase pushlog
Merge brings conflict, and our patch also changes after conflict resolved.
And I also fold those build-break patches into the corresponding patch.
Hence, rebase could not apply clearly afterwards, which should be true since I still solving conflict in the last patches.

We might use hg phase and hg rebase trick if central is not changing a lot.
However, bug 1522579 carries lots of conflict for next rebase, though.

Comment 23

[Junior [inactive]]

However, bug 1522579 carries lots of conflict for next rebase, though.

pContentBridge has nothing to do with us. My bad.

Comment 24

[Junior [inactive]]

https://hg.mozilla.org/projects/larch/pushloghtml?changeset=f8517b35f448