Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accumulate gdbserver output before checking for target port #300

Merged

Conversation

jonahgraham
Copy link
Contributor

Because the needed output from gdbserver may not arrive as a single 'data' event on stdout we need to accumulate the output before passing it to the regex checker.

Fixes #299

@jonahgraham jonahgraham marked this pull request as draft September 25, 2023 16:25
this.sendEvent(
new OutputEvent(
`checking '${data}' for /${regexStr}/`,
'server'
)
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a failure with the earlier version of this change, so I added this extra output event to understand what is happening and now the test passes - still a bit of experimentation to make sure that this accumulation is implemented correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found the issue - the default regex is Listening on port ([0-9]+) which means when the input (e.g. Listening on port 12345) comes in a character at a time, the regex matches Listening on port 1 and thinks the port is 1.

@jonahgraham jonahgraham force-pushed the gdb-windows-stability branch from 4102160 to 9ead66f Compare October 10, 2023 18:29
Because the needed output from gdbserver may not arrive as a single
'data' event on stdout we need to accumulate the output before
passing it to the regex checker.

Fixes eclipse-cdt-cloud#299
@jonahgraham jonahgraham force-pushed the gdb-windows-stability branch from 9ead66f to 3e91a67 Compare October 10, 2023 18:54
@jonahgraham jonahgraham marked this pull request as ready for review October 10, 2023 18:55
@jonahgraham
Copy link
Contributor Author

This change updates the regex from Listening on port ([0-9]+) to Listening on port ([0-9]+)\r?\n, all the docs for default value of serverPortRegExp say "defaults to matching a string like 'Listening on port 41551'":

// defaults to matching a string like 'Listening on port 41551' which is what gdbserver provides

which means I don't think it should be a problem to change the default implementation here.

@jonahgraham jonahgraham merged commit 86c1e77 into eclipse-cdt-cloud:main Oct 10, 2023
@jonahgraham jonahgraham deleted the gdb-windows-stability branch October 10, 2023 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GDB version update on GitHub Actions on Windows causes test failures
1 participant