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.

Sign up for GitHub

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

Jump to bottom

[curl] Update to 7.77.0 #16126

Closed
wants to merge 5 commits into from
Closed

[curl] Update to 7.77.0 #16126

wants to merge 5 commits into from

Conversation

past-due
Copy link
Contributor

@past-due past-due commented Feb 8, 2021 •
edited
Loading

Update curl port to 7.75.0: https://curl.se/changes.html

TODO:

  • Create new version of 0012-fix-dependency-idn2.patch, if needed. (Possibly incorporating the find_library tweaks from the original? But taking advantage of the new USE_LIBIDN2 built-in curl CMake option.)

@PhoebeHui PhoebeHui assigned PhoebeHui and JonLiu1993 and unassigned PhoebeHui Feb 9, 2021
JonLiu1993
JonLiu1993 reviewed Feb 9, 2021
View reviewed changes
ports/curl/0012-fix-dependency-idn2.patch
@@ -1,51 +0,0 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
Copy link
Member

Choose a reason for hiding this comment

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

Why delete this patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As of curl 7.75.0, curl now provides its own USE_LIBIDN2 CMake option. An adapted patch may still be needed to handle the find_library tweaks - have not had a full chance to dig into this.

Copy link
Contributor

Choose a reason for hiding this comment

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

idn2 needs iconv, but curl doesn't link this when testing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll take this to #18971. I even had a look at idn2 recently, for mingw.

@JonLiu1993
Copy link
Member

@past-due ,Can you handle these conflicts?

@JonLiu1993 JonLiu1993 added category:port-update The issue is with a library, which is requesting update new revision requires:author-response labels Feb 9, 2021
@past-due past-due marked this pull request as draft February 9, 2021 14:48
@JonLiu1993 JonLiu1993 marked this pull request as ready for review February 10, 2021 06:20
@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Feb 10, 2021
@ras0219-msft ras0219-msft marked this pull request as draft February 11, 2021 08:04
@ras0219-msft
Copy link
Contributor

@past-due Let us know when you're ready for review/merge by unmarking this as a draft :)

@JonLiu1993 please do not promote other people's PRs from draft to "ready for review"

@JonLiu1993
Copy link
Member

@past-due ,Can this pr be reviewed now?

@past-due
Copy link
Contributor Author

past-due commented Apr 1, 2021 •
edited
Loading

@past-due ,Can this pr be reviewed now?

@JonLiu1993 I apologize for the delay - I have not had a chance to address or investigate the "TODO" listed in the first comment. If that is not an issue for anyone, I have no issues with it being reviewed, but at this point I'm uncertain of the impact.

EDIT: Testing of the idn2 feature of the port is what's needed. I'll mark this as "ready for review", but I have not had a thorough chance to investigate the impact to that port feature. Please feel free to push whatever changes are needed.

@past-due past-due marked this pull request as ready for review April 1, 2021 23:58
@JonLiu1993 JonLiu1993 added requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function and removed info:reviewed Pull Request changes follow basic guidelines labels Apr 2, 2021
@JonLiu1993
Copy link
Member

@past-due ,Could you handle these conflicting files?

@dg0yt
Copy link
Contributor

dg0yt commented May 24, 2021

FTR, upstream curl is at 7.76.1, with two CVEs fixed since 7.75.0

@JonLiu1993 JonLiu1993 added requires:author-response and removed requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function labels May 24, 2021
@JonLiu1993
Copy link
Member

@past-due ,Can you please resolve the conflicts against master?

@past-due
Copy link
Contributor Author

@JonLiu1993 I have rebased and resolved conflicts with the master branch.

curl is now up to 7.77, but there appear to be more patches that need fixing, and I'm not sure when I'll have a chance to address.

Also, as noted above:

Testing of the idn2 feature of the port is what's needed. I'll mark this as "ready for review", but I have not had a thorough chance to investigate the impact to that port feature. Please feel free to push whatever changes are needed.

@past-due past-due marked this pull request as draft May 27, 2021 16:02
@past-due
Copy link
Contributor Author

past-due commented May 27, 2021 •
edited
Loading

I pushed an update to cURL 7.77.0, and some (attempts) at adjusting patches. Waiting for CI.

Did not run x-add-version yet.

@past-due past-due changed the title [curl] Update to 7.75.0 [curl] Update to 7.77.0 May 27, 2021
@dg0yt
Copy link
Contributor

dg0yt commented May 28, 2021

gdal failing on osx? Well, that is part of gdall being broken on Linux (#9068), and I have another curl patch pending for osx which is related, #17790. It is reviewed, but got a merge conflict before being merged... I will take a look if I can align this with your change.

@dg0yt
Copy link
Contributor

dg0yt commented May 29, 2021

I will refresh #17790 independent of this PR, and that start with this PR separately. There a few more issues with port curl at the moment, first of all a mismatch of non-htp/HTTP_ONLY.

@dg0yt dg0yt mentioned this pull request Jul 16, 2021
Merged
8 tasks
@JonLiu1993
Copy link
Member

@past-due ,Can you please resolve the conflicts against master?

@dg0yt
Copy link
Contributor

dg0yt commented Jul 16, 2021

@past-due ,Can you please resolve the conflicts against master?

Actually I already did this. I just wait for the current CI run of #18971 to complete before pushing the cherry-picked commits. And I have pending work with regard to libidn2.

@PhoebeHui
Copy link
Contributor

Closing in favor of #18971.

@PhoebeHui PhoebeHui closed this Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dg0yt dg0yt dg0yt left review comments

@JonLiu1993 JonLiu1993 JonLiu1993 approved these changes

Assignees

@JonLiu1993 JonLiu1993

Labels
category:port-update The issue is with a library, which is requesting update new revision
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[curl] update to <curl-7_75_0>
5 participants
@past-due @JonLiu1993 @ras0219-msft @dg0yt @PhoebeHui