Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
+ Coverage 88.23% 88.27% +0.03%
==========================================
Files 60 61 +1
Lines 2619 2627 +8
Branches 309 309
==========================================
+ Hits 2311 2319 +8
Misses 308 308 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
Author
|
WebAssembly test run succeeded, including all the clone/fetch/push tests that use credentials. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the implementation of the WebAssembly http(s) transport layer that is used by
libgit2. Previously a much simpler version of this was added as a patch to thelibgit2emscripten-forge recipe, this will instead add it to thegit2cppcode base and is registered at runtime withlibgit2to handle http(s) transport using a scope object that is a no-op outside of WebAssembly.I have tested this using a local CORS proxy server and it can access github and gitlab repositories, both public ones and private ones using a personal access token. No doubt changes will be required to work with other git servers and/or CORS proxies, which I will address in time.
The code itself is a combination of the C code that
libgit2expects and the C++ of the rest ofgit2cpp. In places there is tension between the two, I have tried to implement a middle ground so that the architecture is not too different from that used withinlibgit2but we gain the benefit of core C++ classes such as strings and maps. As a result there is some code that is considered bad practice in C++ such as use of public member variables; I don't object to making it more C++-like but I am happy with it as it is.The high-level architecture is that there is a callback
create_wasm_http_transportregistered withlibgit2that is called by anyclone,fetchorpushthat uses http(s) to create awasm_http_subtransport; there is a maximum of onesubtransportpergit2cppcommand run. Each individual http(s) request callswasm_http_actionto create and use awasm_http_stream, usually there is more than one request per command call. A stream works at a relatively high level calling JavaScript code (inEM_JSmacros) to use a synchronousXMLHttpRequest. A single stream may actually use more than one request, for example if there is a redirect fromhttptohttps, or the requirement to enter credentials. Some of these extra requests are automatically handled by the browser or by the CORS proxy server, some of them are handled explicity in this new code. Information about redirects and authorisation credentials is stored at thesubtransportlevel so that subsequentstreams automatically use it without have to re-direct or re-request authorisation.The JavaScript code in
EM_JSmacros is within.cppfiles and subject to the linter's preferences. This is mostly OK as JavaScript is syntactically very similar to C++ but there are a few places containing JS syntax that causes problems, such as the??operator, and there are explicitly excluded from linting.