Skip to content

Get libgit2 working for 8thwall.#24

Merged
VinnyOG merged 2 commits into
mainfrom
upgrade-libgit2
Jun 1, 2022
Merged

Get libgit2 working for 8thwall.#24
VinnyOG merged 2 commits into
mainfrom
upgrade-libgit2

Conversation

@VinnyOG
Copy link
Copy Markdown

@VinnyOG VinnyOG commented May 31, 2022

This PR takes inspiration from https://github.com/petersalomonsen/wasm-git

It adds sha256 hashing to the whole body for code commit and uses emscripten fetch API in C++ instead of javascript fetch with EM_ASM calls.
Original files:

It fixes an issue with initializing where we hit a limit of 16 inits max.
It also fixes an error when trying to delete references.

if (connectionMap.count(connectionNumber) != 1) {
printf("Attempting to read from connection %l but it is not connected\n", connectionNumber);
*bytesRead = 0;
return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we be returning -1 here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It would probably be a good thing to do. And I'll pass through this error throw the relative emscriptenhttp_stream_read and emscriptenhttp_stream_write_single

s = reinterpret_cast<emscriptenhttp_stream *>(git__calloc(1, sizeof(emscriptenhttp_stream)));
GIT_ERROR_CHECK_ALLOC(s);

s->parent.subtransport = &t->parent;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Wait, is &t->parent the same as &(t->parent)? Pretty wild.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Order of operations 🙃

connection.attr.attributes =
EMSCRIPTEN_FETCH_LOAD_TO_MEMORY | EMSCRIPTEN_FETCH_SYNCHRONOUS | EMSCRIPTEN_FETCH_REPLACE;
if (std::string(method) == "GET") {
auto headersToSend = connection.headers;
Copy link
Copy Markdown

@cmbartschat cmbartschat May 31, 2022

Choose a reason for hiding this comment

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

Do we need to do anything here to reject any future xhrWrite/xhrRead here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This runs with the precondition that the smart transport implementation in libgit2 is correct. A new connection is only opened if a fetch isn't already made on line 90 in xhrWrite()

"POST",
{
"Content-Type",
uploadPack ? "application/x-git-upload-pack-request"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I feel like there is a third option here...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There's no doubt several.

@VinnyOG VinnyOG requested a review from cmbartschat June 1, 2022 18:25
@VinnyOG VinnyOG merged commit b5d0bf1 into main Jun 1, 2022
VinnyOG added a commit that referenced this pull request Jun 22, 2022
Upgrade libgit2 with support for 8thwall cloud editor.
VinnyOG added a commit that referenced this pull request Aug 5, 2022
Upgrade libgit2 with support for 8thwall cloud editor.
@VinnyOG
Copy link
Copy Markdown
Author

VinnyOG commented Aug 5, 2022

NOTE: I rebased this squashed commit onto the latest upstream main branch on August 5 2022. Rebase completed without conflicts. The result was force-pushed here.

VinnyOG added a commit that referenced this pull request Jan 30, 2023
Upgrade libgit2 with support for 8thwall cloud editor.
VinnyOG added a commit that referenced this pull request Jan 30, 2023
* net: move url tests into util

* url: remove invalid scp url parsing test

The url::scp::invalid_addresses test attempts to test an invalid IPv6
address. It does not, it calls the regular URL parsing function which
treats it like a possibly invalid scheme.

* url: test that we don't expand % encoding in paths

* url_parse: introduce our own url parsing

Provide our own url parser, so that we can handle Google Code's "fun"
URLs that have a userinfo with an `@` in it. :cry:

* url: only allow @s in usernames for ssh urls

Enforce the RFC for other protocols; Google's questionable choices about
malformed SSH protocols shouldn't impact our ability to properly parse
HTTPS.

* push: revparse refspec source, so you can push things that are not refs

I want to push a commit by OID to a remote branch. Currently, push parses the refspecs such that the source must be the name of a ref (it uses git_reference_name_to_id to resolve it). This commit changes it so push uses git_revparse_single to resolve the source of the refspec. This allows for OIDs or other revs (e.g. `HEAD~2`) to be pushed.

* clone: test long custom header

* winhttp: handle long custom headers

* Don't fail the whole clone if you can't find a default branch

In commit 6bb3587 ("clone: set refs/remotes/origin/HEAD to default
branch when branch is specified, attempt 2") libgit2 was changed to set
the default remote branch when one was copied.

But it makes update_head_to_branch() return an error if the origin
doesn't even *have* a HEAD in the first place, since
git_remote_default_branch() will fail.

That's entirely wrong, and means that you cannot do "git_clone()" of a
particular branch on a remote repository when that remote doesn't have a
default branch at all.

So don't set the error code.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

* fix compile on Windows with -DWIN32_LEAN_AND_MEAN

ensure the needed wincrypt.h is included

* Fix libgit2#6365

* Fix memory leak

Signed-off-by: Sven Strickroth <email@cs-ware.de>

* libgit2#6366: When a worktree is missing, return GIT_ENOTFOUND.

* Delete create.c.bak

* ci: move to macos-11

GitHub has deprecated macOS 10.15; move to their new macOS 11 build
servers.

* ci: clean up daemon processes on exit

We previously (correctly) cleaned up the git daemon and SSH server, but
failed to clean up our bespoke HTTP server and HTTP proxies. Capture
their PIDs on process creation and kill them when we shut down.

* clone: narrow success tests on HEAD-less remotes

Only allow the remote default branch checking to fail when the remote
default branch doesn't exist (`git_remote__default_branch` returns
`GIT_ENOTFOUND`). If there was any other type of error - for example, an
allocation failure - we should not swallow that and continue to fail.

This allows us to support the case when a remote has not advertised a
HEAD -- this is possible, for example, when the remote has constrained
the caller to a particular namespace. But other remote failures remain
as failures.

* clone: test bare clone namespaced repo with no HEAD

Test that we can successfully clone a repository that is namespace
scoped to a bare repository locally. We need not specify a checkout
branch in this case (obviously, since we do not check anything out in a
bare clone).

* clone: test for cloning a repo with namespace scope

Test that we can successfully clone a repository that is namespace
scoped on the remote and does not advertise a HEAD. To do this, we must
specify the branch to checkout.

* Update src/util/rand.c

* tests: skip sha256 tests when not compiled in

Actually `cl_skip` the sha256 tests when we're not compiled for sha256,
instead of passing them.

* cmake: provide empty experimental.h for non-cmake users

Not everybody builds libgit2 using cmake; provide an `experimental.h`
with no experiments configured for those that do not. To support this,
we also now create compile definitions for experimental functionality,
to supplant that empty `experimental.h`. cmake will continue to generate
the proper `experimental.h` file for use with `make install`.

* repo: test ownership validation fails with expected error

* repo: ignore missing 'safe.directory' config during ownership checks

* add 2-clause BSD license to COPYING

The `git_fs_path_basename_r()` function in `src/util/fs_path.c` says
it's based on Android code using the 2-clause BSD license, so I
suppose that means the COPYING file should include that.

* benchmark: update path

The path to the default CLI output has changed, update it.

* http: Update httpclient options when reusing an existing connection.

Httpclient internally stores a copy of the certificate_check callback and
payload. When connecting via HTTPS, and if the server sends back
"Connection: close" after the first request, the following request would
attempt to re-use the httpclient and call the (now outdated) callback. In
particular for pygit2 this is a problem, since callbacks / payloads are only
valid for the duration of a libgit2 call, leading to a ffi.from_handle()
error and crashing the Python interpreter.

* ssh: verify the remote's host key against known_hosts if it exists

It turns out this has been available in libssh2 for a long time and we should
have been verifying this the whole time.

* Fix leak in git_tag_create_from_buffer

If the tag already exists and we are not forcing overwrite we need to free ref_name buffer before return the "tag already exists" error.

* Missing dispose

* Missing dispose in git_tag_create__internal

* commit-graph: only verify csum on git_commit_graph_open().

It is expensive to compute the sha1 of the entire commit-graph file each
time we open it. Git only does this if it is re-writing the file.

This patch will only verify the checksum when calling the external API
git_commit_graph_open(), which explicitly says it opens and verifies
the commit graph in the documentation.

For internal library calls, we call git_commit_graph_get_file(), which
mmaps the commit-graph file in read-only mode. Therefore it is safe to
skip the validation check there.

Tests were added to check that the validation works in the happy path,
and prevents us from opening the file when validation fails.

(Note from Derrick Stolee: This patch was applied internally at GitHub
after we recognized the performance impact it had during an upgrade of
libgit2. The original author left the company before we remembered to
send it upstream.)

Signed-off-by: Derrick Stolee <derrickstolee@github.com>

* tests: append the github.com ssh keys so we have access during tests

Currently just the one test needs it.

The ssh-rsa makes sure we're asking for the cipher we find in `known_hosts` as
that won't be the one selected by default. This will be relevant in later changes.

* tests: move online::clone::ssh_auth_methods into the ssh test suite

We're currently running it as part of the online suite but that doesn't have any
setup for ssh so we won't find the GitHub keys we set up during the test.

It doesn't need the private key setup as we just want to make sure we see some
auth request from the server, but with the addition of hostkey checking we're
now seeing it fail when we skip these tests.

* ssh: look for a key in known_hosts to set the key type for the handshake

The server and client negotiate a single hostkey, but the "best" cipher may not
be the one for which we have an entry in `known_hosts`. This can lead to us not
finding the key in known_hosts even though we should be connecting.

Instead here we look up the hostname with a nonsense key to perform a lookup in
the known hosts and set that. This is roughly what the OpenSSH client does as
well.

* commit_graph: use uint64_t for arithmetic

This should resolve some issues with UBSan builds by using unsigned
64-bit integers for all arithmetic until we finally convert to an offset
or size value.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>

* Add support for "safe.directory *"

Signed-off-by: Sven Strickroth <email@cs-ware.de>

* thread: avoid warnings when building without threads

`git__noop` takes no arguments, so a simple `#define func(a) git__noop`
will produce warnings about the unused `a`. Introduce `git__noop_args`
to swallow arguments and avoid that warning.

* tests: Add new test to submodule::update

Verify that trying to update submodule which has been configured but not added does return an error.

Issue libgit2#6433: git_submodule_update fails to update configured but missing submodule

* submodule: Do not try to update a missing submodule

If a submodule has been configured but not yet added, do not try to update it.

Issue libgit2#6433: git_submodule_update fails to update configured but missing submodule

* transport: fix capabilities calculation

This looks like a typo to me, from what i can see these lines were
added at the same time and because of how capabilities are calculated,
it's likely that this code will work in situations where these
capabilities were the last ones.

* Use `git_clone__submodule` to avoid file checks in workdir

`git_clone` checks for existence of (non-empty) directories that would clash with what is about to be cloned.

This is problematic when cloning submodules since they make sense in the context of a parent module, so they should not use the current working dir.

Since in `git_submodule_update` we clone the submodule only when it is not yet initialized we do not need to perform directory checks.

* README: show v1.5 and v1.4 branch builds

* ci: update version numbers of actions

* push: use resolved oid as the source

211c971 attempts to use the parsed OID
but inverted the arguments to `git_oid_cpy`.

* src: hide unused hmac() prototype

It conflicts with NetBSD's in its libc.

Closes libgit2#6457

* hash: drop all declarations from hmac

The builtin hash uses the code verbatim from rfc6234, including
prototypes for functions that we don't use (like hmac). Remove all
unused prototypes to avoid collisions with things that an operating
system might provide (like hmac).

* tests: update clar test runner

Update to the latest main version of clar, which includes improved xml
summary output.

* tests: fix clar declarations

* clar: cross-platform elapsed time counter

Abstract time counter for tests; use gettimeofday on Unix and
GetTickCount on Windows.

* ci: always create test summaries, even on failure

When the dependent jobs fail -- possibly due to test failures -- we
should still produce the job summary that shows those test failures.

* ci: update upload-artifact dependency

* ci: upgrade test-summary action

* Get libgit2 working for 8thwall. (#24)

Upgrade libgit2 with support for 8thwall cloud editor.

---------

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sven Strickroth <email@cs-ware.de>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Co-authored-by: Edward Thomson <ethomson@edwardthomson.com>
Co-authored-by: Sven Over <sven@cord.com>
Co-authored-by: Kevin Saul <kevinsaul@gmail.com>
Co-authored-by: Linus Torvalds <torvalds@linux-foundation.org>
Co-authored-by: Christoph Cullmann <cullmann@kde.org>
Co-authored-by: Vinz2008 <68145293+Vinz2008@users.noreply.github.com>
Co-authored-by: Sven Strickroth <email@cs-ware.de>
Co-authored-by: Miguel Arroz <750683+arroz@users.noreply.github.com>
Co-authored-by: Laurence McGlashan <mail@laurencemcglashan.com>
Co-authored-by: Martin von Zweigbergk <martinvonz@google.com>
Co-authored-by: Sebastian Lackner <sebastian.lackner@sysmagine.com>
Co-authored-by: Carlos Martín Nieto <carlosmn@github.com>
Co-authored-by: Julian Mesa <julian.mesa@gitkraken.com>
Co-authored-by: Colin Stolley <ccstolley@github.com>
Co-authored-by: Derrick Stolee <derrickstolee@github.com>
Co-authored-by: Edward Thomson <ethomson@vercel.com>
Co-authored-by: tagesuhu <118989859+tagesuhu@users.noreply.github.com>
Co-authored-by: Russell Sim <rsl@simopolis.xyz>
Co-authored-by: Aleš Bizjak <abizjak@users.noreply.github.com>
Co-authored-by: Thomas Klausner <wiz@gatalith.at>
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.

2 participants