Tdesktop: Please make it build on something else other than x86

[mapreri]

So telegram desktop fails to build everywhere but x86.

I also think support for at the very least arm64 should be provided, but tbh I don't see way whitelisting architectures. Most probably many others just work and are blocked by a configure line...

[Aokromes]

3094

[stek29]

@Aokromes I feel like this issue is more about Linux, not Windows :D

[mapreri]

Well, ISTR that there is a Windows build for ARM v8 (i.e. arm64) somewhere?
But yes, my interest is about Linux ;)
IMHO that check shouldn't be there at all, if there is no need to know about the architecture (and there shouldn't be any, or at most only about the endianess), then there should be no check either.

[auchri]

Did your try to remove the check and compile it?

[Aokromes]

@stek29 yeah, but the other can be renamed and make generic :P

[mapreri]

@auchri

Did your try to remove the check and compile it?

Yes, just changing the #error in #warning in that build_config.h file makes the build just succeed in armhf (ARM v7 with hard floating point), arm64 (ARM v8) and ppc64el. I could try on more architectures but it would require an extra effort of me; also note that I didn't really test whether it will actually start and work, I could probably do that for the arm* architectures (but not for the others, or I may for the little endian architectures, QEMU is a thing!).

[ginggs]

I removed the check, successfully compiled for armhf and telegram-desktop seemed to run fine. I was able to register and send a message.

Raspberry Pi 3 running Ubuntu MATE 17.04 (upgraded from 16.04)

[mymedia2]

I think the main problem is the build_config.h file. How are those macros used?

[stek29]

@mymedia2 ARCH_CPU_32_BITS is only one which is used, and it's used here to define SHIFTED_USE_32BIT. But I can't find any usages of that macro.

Tbh I can't understand why've you discussed this on Debian mailing list, filled a bug, made a patch for it and still haven't submitted it to upstream.

But Endianness should possibly matter a lot and break MTP, although I haven't tested it.

[mapreri]

@stek29 TBH, I do not think that's a good patch for this bug. That's still hardcoding architectures in a way that is not scalable (and there are still architectures Debian builds for that are not accounted for, even!).

There are other ways to determine endianess without hard coding an architecture list, if you really need it. And I fear any usage of that ARCH_CPU_32_BITS, as that would be a porting nightmare for whatever reason you'd use it.

BTW: the only architectures that failed to build now with something that is not due that #error macro, is mips and mipsel, which failed due to cc1plus running OOM (this can be workarounded by tweaking some g++ flags about garbage collection, it's not really program's fault).

[mymedia2]

@stek29 if this macro is the only benefit, I think it'd be better to use __LP64__ macro instead. But I can't understand what optimizations are in the ui/animation.h file.

There are only two macros which are used very widely. It's FORCE_INLINE and WARN_UNUSED_RESULT. But they can be defined in another file, for example in config.h. So I daresay the build_config.h file may be deleted and thus a lot of cross-platform issues will be solved.

Tbh I can't understand why've you discussed this on Debian mailing list, filled a bug, made a patch for it and still haven't submitted it to upstream.

I wanted to test it before send to here. But now I agree with @mapreri it's too big.

[stek29]

@mymedia2 I

I can't understand what optimizations are in the ui/animation.h file

I feel like there are no optimizations in it tbh, because that macro is not used.

@john-preston What would you say?

[john-preston]

@stek29 It is used to decide 32 bit or 64 bit code to use, you have a link to an #ifdef in the comment above.

[glaubitz]

Here's a little more complete list of architectures in case you really want to hardwire the list:

https://github.com/mozilla/gecko-dev/blob/master/media/webrtc/trunk/build/build_config.h

Also, currently only little-endian targets seems to be supported:

/«PKGBUILDDIR»/out/Release/../../Telegram/SourceFiles/config.h:269:2: warning: #warning "Be aware, only little endian is supported!" [-Wcpp]
 #warning "Be aware, only little endian is supported!"
  ^~~~~~~

[stek29]

@john-preston but is it actually used?
@glaubitz I think it's related to MTProto Endianness

[mymedia2]

Telegram Desktop already uses Qt, and this framework has macros for identifying CPU architectures. See Q_PROCESSOR_* macros. Also there are macros for operating systems and compilers. What is the build_config.h file for?

[john-preston]

@mymedia2 Yes, perhaps I can use Q_PROCESSOR_* + Q_BYTE_ORDER instead.

[jrtc27]

