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-actions github-actions Bot added the awaiting review Awaiting review label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

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

pitrou commented Sep 2, 2026

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

pitrou commented Sep 2, 2026

Copy link
Copy Markdown
Member

uriparser has had security fixes, so yes.

@pitrou

pitrou commented Sep 2, 2026

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

pitrou commented Sep 2, 2026

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 free to 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