signalapp / Signal-Desktop

A private messenger for Windows, macOS, and Linux.
https://signal.org/download
GNU Affero General Public License v3.0
14.66k stars 2.67k forks source link

Signal is fundamentally borked #5429

Closed Smit-tay closed 3 years ago

Smit-tay commented 3 years ago

Signal is NOT good software.

Signal cannot be built from source on almost any platform, but, especially Linux based platforms, without jumping through hoops and installing hundreds (if not more) Crap-lications, Crap-raries, and craptastically un-useful, non-standard build tools.

Signal willfully disobeys fundamental principles of software engineering, re-inventing almost everything, obfuscating almost everything, poorly documenting almost everything.

Do you think otherwise ?

Please build Signal from source on any standard linux distro, detail exactly what you had to do to get it running.

If your instructions are more than 10 lines of script - YOU ARE DOING IT WRONG !

I'd like to believe this is not by design. I am struggling to believe that.

Dyras commented 3 years ago

Dude just use the Flatpak https://flathub.org/apps/details/org.signal.Signal

hiqua commented 3 years ago

This is not a bug so it doesn't belong here, feel free to discuss this on https://community.signalusers.org/

Other than that, works fine for me as it is.

Smit-tay commented 3 years ago

The idea that I should use flatpack is acceptance of the correctness of my critique.

Flatpack is for people who don't know how to design cross platform applications.

Muku784 commented 3 years ago

Haha, he is not wrong.

First bug on importing:

20:30 Gradle sync failed: It is not fully supported to define distributionSha256Sum in gradle/wrapper/gradle-wrapper.properties. Using an incorrect value may freeze or crash Android Studio. Please manually verify or remove this property from all of included projects if applicable. For more details, see https://github.com/gradle/gradle/issues/9361.

Nope, I won't debug this now.

Smit-tay commented 3 years ago

Trust me, it's way worse than that.

After downloading half the known universe of libraries, the build fails anyway.

It amazes me that anyone dares use Signal when clearly the development standards are sub-par.

Muku784 commented 3 years ago

Well, it works out of the box from the App stores, tho. And the flatpaks, too. Building on the other hand, I've seen better Android repos, that compile flawlessly. :rabbit:

Smit-tay commented 3 years ago

I have looked cursorily through the code and build system, and cannot "for the life of me" understand how this works.

What an utter disaster

Muku784 commented 3 years ago

For Android there are reproducible builds: https://github.com/signalapp/Signal-Android/tree/master/reproducible-builds But atm I don't really care. No much on a Snowden mission atm.

But on a positive note: at least somebody looks at the code. :+1:

davenicoll commented 3 years ago

So much shit-talking. Did you pay for signal @Smit-tay? Didn't think so. Want it to improve? Roll up your sleeves and improve it, get involved in the community. A privacy-centric messaging system not owned or operated by Google, Apple, etc is worth the effort, surely?

mco-system commented 3 years ago

The fact that Signal is open source free software does not imply that it shall not be criticized. Smit-kay is absolutely right when it says that Signal build system is fundamentally borked

It's not enough to roll up sleeves and get involved in the community. First it needs a community that accept criticizim. A community that will accept patches and changes in the way the software is delivered.

A reliable build system is a valid and very valuable feature request. Tag it like it should be, ask for help on this point if you need some (no one is asking to stop every other development to work on this feature) and you'll see "the community" get involved.

Herohtar commented 3 years ago

non-standard build tools

Which ones do you mean? Python? gcc? g++? make? git? Node? None of these are non-standard.

The steps for building from source are very straightforward. Perhaps you should make an actual useful post detailing the specific issues you have had with building so that people can help and/or things can be improved if necessary.

Smit-tay commented 3 years ago

OK, how about this : Why do I need write access to yarn ? Because the npm install command is installing it, right ? Then why isn't this a 'sudo' command ?

