Whole team gets 'inflate returned -5' when cloning a specific repo on Windows #1848
Comments
|
Would you be able to complete the additional version info (etc) that is in the template, including windows version. Have you tried to create a local server with a bare repository, as a test, that can then be used to verify if this is synchronisation issue with the GitLab server. If you can regain control & integrity locally, it maybe that you can then clear (by branch deletion) the destination repo, and then push the local clean copy. If this is Windows only then there may still be extra things that the GfW fork needs to resolve. |
|
Good morning/evening Philip, First of all, here's the info from the template: Suspecting it might have been a recent update issue, I also tested versions I am using Windows 10 (64-bit) Content of I liked the suggestion of cloning from another server. I actually did not setup another server, but instead copy-pasted the repository on a USB stick. I entered a folder on my C: drive and used Git Bash to Could it be a Git+Gitlab+Windows problem? Or perhaps a Git+SSH+Windows problem? To quickly answer the other suggestions, after regaining local control of my repository, I already tried to make a fresh remote on GitLab (new blank project to which I pushed all branches of our repo), but the problem persisted. I also tried to create a new project and push from a colleague's computer, doubting the issue might have been related to my local repo or some local configurations. Same story. Once again, on Ubuntu it looks like nothing is broken anymore and everybody can work normally. Thanks! |
|
Closing, due to continued lack of an MCVE. I will reopen once one is available. |
|
Having the same issue here with Also from Gitlab. In Linux it works fine, in windows I am getting
I checked on my linux computer, and it seems to be a commit that adds a 4.2G data. Other commits then remove that one. So it might be related to a file size. I will try to work on a MCVE. @dscho, could you please reopen this. |
|
I think that I got it. Please try to clone: https://gitlab.com/ricardo.qtec/bug-gitwin |
|
Cloning from my linux computer via ssh also fails, so we cannot blame gitlab for this. |
|
Looks like the famous "unsigned long" problem. However, @ribalda : Which linux did you use ? 32 bit or 64 bit ? And @raggot my suggestion would be to remove the bigfile(s) from your |
|
@tboegi : Debian testing amd64. The "unsigned long" issue is something planned to fix? Thanks! |
I was may be too fast here with the "unsigned long", more investigation is needed.
|
|
@tboegi I can test it tomorrow at work if you provide me a binary. I have no build environment on that computer. (It is just used to run Altium). Also I do not have windows at home |
Doesn't fix it: |
|
I could imagine that the problem lies elsewhere: |
|
(Note that 705032705 is precisely the difference between 5000000000 and the largest number that can be represented by a 32-bit |
|
To accelerate the inner loop, clone this MCVE (thank you for this @ribalda!!!) in WSL, then copy over the pack from |
|
... and the culprit is indeed an |
|
... and of course the very same thing here: https://github.com/t-b/git/blob/488bfb0a6d08144fa4d8dce5938b829de9f15b1c/builtin/unpack-objects.c#L445 |
Currently the length of data which is stored in memory is stored
in "unsigned long" at many places in the code base.
This is OK when both "unsigned long" and size_t are 32 bits,
(and is OK when both are 64 bits).
On a 64 bit Windows system am "unsigned long" is 32 bit, and
that may be too short to measure the size of objects in memory,
a size_t is the natural choice.
Improve the code base in "small steps", as small as possible.
The smallest step seems to be much bigger than expected.
See this code-snippet from convert.c:
const char *ret;
unsigned long sz;
void *data = read_blob_data_from_index(istate, path, &sz);
ret = gather_convert_stats_ascii(data, sz);
The corrected version looks like this:
const char *ret;
size_t sz;
void *data = read_blob_data_from_index(istate, path, &sz);
ret = gather_convert_stats_ascii(data, sz);
However, when the Git code base is compiled with a compiler that
complains that "unsigned long" is different from size_t, we end
up in this huge patch, before the code base cleanly compiles.
Signed-off-by: Torsten Bögershausen <[email protected]>
|
Friendly reminder: The issue is still marked as closed. Shouldn't we reopen it? Thanks for you help looking into this |
|
@ribalda of course! Thanks for noticing (and pointing it out). |
|
Seriously, I think that zlib needs a patch to replace unsigned long with size_t everywhere, And, if you want a later version of size_t branch, you can find it here: |
|
I've been having a look at the issue since Git-Merge after Thomas Braun suggested a test case and a few other things. # One also has to be very careful about "implementation defined" aspects to upcast anything that may be coerced into a 32bit long where a 64bit size_t is needed (plus all the different names for these types..) See https://github.com/PhilipOakley/git/tree/size_t2 for my extra hacks. (it's on top of the rebased to latest gfw tb series) also gitgitgadget#115 I may get more time tonight to further debug the added big file with 5,100 blocks of 1Mbyte, and compare it with the WSL version (byte by byte!, using HxD). Also will need a suitable bug return code if there is an attempt to extricate a >4Gb file on a 32bit system |
I am fairly certain that such a change would break backwards compatibility rather badly. |
not exactly sure which bits of backward compatibility you were meaning, but I'd agree that a simplistic type replacement won't do as expected because that fails to change the zlib library, where the original uInt incompatibility lay. I also noted that crc32() which is used for packfile checking (an other places.?) also has a potential uInt length problem. I did find that, for instance, the git_inflate_end function which bookends the zlib is in 23 places, so there are plenty of places for things to go wrong. It's a big problem, though the early work on the majority change to size_t has been very useful! |
|
Just cross checking: Still to deep read what/why/how that error comes from.. (listed 20+ times in the manual..) |
|
@ribalda had a look at the https://gitlab.com/ricardo.qtec/bug-gitwin/tree/master I had to clone it via WSL, then looked at the repo via both GfW and WSL. Used the simple (from SO https://stackoverflow.com/a/42544963/717355) and got (extracting the two differing lines) blob 41ba8fda505291b7403d5aa7da0555bf04de80a2 705032704 bigfile <- windows answer At least tracing the |
|
A bit more hand tracing of the code led to the so the data caches don't even store the full 64 bit sizes (on Windows)! There are 23 functions in sha1-file.c that would need their size/len fields changing, with a few more spots for internal variables making 33 just there. I guess the usage cascade will be quite large! |
|
@PhilipOakley : Are you sitting on the master branch ? |
@tboegi I had thought I been on that branch (via details Thomas Braun provided) and rebased the changes to my GfW master version, then added my own extra changes. I will check again about the various merge points (doesn't help on GfW that one has / can have multiple installs, so command history can easily be lost) |
|
@tboegi Yes it looks like I was mistaken. Could you summarise what is in (and not in) your branch - is the goal just a simple conversion to size_t alone (without code changes), or maybe just with fixing of printf syntax, or have you included a few selected code changes? |
|
@tboegi also, How do you manage the distinction between the upstream Git (WSL compilable) and the GfW (sdk compilable) code. |
|
The printing of variables is already fixed in an earlier commit, (upcast to intmax_t). |
Philip refers to the "Windows Subsystem for Linux", a way to run Linux programs on top of the Windows kernel. That feature also is known as "Bash on Ubuntu on Windows", although it's much more than that by now.
@PhilipOakley I fear you replicated my investigation that led to the discussion at t-b@7557481#r32961730 (which reflects my finding that changing the data type of |
|
hi @dscho thanks for clarifying that WSL is shorthand for Windows Subsystem for Linux - I'd forgotten my NUA rule (No Unnamed Abbreviations 😉 ). Torsten (@tboegi) also mentioned the 'We can compile git.master under GfW' (Git-for-Windows) which I think I've also seen you say. I'm not clear on what that means/implies? I could see that it might be reasonable to get a clean compile of upstream git/master from the GfW SDK, but I wasn't expecting that the 'output' would be installable/runnable, and for that (install/run) step one would also need the GfW patches. The confusing (for me) part is exactly where the '32-bit' choice shims itself into build/link/install/run chain, with (different) nuances at each step? You also mentioned the |
|
The "unsigned long c" should be fixed here, thanks @dscho |
|
@dscho Sorry to be a bother, but I can't appear to get the "Rebasing Git for Windows" to work for me for the case where I have added my extra patches (size_t3) on top of @tboegi branch from yesterday (which I'm sure is on top of upstream git). I'm trying to go through the GfW rebase but only got the single staring rebase 'merge commit', without anything else - I was expecting a forest of stuff. The todo list looked like i.e. too short.. I'm at a bit of a loss as to what I've done wrong. Any clues, suggestions or pointers would be of great help. |
|
It looks like the current calculation of That commit is "Start the merging-rebase to upstream/pu" (2019-04-02), so I'll look for the right one |
|
I've updated the wiki Rebasing Git for Windows. Use Dscho has noted footnote that he now also keeps a recent rebase in the /shears/ namespace. so benefit all-round. |
|
@PhilipOakley sorry to be so quiet, I was heads-down in the conversion of To answer some of them: yes, you are right, the Anyway, for lurkers: @PhilipOakley opened #2179 and I just updated it to the current |
Indeed. The problem with I updated https://github.com/git-for-windows/git/wiki/Rebasing-Git-for-Windows in a couple of ways, hopefully you find it helpful:
|
|
Closing this as stale. |
Test whether the Git CI system allows, or fails, when LLP64 systems need variable type coercion from size_t back to unsigned long. All the *nix systems should pass, but compile checks for LLP64 systems may fail. This is a crucial information spike for Git files > 4Gb, i.e. Issues: git-lfs git-for-windows#2434, GfW git-for-windows#2179, git-for-windows#1848, git-for-windows#3306. Signed-off-by: Philip Oakley <[email protected]>
and others
My team and I have a private repository hosted on GitLab. We have had some issues with it lately, and after some testing and hacking, we ended up realising we seem not to be able to
clonefromoriginon Windows, but we can on Ubuntu.I have posted a detailed question on Stack Overflow about the issue we have, so I won't repeat myself, but very briefly, this is what we get when trying to
clone:I read on some other websites that in the past this was associated to a bug in Git. I wonder if it's possible, as in Ubuntu everything seems to work.
Thanks in advance for your time.
The text was updated successfully, but these errors were encountered: