[quality] Switch back to node-pty.

[kittaakos]

Currently, we are using a fork of the node-pty lib: https://github.com/theia-ide/node-pty. We consume the forked node-pty in @theia/process.

1. We are already behind a few commits.
2. [electron][windows] Cannot start the electron application #4526 might be related.

[kittaakos]

2. #4526 might be related.

Fixed via #4527

[kittaakos]

We are almost 200 commits behind the origin. Is there a chance to switch back to the original node-pty dependency and contribute back whatever we have modified?

CC: @marcdumais-work @akosyakov

[marcdumais-work]

@kittaakos yes, that would be ideal - there is no technical reason not to do so. However we (at Ericsson) have been internally denied permission sign the Microsoft CLA, and without it we can't contribute to Microsoft open-source projects.

That being said, we can uplift our fork when we think it's worthwhile and/or periodically, so that our fork does not fall too far behind.

Can I ask if you know is a specific reason to uplift at this time, other than not drifting too far from upstream?

[kittaakos]

Thanks for the explanation.

is a specific reason to uplift at this time

No, there isn't any specific reason. We are hitting some native build issues on Windows with VS19 installed on the environment for a downstream project, and I've just accidentally stumbled upon this issue.

other than not drifting too far from upstream

I asked exactly because of this.

[simark]

We are almost 200 commits behind the origin.

Wow, there has been a lot of changes in node-pty recently!

[marcdumais-work]

Wow, there has been a lot of changes in node-pty recently!

Indeed - I was hoping that they may have fixed the issue for which we forked-over, but it's still open.

[kittaakos]

Does anyone know why are we 86 commits ahead (and 236 behind) the fork? 😕

[marcdumais-work]

Hi @kittaakos,

We uplifted our node-pty fork in June but did not publish since there was no concrete reason to do so. I just went ahead and published version 0.8.1-theia.5. Do you want to try if it helps with your issue and get back to us whether we should uplift to the most recent upstream master to fix your issue?

[kittaakos]

if it helps with your issue

I do not have an actual issue 😊; whenever I build the electron example, I can see rebuilding @theia/node-pty. I was wondering whether we can push back the contributions to MS and continue with the upstream instead of the fork. I am pretty sure, sooner or later, we won't be able to track what was the customization and why we did it. The main reason I updated this thread is the 86 commits ahead. Any ideas on that?

[marcdumais-work]

I was wondering whether we can push back the contributions to MS and continue with the upstream instead of the fork.

I agree this would be the ideal way to proceed. However we have not been allowed, so far, to sign the Microsoft contributor agreement; it seems my employer doesn't like some provision(s) of that agreement, and Microsoft does not accept contributions without this document signed.

We have recently requested that this be reconsidered - if the answer is positive, we'll attempt to submit the changes upstream.

I am pretty sure, sooner or later, we won't be able to track what was the customization and why we did it.

You're probably right. So it's documented at least here: digging through our branch, the customisation we did is comprised of the following 3 commits, that should be taken as a whole (i.e. there is some back-tracking happening). From oldest to newest:
theia-ide/node-pty@f58e6ee
theia-ide/node-pty@186e39c
theia-ide/node-pty@89e077b

Other commits would not be relevant to upstream: e.g. they change the package name to our fork's, set our own version, ....

The main reason I updated this thread is the 86 commits ahead. Any ideas on that?

It does look weird to be both ahead and behind upstream :) I do not know for sure, but guess is that it could mean that we diverged from upstream 86 commits back, and do not have their latest 236 commits.

[kittaakos]

theia-ide/node-pty@f58e6ee
theia-ide/node-pty@186e39c
theia-ide/node-pty@89e077b

👍 Thank you for collecting the effective changeset.

[msujew]

We're using the official node-pty now, see #9936.