index: preserve extension parsing errors#4858
Conversation
|
The index changes look good to me - (the tests are failing on Linux though). |
|
I'm not sure why though. It seems the "splitindex" test repo I added isn't there ? If that makes any sense ? Wait, the macOS build should be red as well, the test is failing there (at 16957). |
Ugh, it certainly is. I think the hoops that I had to jump through to get leaks to play nicely (it wants to background itself but keep some file descriptors open 😡) must not have worked right. Sigh. It's already very brittle (if you can't convince it to close that last file descriptor then it will just never exit and hang the builds), so I think I may have to disable leaks until I can make it more stable. |
|
/rebuild |
|
Okay, @tiennou, I started to rebuild this pull request. |
|
I'm really confused by CI here, I can run that new testcase using the new test resource locally and I get a successful run, but CI fails because the resource is missing. Also, I suspect the Windows also has a problem with considering the build green while it failed… |
e93e614 to
57c6970
Compare
57c6970 to
00f17de
Compare
|
Rebased, was hoping to get CI to play nice but it still doesn't want to play along. I feel like I'm missing something, as I can't see anything wrong with the testcase setup… |
This has happened to me before, and to troubleshoot I'll often clone a new version of the repository and check out my PR branch. I'm not positive, but I suspect that your I think that if you create some empty files to preserve the diff --git a/tests/resources/splitindex/.gitted/objects/info/dummy-marker.txt b/tests/resources/splitindex/.gitted/objects/info/dummy-marker.txt
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
diff --git a/tests/resources/splitindex/.gitted/objects/pack/dummy-marker.txt b/tests/resources/splitindex/.gitted/objects/pack/dummy-marker.txt
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
diff --git a/tests/resources/splitindex/.gitted/refs/heads/dummy-marker.txt b/tests/resources/splitindex/.gitted/refs/heads/dummy-marker.txt
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
diff --git a/tests/resources/splitindex/.gitted/refs/tags/dummy-marker.txt b/tests/resources/splitindex/.gitted/refs/tags/dummy-marker.txt
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391(For what it's worth, if you create branches in libgit2/libgit2 instead of your fork, I or another maintainer could push up a fix into your branch and then merge it.) |
00f17de to
4271c6b
Compare
|
OMG I've been wrangling with the repository open code and it didn't register that there were missing directories… Thanks for the pointer @ethomson, I've now rebased it with some |
I don't think I have the permissions to do that (and I'm kinda evading doing that, because I'm a rebase-junkie and I like to keep it separated). I also was under the impression that it's only dependent on whether "Allow edit from maintainers" being checked ? BTW, kudos for the Git Advent series, especially the one about fetching PRs 😉. |
My workflow in |
4271c6b to
4138c0d
Compare
|
Rebased, with style-fix applied. Thanks for the review! |
4138c0d to
4ccb301
Compare
Previously, we would clobber any extension-specific error message with an "extension is truncated" message. This makes `read_extension` correctly preserve those errors, takes responsibility for truncation errors, and adds a new message with the actual extension signature for unsupported mandatory extensions.
4ccb301 to
0bf7e04
Compare
Previously, we would clobber any extension-specific error message with an "extension is truncated" message. This makes
read_extensioncorrectly preserve those errors, takes responsibility for truncation errors, and adds a new message with the actual extension signature for unsupported mandatory extensions.Inspired by #4857.