-
-
Notifications
You must be signed in to change notification settings - Fork 742
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
build issue (with potential fix) on bigendian architectures #6105
Comments
Big endian isn't much tested any more (at least not by me personally) due to the lack of big endian platforms / OSes. Maybe the debian build farm has some insights? |
https://buildd.debian.org/status/package.php?p=borgbackup&suite=sid it seems to work on debian sid ppc64, which is also big endian. |
It's strange that both BIG and LITTLE ENDIAN are defined. Can you grep -r on that system where these macros are defined? |
Also, what does your python3 |
@FelixSchwarz do you remember why we have that code? git says the changeset was based on a patch by you. |
@ThomasWaldmann Do you have a git commit for me so I can have a look? Edit: I think you referred to f9cb040 |
I guess "my patch" was referring to a change I created for Fedora's borgbackup package so it would bundle msgpack: I don't remember any specifics, I probably just did some copy&paste of various code until the magic incantations aligned. I experienced some big endian build issues on Fedora's s390x infrastructure but that was also due to flaky tests. |
The question is somehow why we need to define that in our setup.py, guess it should be in some C header, like endian2.h comes from the spksrc cross-compile build environment. |
I agree that two mechanisms to define the byte order are bad. I guess my inspiration was the code from msgpack's
|
Suspicion: is the issue because you use a little-endian python with headers for a big-endian target system in your crosscompiling environment? |
@methane do you remember why you do that endian magic in your msgpack setup.py? |
@ThomasWaldmann it only fails when cross-compiling (host is always x64_64) to qoriq PPC target (powerpc-e500v2-linux-gnuspe) I was able to find
Hope this helps 🤷 EDIT: Also, found here https://www.nxp.com/docs/en/reference-manual/E500CORERM.pdf page 1-34 it seems it is little endian and would align with the |
Because it is simple than detecting endian from C/C++ source (note that we needed to support old VC++) and I didn't think |
@methane so you think that the suggested patch (just removing that code and relying on the stuff in the C/CPP header files) should not be applied upstream? |
Situation is changed from when I added the code (e.g. I don't need to support Python 2.7 & VC++ 2008 anymore). So I am not sure for now. |
@methane and @ThomasWaldmann , just re-mentioning if that helps, there are no traces of that code on master already. |
@th0ma7 you mean borg master? yes, that is because we only bundle a known-good working msgpack into borg 1.1.x (1.1-maint branch). for master, we just require msgpack (and because of additional wrapper only present in master, we are more flexible concerning new msgpack versions than in 1.1.x). but the msgpack setup.py also has that endian stuff, so the problem then just happens elsewhere. |
@methane it would be great if you could look into fixing this upstream (we would then just copy your solution to our borg 1.1-maint branch setup.py). Guess it doesn't make much sense fixing it just here for 1.1-maint and still run into same issue in master branch (soon borg 1.2.x) where we use msgpack's setup.py for this. |
So, for borg this is "wait for upstream fix" now, thus tagged "later". Also added 1.1.x scope. |
Looking at recent upstream fix from @methane, I now wonder (and for release planning purpose), should I simply wait for a new 1.2.x or 2.x release in the short-term using master, hence fixing the bigendian issue? Or is it expected that a port of the upstream fix to be applied over the 1.1.x? |
In borg 1.2.x (current master branch, beta releases available), we do not have msgpack bundled, so this is not a borg issue there (but msgpack is still required as an external dependency, so you'll get the fix as soon as a fixed msgpack is released). For borg 1.1.18 (which has msgpack 0.5.6 bundled), I am applying an equivalent fix to our setup.py as @methane has applied to msgpack master. Which ends up semantically being the same as the patch proposed by @th0ma7 because borg 1.1.x does not support native win32 anyway. See #6149. |
remove endianness macro, fixes #6105
Fixed in 1.1-maint branch by #6149, no change in master branch needed (see comment above). |
Thnx guys, great work! And confirm working all right (see details below). Cross-compiling details (SynoCommunity/spksrc#5048)
Which now builds fine on PPC (qoriq)
Resulting in a proper wheel:
|
No date yet for 1.1.18, finally getting 1.2.0 out has priority right now. |
The fix for this issue broke big-endian sparc64 on OpenBSD, see #6149 (comment). |
Have you checked borgbackup docs, FAQ, and open Github issues?
Yes
Is this a BUG / ISSUE report or a QUESTION?
Bug, seems already resolved in master but present in 1.1 branch (at least no sign of
__BIG_ENDIAN__
insetup.py
in master)System information. For client/server mode post info for both machines.
Your borg version (borg -V).
1.1.17
Operating system (distribution) and version.
SynoCommunity, Synology DSM-6 (linux)
Hardware / network configuration, and filesystems used.
PPC qoriq arch
How much data is handled by borg?
n/a - build issue only on PPC due to endianess
Full borg commandline that lead to the problem (leave away excludes and passwords)
n/a
Describe the problem you're observing.
build log:
Can you reproduce the problem? If so, describe how. If not, describe troubleshooting steps you took before opening the issue.
Always, only fails on PPC.
Include any warning/errors/backtraces from the system logsActual patch we use to circumvent this (a workaround upstream part of next release would be really great!)SynoCommunity/spksrc@72866f6#diff-7b228d5ca562f5bc3b0ebfbbe98d851ed882feab3721fbb04b57ad844256eb78
The text was updated successfully, but these errors were encountered: