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

Add Support TLS in Webternet. #1856

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Conversation

idk1703
Copy link

@idk1703 idk1703 commented Nov 4, 2022

Resolves #1497.

Update config-sm-linux.h.
Update config-sm-mac.h. (mac supported now ?)
Add config-sm-win.h.
Update Webternet AMBuilder.
Update cURL AMBuilder.
Add Mbed TLS AMBuilder.
Add path to Certificate Authority (CA) bundle.
Remove unused func UTIL_Format and UTIL_FormatArgs.
@sapphonie
Copy link
Contributor

image
I'm no alliedmodders developer, but I would consider paring down your merge request a bit...

@idk1703
Copy link
Author

idk1703 commented Nov 4, 2022

изображение Я не разработчик alliedmodders, но я бы подумал о том, чтобы немного сократить ваш запрос на слияние...

I think so too, but I see the full version of cURL in repo.
Need delete all, except include, lib/library and license.

@rumblefrog
Copy link
Contributor

I think a git submodule would be appropriate.

@asherkin
Copy link
Member

asherkin commented Nov 4, 2022

Thanks for working on this!

I'm no alliedmodders developer, but I would consider paring down your merge request a bit...

This is fine, the individual commits are suitably sized.

Update PackageScript: remove copy not exist dir.
Update extension.h: change path ca-bundle.
For replace curl-src and mbedtls-src to submodules.
Move custom files in submodules on build.
@idk1703
Copy link
Author

idk1703 commented Nov 5, 2022

I think this better, for keep clear repo.

@idk1703 idk1703 requested a review from asherkin November 5, 2022 16:04
Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

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

This looks really good - great work, and nice to get a cURL update out of it too.

I think the deps are probably a question that'll take some debate - @alliedmodders/sourcemod how do we feel about introducing submodules for external dependencies? I was initially a little against it and preferred having things vendored in-tree for reliability, but especially given the provenance of these projects (and our other deps that could benefit from this treatment - sqlite and libpcre) and the fact we have SP submoduled so a recursive checkout is already required, keeping the repo cleaner is appealing.

extensions/curl/AMBuilder Outdated Show resolved Hide resolved
@psychonic
Copy link
Member

how do we feel about introducing submodules for external dependencies?

I am in favour of this.

Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

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

One additional request to go with the AMBuild changes above, please could you add a feature provider for this so that exts can check if it is available? Take a look at the search results for AddCapabilityProvider for inspiration.

@idk1703 idk1703 requested a review from asherkin November 27, 2022 11:00
@idk1703 idk1703 removed the request for review from asherkin November 29, 2022 19:52
@idk1703 idk1703 requested a review from peace-maker November 29, 2022 19:52
@peace-maker
Copy link
Member

Symlinks on windows require special capabilitities or SYSTEM privileges. So this breaks casual builds on windows :(
@dvander knows AMBuild the best and probably knows how to integrate this best. Maybe we could get curl to look for the config files in other directories as well?

@dvander
Copy link
Member

dvander commented Dec 2, 2022

symlink isn't the only problem. (1) third-party links in submodules are risky. They can and do disappear. They should be cloned into the alliedmodders organization. (2) modifying the source tree during the build is asking for trouble. instead, once the clone is available, these can be checked in as prebuilts, and updated on downstream merges.

Remove symlinks, now it's not need.
@idk1703
Copy link
Author

idk1703 commented Dec 15, 2022

Need import or fork repos in AlliedModders.

curl, mbedtls

@idk1703
Copy link
Author

idk1703 commented Dec 18, 2022

How create pr to new branch ? on curl and mbedtls repos.
sm-ext <- sm-ext
curl: sm-ext - new branch from tag curl-7_86_0
mbedtls: sm-ext - new branch from tag mbedtls-3.3.0

@KyleSanderson
Copy link
Member

@idk1703 are you still blocked on this?

Move submodules, use repos from AM org
Update AMBuilder
Add AMBuilder for curl and mbedtls libs
Add configs for curl
@idk1703
Copy link
Author

idk1703 commented Nov 30, 2023

So i think this done

curl - 7.86.0
mbedtls - 3.3.0

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.

Support TLS in Webternet
9 participants