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

curl 8.6.0 #157

Merged
merged 8 commits into from
Jan 31, 2024
Merged

curl 8.6.0 #157

merged 8 commits into from
Jan 31, 2024

Conversation

GrahamCampbell
Copy link
Contributor

@GrahamCampbell GrahamCampbell commented Jan 31, 2024

Prior to cURL 8.6.0, configure would silently skip when libpsl was missing, compiling without support for public suffix lists. For Bref v2 at least, I think it's probably a good idea to compile cURL with support for this, so I've added in a build step to compile libpsl.

For Bref v1 (brefphp/bref#1729), I'm not sure it's such a good idea to start making what is arguably a breaking change, so I've added in the configure flag to continue to ignore the fact that libpsl is missing.

@GrahamCampbell GrahamCampbell marked this pull request as ready for review January 31, 2024 10:13
@mnapoli
Copy link
Member

mnapoli commented Jan 31, 2024

The failure says:

configure: error: libpsl was not found

Any idea?

@GrahamCampbell
Copy link
Contributor Author

I've updated the PR description. ;)

@mnapoli
Copy link
Member

mnapoli commented Jan 31, 2024

Got it!

Looking at what psl is, and the fact that the website says it's 50MB, does that mean the layers will grow as well?

@GrahamCampbell
Copy link
Contributor Author

Good question. Let me see...

@GrahamCampbell
Copy link
Contributor Author

No. The image is ~0.3MB larger, only.

@GrahamCampbell
Copy link
Contributor Author

Though possibly we need to manually copy additional files?

@mnapoli
Copy link
Member

mnapoli commented Jan 31, 2024

That might be possible, any way to test that (and add an automated test)?

@GrahamCampbell
Copy link
Contributor Author

Trying to set a cookie with domain .com should fail with PSL enabled.

@GrahamCampbell
Copy link
Contributor Author

I may not have time to do this today, but I don't think getting this curl upgrade out is particularly urgent. 🤷

@GrahamCampbell
Copy link
Contributor Author

I think 0.3MB is directionally correct though, for the size I would expect. Where did the 50MB number come from?

@mnapoli
Copy link
Member

mnapoli commented Jan 31, 2024

Ah nevermind, I looked in the wrong place. I ended up looking at the sources and it seems to be using this list of domains: https://github.com/publicsuffix/list/blob/5db9b65997e3c9230ac4353b01994c2ae9667cb9/public_suffix_list.dat That's 250KB so we should be good, sounds consistent 👍

@mnapoli mnapoli merged commit 267db43 into brefphp:main Jan 31, 2024
5 checks passed
@GrahamCampbell GrahamCampbell deleted the curl-8-6-0 branch January 31, 2024 17:36
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.

None yet

2 participants