Skip to content

Bumping vendored uriparser - #51123

Draft
imtherealnaska wants to merge 1 commit into
apache:mainfrom
imtherealnaska:issue-51007
Draft

Bumping vendored uriparser#51123
imtherealnaska wants to merge 1 commit into
apache:mainfrom
imtherealnaska:issue-51007

Conversation

@imtherealnaska

Copy link
Copy Markdown

Rationale for this change

Bumping up the version of uriparser

What changes are included in this PR?

All files from upstream git tag uriparser-1.0.2 following the process in cpp/src/arrow/vendored/uriparser/README.md

Are these changes tested?

Yes. No new tests were added.

Are there any user-facing changes?

No.

Help needed from reviewers

  1. I am not too sure where else the changes must be propagated. Places like README were clear , but any other place idk .

AI Usage :

Did a preliminary review with claude ,it suggested some licenses.txt has 0.9.3 wordings . Please let me know if that is something you would be interested in .

@github-actionsgithub-actionsBot added the awaiting review Awaiting review label Sep 1, 2026
@github-actions

Copy link
Copy Markdown

Thanks for opening a pull request!

This pull request has been automatically converted to a draft because its title doesn't match Arrow's required format.

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

After updating the title, you can mark the pull request as ready for review.

See also:

@pitrou

Copy link
Copy Markdown
Member

Thanks @imtherealnaska . Actually, this is a lot of additional vendored code and perhaps it would be better to make it a mandatory external dependency... What do you think @rok@kou ?

@rok

rok commented Sep 2, 2026

Copy link
Copy Markdown
Member

Thanks for opening this @imtherealnaska !
I don't have a strong opinion. Do we need the update?

@pitrou

Copy link
Copy Markdown
Member

uriparser has had security fixes, so yes.

@pitrou

Copy link
Copy Markdown
Member

Sorry @rok , I meant to ping @raulcd :)

@rok

rok commented Sep 2, 2026

Copy link
Copy Markdown
Member

@pitrou Raul is away for some more time I think.

@pitrou

Copy link
Copy Markdown
Member

@pitrou Raul is away for some more time I think.

Until Monday 7th I think.

@imtherealnaska

Copy link
Copy Markdown
Author

Thanks @imtherealnaska . Actually, this is a lot of additional vendored code and perhaps it would be better to make it a mandatory external dependency... What do you think @rok@kou ?

Yes , but I saw this done some time ago #5865 . If you would like to have an external dependency, I can work on that.

@rok

rok commented Sep 2, 2026

Copy link
Copy Markdown
Member

@imtherealnaska we better wait for @kou to reply so we can decide on the direction. And thanks for being flexible regarding the approach!

@imtherealnaska

Copy link
Copy Markdown
Author

@imtherealnaska we better wait for @kou to reply so we can decide on the direction. And thanks for being flexible regarding the approach!

Sure. Happy to contribute

@kou

kou commented Sep 3, 2026

Copy link
Copy Markdown
Member

Actually, this is a lot of additional vendored code and perhaps it would be better to make it a mandatory external dependency...

+1

Let's do it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@imtherealnaska@pitrou@rok@kou