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.

Sign up for GitHub

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

Jump to bottom

rambox: don't build from sources #80370

Merged
merged 2 commits into from Feb 21, 2020
Merged

rambox: don't build from sources #80370

merged 2 commits into from Feb 21, 2020

Conversation

ghost
Copy link

@ghost ghost commented Feb 17, 2020 •
edited by ghost

Motivation for this change

See #76618

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @Ma27 @Mic92

@Ma27 Ma27 requested a review from lukateras February 17, 2020 19:52
lukateras
lukateras approved these changes Feb 17, 2020
View reviewed changes
Copy link
Member

@lukateras lukateras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the inconvenience!

@Mic92
Copy link
Member

Mic92 commented Feb 17, 2020

The non-pro version does not start for me (also it is called ramboxpro atm):

$ ramboxpro
App threw an error during load
Error: EEXIST: file already exists, mkdir '/home/joerg/.config/Rambox'
at Object.mkdirSync (fs.js:731:3)
at make (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/make-dir/index.js:61:12)
at Function.module.exports.sync (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/make-dir/index.js:84:9)
at ElectronStore.get store [as store] (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/conf/index.js:141:13)
at new Conf (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/conf/index.js:46:26)
at new ElectronStore (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/electron-store/index.js:20:3)
at Object. (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/electron/main.js:21:16)
at Object. (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/electron/main.js:509:3)
at Module._compile (internal/modules/cjs/loader.js:693:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:704:10)
A JavaScript error occurred in the main process
Uncaught Exception:
Error: EEXIST: file already exists, mkdir '/home/joerg/.config/Rambox'
at Object.mkdirSync (fs.js:731:3)
at make (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/make-dir/index.js:61:12)
at Function.module.exports.sync (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/make-dir/index.js:84:9)
at ElectronStore.get store [as store] (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/conf/index.js:141:13)
at new Conf (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/conf/index.js:46:26)
at new ElectronStore (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/node_modules/electron-store/index.js:20:3)
at Object. (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/electron/main.js:21:16)
at Object. (/nix/store/dc6rvad9ln7nnm8nmi10qxnq5d8aybgk-rambox-0.7.3/opt/RamboxPro/resources/app.asar.unpacked/electron/main.js:509:3)
at Module._compile (internal/modules/cjs/loader.js:693:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:704:10)
Fontconfig warning: "/etc/fonts/fonts.conf", line 86: unknown element "blank"

@ghost
Copy link
Author

ghost commented Feb 21, 2020

@Mic92

The non-pro version does not start for me (also it is called ramboxpro atm):

Looks like that solution doesn't work with rambox community edition, I've changed back to using binaries instead of launching electron

@yegortimoshenko No need to sorry, it was a nice idea, but I would like to simplify maintaining of rambox

@Mic92
Copy link
Member

Mic92 commented Feb 21, 2020

I had to delete ~/.config/Rambox and than it worked.

@Mic92 Mic92 merged commit bec89d1 into NixOS:master Feb 21, 2020
@ghost
Copy link
Author

ghost commented Feb 21, 2020

@Mic92 Thank you!

@ghost ghost deleted the rambox branch February 21, 2020 16:00
@Mic92
Copy link
Member

Mic92 commented Feb 21, 2020

We should probably backport this as we in future likely also need to upgrade rambox in stable as well to unbrick services.

Ma27
Ma27 reviewed Feb 21, 2020
View reviewed changes
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the inconvenience!

No worries! It was a great effort since source-builds should be preferred over pre-built binary blobs in general IMHO. Also, the error is pretty hard to catch and reproduce (already seen several hash-updates in this package that weren't reproducible by everyone).

@gnidorah the change seems mostly fine, I have two questions though:

  • I stopped rambox 0.7.3 (from a fairly recent release-20.03) and ran ./result/bin/rambox after building it on your branch. The program worked fine, however all of my tabs were gone. After stopping this rambox process and reopening the one I installed globally, the tabs reappeared. Do you know if this is somehow related to the environment rambox is started in or an actual regression of your patch? In case of the latter it would be awesome if you could either try to find a fix or document this in the release notes at least.

  • The comment at the top of fetchNodeModules.nixsays that Only npm >= 5.4.2 is deterministic. So is there any chance to provide another fix atm? (or is it known what caused the issues?).

@Ma27
Copy link
Member

Ma27 commented Feb 21, 2020

So I was obviously too slow to with writing a review 😅

@gnidorah would you mind taking a look at my comments anyway? :)

@ghost
Copy link
Author

ghost commented Feb 21, 2020

@Ma27

I stopped rambox 0.7.3 (from a fairly recent release-20.03) and ran ./result/bin/rambox after building it on your branch. The program worked fine, however all of my tabs were gone. After stopping this rambox process and reopening the one I installed globally, the tabs reappeared. Do you know if this is somehow related to the environment rambox is started in or an actual regression of your patch? In case of the latter it would be awesome if you could either try to find a fix or document this in the release notes at least.

Can't reproduce. Do you use auth0?

@Mic92
Copy link
Member

Mic92 commented Feb 21, 2020

@gnidorah I might did that too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
8.has: clean-up 10.rebuild-darwin: 0 10.rebuild-linux: 0 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Mic92 @Ma27 @lukateras