That architecture list is also wrong; __sparc__ can be either 32-bit or 64-bit depending on whether __arch64__ is defined. Is there a reason why a configure script can't detect these characteristics?

[john-preston]

@stek29 Yes it is used. There are two codes for dealing with fast 32bit color manipulation: one for 32 bit (that uses two 32 bit unsigned integers and their math) and one for 64 bit (that uses one 64 bit unsigned integer and its math).

[tdesktop-closer]

Hey there!

We're automatically closing this issue since there was no activity in this issue since 391 days ago. We therefore assume that the user has lost interest or resolved the problem on their own. Closed issues that remain inactive for a long period may get automatically locked.

Don't worry though; if this is in error, let us know with a comment and we'll be happy to reopen the issue.

Thanks!

_{(Please note that this is an automated comment.)}

[mapreri]

Can't quite imagine why "no activity in this issue since 391 days ago" means "the user has lost interest". All the informations needed for this bug are provided, it only needs a developer to actually say something useful on the case.

Please reopen this bug report.

[john-preston]

Well, I guess tested (!) pull requests are welcome. I have no way of testing (and sadly no time) other architectures. The big-endian architectures can fail in some random places I'm afraid, need to test carefully.

[mapreri]

So you wouldn't accept a PR just removing the include for base/build_config.h (which is a very tiny whitelist of directories) and the #error for BE systems (making sure the macros they defined are still defined correctly, but nothing else).

After all I don't see the reason to forbid building on unsupported architectures, IMHO just mentioning what architectures are supported officially by you and leaving the others to external contributions whenever needed is a perfectly fine way to deal with this.

[stek29]

@mapreri Error for BE systems is there because a lot of low level mtproto related stuff depends on system being LE. Unless it indeed does work on LE systems, I don't think it is a good thing to remove error. Compile time error is better than weird runtime issues.

[john-preston]

@mapreri Someone should check the build on BE systems first, I'm almost sure it will be buggy if working at all.

Other things, like architecture checks, perhaps indeed could be removed.

[e2gal]

Any news about the support for big-endian systems?
I noticed that Debian people has suceeded building tdesktop in some big-endian systems (https://packages.debian.org/sid/telegram-desktop#pdownload), but the package seems to be buggy. I tried installing the PowerPC package, and it launched sucessfully but I can't seem to send the login code with it (it just hangs with the loading spinner in the lower left corner).

For now I'm just trying to figure out how to build it manually on the PowerPC, if I managed to do it I might be able to help debugging it a bit.
(But I'm quite unfamiliar with C++, Qt, mtproto, and tdesktop code itself so directions are really appreciated)

[RussianNeuroMancer]

I wonder if anyone here have an idea what to do about crash on aarch64?

[john-preston]

Now https://snapcraft.io/telegram-desktop has armhf and arm64 builds. Please check if they're OK.

[glaubitz]

Still fails to build on big-endian targets in Debian: https://buildd.debian.org/status/package.php?p=telegram-desktop&suite=sid

In file included from /<<PKGBUILDDIR>>/obj-s390x-linux-gnu/Telegram_pch/stdafx.h:121:
/<<PKGBUILDDIR>>/obj-s390x-linux-gnu/Release/../../Telegram/SourceFiles/config.h:180:2: error: #error "Only little endian is supported!"
  180 | #error "Only little endian is supported!"
      |  ^~~~~

Although I haven't checked how recent the Debian package is.

[mapreri]

Although I haven't checked how recent the Debian package is.

It's not _that_ recent, as the latest release requires a new cmake apparently.

We'll check how it goes soon :)

[john-preston]

@glaubitz Big Endian is not supported atm, I heard that it doesn't work, some protocol implementation fixes are required.

[ng0177]

Little Endian on Raspberry Pi 4 under Fedora also seems not to be supported.

[glaubitz]

@ng0177 That's an unrelated issue and should be reported separately. This issue is solely about big-endian machines while the Raspberry Pi is little-endian.

[ilya-fedin]

We'll check how it goes soon :)

And what's the result? Can the issue be closed?

[Aokromes]

no answer for over a month closing, if this bug is still valid plz open a new issue with updated telegram.

[jrtc27]

@ng0177 please update the Debian packaging to actually build on BE targets so people can test this?

[ilya-fedin]

please update the Debian packaging to actually build on BE targets so people can test this?

Big endian is not supported by mtproto implementation, someone should contribute on the protocol implementation to be able to use tdesktop on BE.