$ node -v v14.18.0 $ git-lfs install Updated git hooks. Git LFS initialized. $ npm install --global yarn npm WARN checkPermissions Missing write access to /usr/local/lib/node_modules/yarn npm WARN checkPermissions Missing write access to /usr/local/lib/node_modules npm ERR! code EACCES npm ERR! syscall access npm ERR! path /usr/local/lib/node_modules/yarn npm ERR! errno -13 npm ERR! Error: EACCES: permission denied, access '/usr/local/lib/node_modules/yarn' npm ERR! [Error: EACCES: permission denied, access '/usr/local/lib/node_modules/yarn'] { npm ERR! errno: -13, npm ERR! code: 'EACCES', npm ERR! syscall: 'access', npm ERR! path: '/usr/local/lib/node_modules/yarn' npm ERR! } npm ERR! npm ERR! The operation was rejected by your operating system. npm ERR! It is likely you do not have the permissions to access this file as the current user npm ERR! npm ERR! If you believe this might be a permissions issue, please double-check the npm ERR! permissions of the file and its containing directories, or try running npm ERR! the command again as root/Administrator.

npm ERR! A complete log of this run can be found in: npm ERR! /home/me/.npm/_logs/2021-10-10T08_58_21_937Z-debug.log

Smit-tay commented 3 years ago

Once that issue is overcome, I attempt this:

$ yarn install --frozen-lockfile yarn install v1.22.15 [1/6] Validating package.json... error signal-desktop@5.20.0-beta.1: The engine "node" is incompatible with this module. Expected version "14.16.0". Got "14.18.0" error Found incompatible module. info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

It's fairly obvious that fails due to an version incompatibility with Node.js (what a surprise), so, now I have to try to use the correct version of Node, so ....

$ nvm use Found '/home/jack/dev/signal/Signal-Desktop/.nvmrc' with version <14.16.0> N/A: version "14.16.0 -> N/A" is not yet installed.

You need to run "nvm install 14.16.0" to install it before using it. That succeeds, and I move on.

Smit-tay commented 3 years ago

$ yarn install --frozen-lockfile yarn install v1.22.15 [1/6] Validating package.json... [2/6] Resolving packages... [3/6] Fetching packages... [#####################--------------------------------------------------------------------------------------------------------] 446/2668

2668 packages !!!! WTF, that's an entire OS !

Smit-tay commented 3 years ago

Maybe it's just me, but, "The platform "linux" is incompatible with this module." looks pretty serious., Of course, it's an optional package, so, I can pretend that's not serious.

warning url-loader@1.1.2: Invalid bin field for "url-loader". info fsevents@1.2.9: The platform "linux" is incompatible with this module. info "fsevents@1.2.9" is an optional dependency and failed compatibility check. Excluding it from installation. info dmg-license@1.0.9: The platform "linux" is incompatible with this module. info "dmg-license@1.0.9" is an optional dependency and failed compatibility check. Excluding it from installation. info iconv-corefoundation@1.1.6: The platform "linux" is incompatible with this module. info "iconv-corefoundation@1.1.6" is an optional dependency and failed compatibility check. Excluding it from installation. info fsevents@2.3.2: The platform "linux" is incompatible with this module. info "fsevents@2.3.2" is an optional dependency and failed compatibility check. Excluding it from installation. [4/6] Linking dependencies... warning " > focus-trap-react@8.8.1" has unmet peer dependency "prop-types@^15.7.2". warning " > react-contextmenu@2.11.0" has unmet peer dependency "prop-types@^15.0.0". warning " > react-sortable-hoc@1.9.1" has unmet peer dependency "prop-types@^15.5.7". warning " > css-loader@3.2.0" has incorrect peer dependency "webpack@^4.0.0". warning " > eslint-config-airbnb-typescript-prettier@3.1.0" has incorrect peer dependency "typescript@>=3.3.1 <3.10.0". warning " > file-loader@4.2.0" has incorrect peer dependency "webpack@^4.0.0". warning " > grunt-gitinfo@0.1.7" has incorrect peer dependency "grunt@~0.4.5". warning " > sass-loader@7.2.0" has incorrect peer dependency "webpack@^3.0.0 || ^4.0.0". warning " > style-loader@1.0.0" has incorrect peer dependency "webpack@^4.0.0".

Smit-tay commented 3 years ago

I'm no Node expert (in fact I stay away from that shit intentionally), but if I get any warning or error messages in my builds, I fix them. I guess Node devs work to a different standard

Is this really attempting to build a Mac module on a Linux platform ???


  • build native dependency from sources  name=mac-screen-capture-permissions
                                          version=2.0.0
                                          platform=linux
                                          arch=x64
                                          napi=
                                          reason=prebuild-install failed with error (run with env DEBUG=electron-builder to get more information)
                                          error=/home/me/dev/signal/Signal-Desktop/node_modules/node-abi/index.js:30
      throw new Error('Could not detect abi for version ' + target + ' and runtime ' + runtime + '.  Updating "node-abi" might help solve this issue if it is a new release of ' + runtime)
      ^

    Error: Could not detect abi for version 13.3.0 and runtime electron.  Updating "node-abi" might help solve this issue if it is a new release of electron
        at getAbi (/home/jack/dev/signal/Signal-Desktop/node_modules/node-abi/index.js:30:9)
        at module.exports (/home/jack/dev/signal/Signal-Desktop/node_modules/prebuild-install/rc.js:53:57)
        at Object.<anonymous> (/home/jack/dev/signal/Signal-Desktop/node_modules/prebuild-install/bin.js:8:25)
        at Module._compile (internal/modules/cjs/loader.js:1063:30)
        at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
        at Module.load (internal/modules/cjs/loader.js:928:32)
        at Function.Module._load (internal/modules/cjs/loader.js:769:14)
        at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
        at internal/main/run_main_module.js:17:47
Smit-tay commented 3 years ago

No idea what this is all about !

$ grunt test Running "unit-tests" task Starting path /home/me/dev/signal/Signal-Desktop/node_modules/.bin/electron with args [ '/home/me/dev/signal/Signal-Desktop/app/main.js' ] App started. Now waiting for test results...

Error: Promise was rejected with the following reason: no such window: target window already closed from unknown error: web view not found Warning: Task "unit-tests" failed. Use --force to continue.

Aborted due to warnings. error Command failed with exit code 3. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. error Command failed with exit code 3. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. error Command failed with exit code 3. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Smit-tay commented 3 years ago

$ grunt test --force Running "unit-tests" task Starting path /home/me/dev/signal/Signal-Desktop/node_modules/.bin/electron with args [ '/home/me/dev/signal/Signal-Desktop/app/main.js' ] App started. Now waiting for test results...

Error: Expected to find window.mochaResults set! Warning: Task "unit-tests" failed. Used --force, continuing.

Running "lib-unit-tests" task Starting path /home/me/dev/signal/Signal-Desktop/node_modules/.bin/electron with args [ '/home/me/dev/signal/Signal-Desktop/app/main.js' ] App started. Now waiting for test results...

2 tests passed.

Done, but with warnings. Done in 37.89s.

Smit-tay commented 3 years ago

And finally, running via "yarn start" almost works, except that I get HTTP errors attempting to register. I can't link with my phone either.

So, after a fairly tedious build process, full of warnings and errors, I have an unusable Application.

I can appreciate that this is probably a minor problem - but, let's face it, the current state of SignalDesktop building is a mess.

I'm not a Node developer, but, I am fairly experienced with software development. I have never heard of "yarn", "npm" or "nvm". Clearly these are Node specific tools, which I also basically object to. Please use standard cross-platform build systems, like make, cmake, or even (reluctantly) meson. I would be much more inclined to help resolve problems if I didn't have to learn Yet Another Build System (YABS),

I also have to wonder about the use of Node.js for a Desktop application. Node was developed primarily for Server-side web development, is this really the right tool for the job ?

Herohtar commented 3 years ago

So basically your complaint is, "I'm not familiar with Node.js, but I can't be bothered to learn anything about it, so I'm going to complain about it"?

npm, yarn, and nvm are all standard tools that are extremely common when working with it. make, cmake, meson and the like are not tools that are used with Node.js (and likely can't be, at least not in any easy manner). They would need to use an entirely different framework/language to do that.

As for its use in a desktop application, the reason it is used is because it powers the Electron cross-platform desktop application framework, which has been used for a large number of widely used apps, including tools like VS Code, Slack, WhatsApp, etc.

I would be much more inclined to help resolve problems if I didn't have to learn Yet Another Build System (YABS),

So the Signal team should drop everything and rewrite the app in some other language so that it can use a build system that you're happy with?

Smit-tay commented 3 years ago

Indeed, that is what it appears I am saying.

But, what I am really commenting upon is the poor (or shall I say, not very good) quality of the existing build.

If I check out a release build branch, I expect to be able to follow a very few instructions to build a working application. I do not expect to encounter warnings and errors of any sort during the build or test cycle unless my system is somehow fundamentally incompatible. The fact that this is all written in a language I am unfamiliar with makes it all the more confusing, but, remember, I am already a fairly experienced software developer. This stuff needs to be absolutely painless and confusion free for a total novice.

I also think it's totally reasonable to expect to be able to find instructions for building, installing and running a "release" version with a release configuration. I finally discovered that I needed to copy a config file named production.json and name it to local-production.json to get login and device link working - which I have done, to great delight. I still don't see instructions for installation, so that I don't have to run "yarn start" command from the console.

Don't get me wrong, I think this is a great product. I just think that the build process in particular needs some very urgent work, and unfortunately, since this is all Node stuff, I really don't want to have to invest more time coming up to speed on that just to contribute what I think are urgent fixes.

josh-signal commented 3 years ago

And finally, running via "yarn start" almost works, except that I get HTTP errors attempting to register. I can't link with my phone either.

You'll want to copy the prod config over to development. Or better yet, create a local-development.json one so that you don't commit it.

Building Signal-Desktop does have its learning curve but once you're setup it's fairly painless and I say this as someone that builds Desktop hundreds of times. You don't have to be too familiar with yarn or node to build it but it does help in certain cases.

Going to close this issue since it's not a bug with the software. Feel free to continue discussing in the community forums.

mweser commented 2 years ago

@Smit-tay You're definitely right about the "build from scratch" being severely difficult to get working. It seems to be a common occurrence to have build systems written on a machine that already has all the dependencies installed, rather than one that is starting from "vanilla".

Smit-tay commented 1 year ago

No improvement at all.

Still an absolute shit-show trying to get a working build from current Release branch.

Sorinaki commented 1 year ago

A+ yeah! trying to build this POS from source is a joke and they want us to TRUST it? sorry Signal, but you FAIL!

Smit-tay commented 1 year ago

Indeed - as predicted.

jamiebuilds-signal commented 1 year ago

I'm sorry you're struggling to understand how to use Node tooling, some of it is tricky! However, we have no plans to rewrite our application using new a new toolchain, and we're only going to actively maintain a single workflow that is the same one we use daily.

In the future, please try to keep the discussion productive. If you are having trouble with a particular step, I suggest you either try Stack Overflow or open an issue with the steps you took and the errors you encountered.

dandv commented 11 months ago

@jamiebuilds-signal I tried to build by running yarn but I get

error signal-desktop@6.39.1: The engine "node" is incompatible with this module. Expected version "18.15.0". Got "18.18.0"

After patching the engine in package.json and re-running yarn, I get

error An unexpected error occurred: "ENOENT: no such file or directory, open '~/.config/yarn/global/.yarnclean'".

I'm only wrestling trying to build because the flatpak binary crashes silently, and the snap one can't connect to the Internet on Fedora.

bruce965 commented 8 months ago

error An unexpected error occurred: "ENOENT: no such file or directory, open '~/.config/yarn/global/.yarnclean'".

@dandv if you are still interested, I managed to build with the specific version of npm and yarn from the CI action: https://github.com/signalapp/Signal-Desktop/blob/ae9181a4b26264ce553c7d8379a3ee5a07de018b/.github/workflows/ci.yml#L25