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

make Node.toString stack safe #677

Merged
merged 2 commits into from
Jun 22, 2023
Merged

make Node.toString stack safe #677

merged 2 commits into from
Jun 22, 2023

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Jun 21, 2023

No description provided.

@lrytz lrytz requested a review from dubinsky June 21, 2023 13:01
Copy link
Contributor

@dubinsky dubinsky left a comment

Choose a reason for hiding this comment

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

Wow! LGTM.

@dubinsky
Copy link
Contributor

@lrytz are you going to merge this or should I do it? Thanks!

@lrytz
Copy link
Member Author

lrytz commented Jun 22, 2023

@dubinsky feel free to go ahead. I might backport it to 1.x. Do you have any releases (1.x / 2.x) planned?

@dubinsky
Copy link
Contributor

@dubinsky feel free to go ahead. I might backport it to 1.x. Do you have any releases (1.x / 2.x) planned?

I have one more change planned before the 2.2 release.
No 1.x releases are planned that I know of - @SethTisue?

@SethTisue
Copy link
Member

SethTisue commented Jun 22, 2023

No 1.x releases are planned that I know of - @SethTisue?

The last 1.x release we did was in 2020. I see in the history on the 1.x branch that it has #418 where Lukas back ported the sbt-ci-release based publishing stuff, so there's a good chance we could publish there without too much extra trouble, but it would involve some extra trouble, for sure — for example, that branch still uses Travis-CI and not GitHub Actions, so that change ought to be backported.

I can see one could argue it either way, but my own take on this stack safety issue that is that we shouldn't feel obligated to do another 1.x release on account of it. It's not that uncommon or unexpected for a library to overflow the stack when processing deeply nested input.

Note that since 2.12.17, Scala 2.12 ships with scala-xml 2.x.

@dubinsky dubinsky merged commit 7aebf85 into scala:main Jun 22, 2023
srowen pushed a commit to apache/spark that referenced this pull request Jul 27, 2023
### What changes were proposed in this pull request?
This pr aims to upgrade `scala-xml` from 2.1.0 to 2.2.0.

### Why are the changes needed?
The new version bring some bug fix like:
- scala/scala-xml#651
- scala/scala-xml#677

The full release notes as follows:
- https://github.com/scala/scala-xml/releases/tag/v2.2.0

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
- Pass GitHub Actions
- Checked Scala 2.13, all Scala test passed: https://github.com/LuciferYang/spark/runs/15278359785

Closes #42119 from LuciferYang/scala-xml-220.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
ragnarok56 pushed a commit to ragnarok56/spark that referenced this pull request Mar 2, 2024
### What changes were proposed in this pull request?
This pr aims to upgrade `scala-xml` from 2.1.0 to 2.2.0.

### Why are the changes needed?
The new version bring some bug fix like:
- scala/scala-xml#651
- scala/scala-xml#677

The full release notes as follows:
- https://github.com/scala/scala-xml/releases/tag/v2.2.0

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
- Pass GitHub Actions
- Checked Scala 2.13, all Scala test passed: https://github.com/LuciferYang/spark/runs/15278359785

Closes apache#42119 from LuciferYang/scala-xml-220.

Authored-by: yangjie01 <yangjie01@baidu.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
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.

3 participants