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

git_worktree_lookup returns GIT_ERROR instead of GIT_ENOTFOUND for a non-existing worktree #6366

Closed
arroz opened this issue Jul 27, 2022 · 2 comments

Comments

@arroz
Copy link
Contributor

arroz commented Jul 27, 2022

Reproduction steps

Call git_worktree_lookup with a name that does not match the internal name of any existing worktree.

Expected behavior

An error code of GIT_ENOTFOUND should be returned.

Actual behavior

An error code of GIT_ERROR is returned instead.

Version of libgit2 (release number or SHA1)

Current main (22f3825 at time of writing).

Operating system(s) tested

macOS but should happen on any OS.

@arroz
Copy link
Contributor Author

arroz commented Jul 27, 2022

Likely cause: this line should return GIT_ENOTFOUND instead of -1 (the code for GIT_ERROR): https://github.com/libgit2/libgit2/blob/main/src/libgit2/worktree.c#L136

arroz added a commit to arroz/libgit2 that referenced this issue Aug 31, 2022
arroz added a commit to arroz/libgit2 that referenced this issue Aug 31, 2022
ethomson added a commit that referenced this issue Sep 19, 2022
…rror-code

#6366: When a worktree is missing, return GIT_ENOTFOUND.
VinnyOG added a commit to 8thwall/libgit2 that referenced this issue 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>
@csware
Copy link
Contributor

csware commented Feb 24, 2024

Fixed using #6395.

@ethomson ethomson closed this as completed Mar 9, 2024
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

No branches or pull requests

3 participants