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

brave: init at 0.24.0 #47864

Merged
merged 1 commit into from
Oct 22, 2018
Merged

brave: init at 0.24.0 #47864

merged 1 commit into from
Oct 22, 2018

Conversation

uskudnik
Copy link
Contributor

@uskudnik uskudnik commented Oct 4, 2018

Motivation for this change

Add brave browser

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

I just started using it as browser "on the side" but most of the stuff seems to work (e.g tor in private mode, youtube).

@GrahamcOfBorg GrahamcOfBorg added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Oct 4, 2018
@emmanuelrosa
Copy link
Contributor

Tested on Xubuntu :)

Given it's an open-source project but a binary package, why not name it brave-bin?

@bhipple
Copy link
Contributor

bhipple commented Oct 4, 2018

Presumably it must be possible to build this from source instead of unpacking the deb? Doing so would likely handle things like the rpath automatically.

@matthewbauer
Copy link
Member

matthewbauer commented Oct 4, 2018

Presumably it must be possible to build this from source instead of unpacking the deb? Doing so would likely handle things like the rpath automatically.

I think that's strongly encouraged given how often we deal with binary package breakages. It's not a hard requirement though - we'd much rather have this package built from the deb than not available at all.

Brave is built on libchromiumcontent which would be nice to start building in Nixpkgs though (Electron also uses it IIRC).

@uskudnik
Copy link
Contributor Author

uskudnik commented Oct 4, 2018

@matthewbauer Yup, that was my feeling too (so, better deb than nothing).

I actually started off with source but had issues getting nodejs / npm to install electron_prebuilt and at some point gave up on that approach and decided to try with deb which turned out to be very simple (basically following the wiki and some inspection of apps with electron, e.g. signal).

My suspicion was that a custom version of both electron and electron_prebuilt was at play but in the end I didn't pursue that avenue long enough to say anything too definite. But I'm also quite new to packaging apps so I might have missed something obvious.

I was also not aware of issues of binary packages breaking so if that's the case we might want to just close it and prevent headaches.

Also, brave is performing major changes so I'm not sure how much of an effort it makes sense to spend on something that is getting deprecated in near future.

@uskudnik
Copy link
Contributor Author

uskudnik commented Oct 4, 2018

@emmanuelrosa Thanks for verifying! I was not aware of any convention to name binary packages with -bin suffix (e.g. no mention in contributing) - is that some convention? Do we have anything on that front that I can read on?

@emmanuelrosa
Copy link
Contributor

@uskudnik The -bin suffix is something I've noticed with packages which come in binary and compiled forms. For example, there's firefox and firefox-bin.

Perhaps I was thinking too far ahead given this is the only Brave package. ;)

@oxij
Copy link
Member

oxij commented Oct 5, 2018 via email

@oxij
Copy link
Member

oxij commented Oct 5, 2018 via email

@Mic92
Copy link
Member

Mic92 commented Oct 6, 2018

I don't think we should have package attribute names ending in -bin unless we really want to have both package variants. Once we are able to build from source ideally we no longer need the other one with a few exception like firefox. However once we a have a -bin suffix we need to continue to support it or put it in pkgs/top-level/aliases.nix. There should be better ways to track whether a package is build from source or not.

version = "0.24.0";

src = fetchurl {
url = "https://github.com/brave/browser-laptop/releases/download/v${version}dev/brave_${version}_amd64.deb";
Copy link
Member

Choose a reason for hiding this comment

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

The github page redirects me to this site: https://github.com/brave/brave-browser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

brave-browser is their next "release", but at this moment the README still says:

Downloads
We're not offering downloads yet for this next generation desktop browser.

I'm also not sure about what github page do you mean? I tried the URL and successfully downloaded the package?

$ wget https://github.com/brave/browser-laptop/releases/download/v0.24.0dev/brave_0.24.0_amd64.deb
--2018-10-06 20:51:29--  https://github.com/brave/browser-laptop/releases/download/v0.24.0dev/brave_0.24.0_amd64.deb
Resolving github.com (github.com)... 192.30.253.112, 192.30.253.113
Connecting to github.com (github.com)|192.30.253.112|:443... connected.
HTTP request sent, awaiting response... 302 Found
Location: https://github-production-release-asset-2e65be.s3.amazonaws.com/46891997/60b47c00-bb32-11e8-9797-9f9575ff9a2c?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20181006%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20181006T185129Z&X-Amz-Expires=300&X-Amz-Signature=b1803354e98ab5e2b820fce5ff0bb13779c8c445774a5e55a4ea82189c4062b2&X-Amz-SignedHeaders=host&actor_id=0&response-content-disposition=attachment%3B%20filename%3Dbrave_0.24.0_amd64.deb&response-content-type=application%2Foctet-stream [following]
--2018-10-06 20:51:29--  https://github-production-release-asset-2e65be.s3.amazonaws.com/46891997/60b47c00-bb32-11e8-9797-9f9575ff9a2c?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20181006%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20181006T185129Z&X-Amz-Expires=300&X-Amz-Signature=b1803354e98ab5e2b820fce5ff0bb13779c8c445774a5e55a4ea82189c4062b2&X-Amz-SignedHeaders=host&actor_id=0&response-content-disposition=attachment%3B%20filename%3Dbrave_0.24.0_amd64.deb&response-content-type=application%2Foctet-stream
Resolving github-production-release-asset-2e65be.s3.amazonaws.com (github-production-release-asset-2e65be.s3.amazonaws.com)... 52.216.105.83
Connecting to github-production-release-asset-2e65be.s3.amazonaws.com (github-production-release-asset-2e65be.s3.amazonaws.com)|52.216.105.83|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 97726210 (93M) [application/octet-stream]
Saving to: ‘brave_0.24.0_amd64.deb’

brave_0.24.0_amd64.deb                                    100%[===================================================================================================================================>]  93.20M  1.78MB/s    in 53s     

2018-10-06 20:52:23 (1.76 MB/s) - ‘brave_0.24.0_amd64.deb’ saved [97726210/97726210]

@uskudnik
Copy link
Contributor Author

uskudnik commented Oct 6, 2018

Personally I'm with @Mic92 (just ~greping for packages that import dpkg/etc. should give us the list of binary packages?).

Much better approach that I see would be to have a meta flag or something similar (which in the fashion of non-free packages someone could also disallow to install if that would be his/her desire).

@emmanuelrosa
Copy link
Contributor

The current release is 0.25.2 and the sha256 is 1r3rsa6szps7mvvpqyw0mg16zn36x451dxq4nmn2l5ds5cp1f017

I'm using the package as an overlay.

@uskudnik
Copy link
Contributor Author

@emmanuelrosa Thanks, updated.

So, what do we do with this package? Do we want it as dpkg or do I close the PR? /cc @Mic92

@Mic92 Mic92 merged commit 119d539 into NixOS:master Oct 22, 2018
@uskudnik uskudnik deleted the add-brave-browser branch July 30, 2019 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants