Skip to content
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

rocketchat-2.10.5: initial commit #39604

Closed
wants to merge 2 commits into from
Closed

rocketchat-2.10.5: initial commit #39604

wants to merge 2 commits into from

Conversation

hlolli
Copy link
Member

@hlolli hlolli commented Apr 27, 2018

Motivation for this change

It's basically an Electron desktop app like Slack. I tested it as nix-overlay. There are some warnings printed about Rocket Chat not being able to auto update via electron-updater, but that's ok as we wouldn't want this feature to work. Otherwise no problem. Need to get myself comfortable with the sandbox, slowly learning nixos, so I decided to PR this derivation.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • [ x ] NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • [ x ] Tested execution of all binary files (usually in ./result/bin/)
  • [ x ] Fits CONTRIBUTING.md.

@emmanuelrosa
Copy link
Contributor

Given that there's also a rocketchat web app, which on NixOS would be implemented as a service, there will need to be a way to distinguish the server side package (which doesn't exist yet) from this desktop app package: Because typically services, say postgres, have packages in the top-level.

As an example, this package can be named rocketchat-desktop while rocketchat could refer to the web app. Or, this package name can stay and the web app package be the one to take the rename.

@emmanuelrosa
Copy link
Contributor

The derivation builds just fine (with sandboxing). I did get the following build output:

cannot find section .interp
cannot find section .interp
not an ELF executable
not an ELF executable
not an ELF executable
not an ELF executable
not an ELF executable
not an ELF executable
not an ELF executable
not an ELF executable

I successfully ran the app from the .desktop file (inside of a vm built with nixos-rebuild build-vm) and also from the commandline. I did get this error regarding a missing file:

$ rocketchat
Checking for update
Error: Error: ENOENT: no such file or directory, open '/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app-update.yml'
    at /nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/electron-updater/src/AppUpdater.ts:370:27
    at Generator.next (<anonymous>)
From previous event:
    at AppImageUpdater.loadUpdateConfig (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/electron-updater/out/AppUpdater.js:395:11)
    at Lazy.AppUpdater.configOnDisk.Lazy [as creator] (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/electron-updater/src/AppUpdater.ts:132:43)
    at Lazy.get value [as value] (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/lazy-val/src/main.ts:18:23)
    at /nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/electron-updater/src/AppUpdater.ts:281:33
    at Generator.next (<anonymous>)
From previous event:
    at AppImageUpdater.doCheckForUpdates (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/electron-updater/out/AppUpdater.js:353:11)
    at /nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/electron-updater/src/AppUpdater.ts:264:25
    at Generator.next (<anonymous>)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)
From previous event:
    at AppImageUpdater._checkForUpdates (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/electron-updater/out/AppUpdater.js:307:11)
    at AppImageUpdater.checkForUpdates (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/node_modules/electron-updater/src/AppUpdater.ts:213:35)
    at checkForUpdates (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/app/background.js:391:37)
    at afterMainWindow (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/app/background.js:516:5)
    at App.<anonymous> (/nix/store/ryvzhlz8a2inm26aw2q04ygrj0r5vz6x-rocketchat-2.10.5/lib/rocketchat/resources/app.asar/app/background.js:606:5)
    at emitTwo (events.js:130:20)
    at App.emit (events.js:213:7)

@hlolli
Copy link
Member Author

hlolli commented Apr 27, 2018

@emmanuelrosa that's the warning I mentioned. The electron-updater is trying to write into this file and check for newer versions. But the app works completely despite of this error messages, and if theey weren't there, we would have electron trying to mutate the nix-store artifacts. So to bypass this, we would have to build the app for ourselves, and I guess for all the node_modules, it's going to be hell writing sha256 for all of it. If there's an example project that does that, I could do that, remove in the source code electron updater and try to make it work without it.

As for the name, I didn't think about the server package. But from utilitarian point of view, typing the extra -desktop seems annoying when you take into account that most people are going to start the client way more often and regularly than the server module. The binary itself from the debian package was names rocketchat, I doubt the RocketChat project would have named the server module the exact same name.

not an ELF executable

probably a result from the find command, I c/p it from the slack derivation, I could be more specific of what library modules needs to be modified.

@emmanuelrosa
Copy link
Contributor

The thought of building an electron app from source on Nix makes my stomach turn.

I think it's perfectly fine to keep the package name. Then whoever creates a server package can just call it something like rocketchat-server, and the service module could simply be called rocketchat.

As for the error, aside from ignoring it, the only thing I can think of that is somewhat reasonable is to replace the AppUpdater.js in the asar archive with with a Nix-friendly neutered version.

@hlolli
Copy link
Member Author

hlolli commented Apr 27, 2018

I'd go for try catch or something. This is typescript, and we may be looking at source map code, could be a stir into a minified spagetti. First I have to unzip/untar the asar archive, will research...

@hlolli
Copy link
Member Author

hlolli commented May 3, 2018

@emmanuelrosa the elecetron-updater is completly supressed in my last commit. The .aspar archive is basically a textfile with header which maps to js files. And the metadata of each file is the file line count and the position of the start of the file in character. So sed-ing on this file works if one does not change the netto character count of the replace.

@emmanuelrosa
Copy link
Contributor

@hlolli Nice!

On my system, the About RocketChat dialog shows a blank window, where as in the original commit the dialog displays a button to check for updates, along with a checkbox to enable/disable checking for updates upon start. What about on your system?

@hlolli
Copy link
Member Author

hlolli commented May 3, 2018

Yes I get the same, it shows empty dialog. No warning or error printed... strange. Will look deeper into this (could be after the weekend).

@matthewbauer
Copy link
Member

So, I think we are going to want to build this from source. Have you tried using yarn2nix?

@hlolli
Copy link
Member Author

hlolli commented May 7, 2018

@matthewbauer no but I'm planing to try yarn2nix for another node based app. I guess I could use it here too, I went this elfpatching path because the slack derivation does the same thing, basically copy paste. But I'm not sure if yarn2nix would solve the electron-updater issue here.

RocketChat/Rocket.Chat.Electron#411

Do you have any suggestions on this. If I'm able to get the "about" dialog back, and maybe remove the "Check for updates" button too. Would it be enough. I'm also happy to do it over with yarn2nix, need to learn it sooner or later :)

@hlolli
Copy link
Member Author

hlolli commented Jul 21, 2018

@matthewbauer it's really a pain to use yarn2nix. It generates pinned dependencies in a nix expression with fetchurl's. Well that's good, I can refernce these sources and fetch them. But the problem arises when npm or yarn tries to use these sources. These pinned dependencies mostly don't have peerdeps/recursive deps. And useing npm/yarn in offline mode just wont work. Preferably I'd just export the contents of these .tgz files into node_modules, but the name of the packed directory sometimes doesn't match the expected node_modules name, sometimes because it's a git: dep or because the version tag is suffixed, so most safe would be to read package.json. Long story short, is there any success project on nixpkgs that uses either yarn2nix or node2nix, which is able to use npm modules in build? If so, I could have some reference to work with. Would love to get this PR merged, before it gets forgotten.

ADDED: Nevermind, I think I know how to do it :-) yarn2nix provides all the functions to replace package.json with a new one in offline mode, https://github.com/moretea/yarn2nix/blob/master/default.nix just seems this hasn't been done before

@d1rewolf
Copy link

Is this still a work in progress? I'd like to run this on NixOS as well. Thanks.

@hlolli
Copy link
Member Author

hlolli commented Sep 14, 2018

@jbwiv you could use my nix expression as an overlay and start running Rocketchat. I'm guessing there's still 4 weeks until I have time to look into this PR, there's to say if someone doesn't beat me to it.

@mmahut
Copy link
Member

mmahut commented Aug 11, 2019

Are there any updates on this pull request, please?

@hlolli
Copy link
Member Author

hlolli commented Sep 25, 2019

I'm going to have another go at this, like @matthewbauer mentioned, we are going to want to build this from source. I'm going to start with patching the electron-builder and make a PR for it, after that, it should be easy. Similarly what I did with yakyak #69393 (in case someone here is up for code-reviewing it). I close this PR for now.

@hlolli hlolli closed this Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants