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

Fix some problems with the HTTP3 documentation #7842

Closed
wants to merge 1 commit into from
Closed

Fix some problems with the HTTP3 documentation #7842

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 12, 2021

  1. If writing to a system path if the command is not prefixed with sudo it will cause a permission denied error
  2. The patched OpenSSL branch has been updated to openssl-3.0.0+quic to match upstream OpenSSL version.
  3. We should not disable GnuTLS docs.
  4. The autoreconf command should be autoreconf -fi

If you believe my changes are wrong, feel free to request changes.

@ghost ghost changed the title Fix some problems with the documentation Fix some problems with the HTTP3 documentation Oct 12, 2021
@bagder
Copy link
Member

bagder commented Oct 12, 2021

I would like us to mention/recommend the same version ngtcp2 itself documents, so if this is the right version to move to, it would be great if you can get the ngtcp2 people to confirm that by updating their recommendation first.

@bagder
Copy link
Member

bagder commented Oct 12, 2021

Since we encourage the use of an installation prefix for this instruction, I think it implies that the user can actually create files there. I think the addition of sudo on all the lines for make install is a complication that we can skip, especially since you add the note in the bottom about it anyway.

OpenSSL 3.0.0 puts the stuff in lib64/ on 64 bit builds and not just in plain lib/ which I suppose you also need to update? (and it makes things more complicated since now the path depends on the arch you build for)

@ghost
Copy link
Author

ghost commented Oct 12, 2021

Since we encourage the use of an installation prefix for this instruction, I think it implies that the user can actually create files there. I think the addition of sudo on all the lines for make install is a complication that we can skip, especially since you add the note in the bottom about it anyway.

OpenSSL 3.0.0 puts the stuff in lib64/ on 64 bit builds and not just in plain lib/ which I suppose you also need to update? (and it makes things more complicated since now the path depends on the arch you build for)

Thank you for reminding me about that
However
lib64
That is for Linux not for all MacOS versions especially on Apple Silicon machines.

I will edit it when I have time. I have to go to university for courses.

@bagder
Copy link
Member

bagder commented Oct 14, 2021

lib64. That is for Linux not for all MacOS versions especially on Apple Silicon machines

My point: the instruction as it reads right now might fail on Linux x86-64 machines, which are quite commonly used!

@ghost

This comment has been minimized.

docs/HTTP3.md Show resolved Hide resolved
1. If writing to a system path if the command is not prefixed with `sudo` it will cause a permission denied error
2. The patched OpenSSL branch has been updated to `openssl-3.0.0+quic` to match upstream OpenSSL version.
3. We should not disable GnuTLS docs.

If you believe my changes are wrong, feel free to request changes.

Updated some commands about `make install`

Make the `sudo` optional because one does not need to prepend `make install` with `sudo` all the time.

http2: make getsock not wait for write if there's no remote window

While uploading, check for remote window availability in the getsock
function so that we don't wait for a writable socket if no data can be
sent.

Reported-by: Steini2000 on github
Fixes #7821
Closes #7839

runtests: split out ignored tests

Report ignore tests separately from the actual fails.

Don't exit non-zero if test servers couldn't get killed.

Assisted-by: Jay Satiro

Fixes #7818
Closes #7841

tests: disable test 2043

It uses revoked.badssl.com which now is expired and therefor this now
permafails. We should not use external sites for tests, this test should
be converted to use our own infra.

Closes #7845

curl: correct grammar in generated libcurl code

Closes #7802

Added a note about OpenSSL > 3.0.0

Fixed the wording about the lib64 folder
@ghost ghost requested a review from bagder October 22, 2021 23:40
@bagder bagder closed this in c2e804c Oct 25, 2021
@bagder
Copy link
Member

bagder commented Oct 25, 2021

Thanks!

@ghost ghost deleted the patch-1 branch November 28, 2021 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants