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

Make stable brave-browser depend on brave-keyring #2338

Merged
merged 1 commit into from
Apr 25, 2019

Conversation

fmarier
Copy link
Member

@fmarier fmarier commented Apr 24, 2019

Fixes brave/brave-browser#4205.

Submitter Checklist:

Test Plan:

With packages on the release channel:

  • try installing them without the brave-keyring package (e.g. apt install brave-browser)
    • the brave-keyring will be pulled in automatically
    • brave-browser can't be installed on its own
  • remove brave-keyring (e.g. apt remote brave-keyring):
    • this also removes brave-browser

With packages on the beta or dev channels:

  • try installing them without the brave-keyring:
    • it will work and brave-keyring won't be pulled in automatically
  • install brave-keyring manually after brave-browser-beta
  • remove brave-keyring
    • this will not remove brave-browser-beta

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@fmarier fmarier requested a review from mbacchi April 24, 2019 20:20
@fmarier fmarier self-assigned this Apr 24, 2019
@fmarier
Copy link
Member Author

fmarier commented Apr 24, 2019

I inspected the .deb and .rpm manually and they both included the correct metadata.

I tried installing the generated release package on Fedora and it correctly pulled in the dependency:

$ dnf install /home/francois/brave-browser-0.66.18-1.x86_64.rpm 
Last metadata expiration check: 1:50:41 ago on Wed 24 Apr 2019 06:01:18 PM UTC.
Dependencies resolved.
================================================================================================================
 Package             Architecture Version            Repository                                            Size
================================================================================================================
Installing:
 brave-browser       x86_64       0.66.18-1          @commandline                                          72 M
Installing dependencies:
 brave-keyring       noarch       1.1-1.fc30         brave-browser-rpm-release.s3.brave.com_x86_64_        10 k

Transaction Summary
================================================================================================================
Install  2 Packages

Total size: 72 M
Total download size: 10 k
Installed size: 234 M
Is this ok [y/N]: 

I tried installing the generated .deb and it correctly failed due to a missing dependency:

$ dpkg -i /home/francois/devel/brave-browser/src/out/Release/brave-browser_0.66.18_amd64.deb 
Selecting previously unselected package brave-browser.
(Reading database ... 31124 files and directories currently installed.)
Preparing to unpack .../brave-browser_0.66.18_amd64.deb ...
Unpacking brave-browser (0.66.18) ...
dpkg: dependency problems prevent configuration of brave-browser:
 brave-browser depends on brave-keyring; however:
  Package brave-keyring is not installed.

dpkg: error processing package brave-browser (--install):
 dependency problems - leaving unconfigured
Processing triggers for mime-support (3.60ubuntu1) ...
Errors were encountered while processing:
 brave-browser

Then I rebuilt the packages on the beta channel this time:

$ npm run create_dist Release -- --debug_build=true --channel=beta

I was able to install both beta packages without brave-keyring:

$ dnf install /home/francois/brave-browser-beta-0.66.18-1.x86_64.rpm 
created by dnf config-manager from https://brave-browser-rpm-release.s3.brave.c  80 kB/s | 3.3 kB     00:00    
Fedora Modular 29 - x86_64                                                       33 kB/s |  16 kB     00:00    
Fedora Modular 29 - x86_64 - Updates                                             66 kB/s |  15 kB     00:00    
Fedora 29 - x86_64 - Updates                                                     49 kB/s |  16 kB     00:00    
Fedora 29 - x86_64                                                               46 kB/s |  16 kB     00:00    
Dependencies resolved.
================================================================================================================
 Package                          Architecture         Version                 Repository                  Size
================================================================================================================
Installing:
 brave-browser-beta               x86_64               0.66.18-1               @commandline                73 M

Transaction Summary
================================================================================================================
Install  1 Package

Total size: 73 M
Installed size: 240 M
Is this ok [y/N]: 

$ dpkg -i /home/francois/devel/brave-browser/src/out/Release/brave-browser-beta_0.66.18_amd64.deb 
Selecting previously unselected package brave-browser-beta.
(Reading database ... 31444 files and directories currently installed.)
Preparing to unpack .../brave-browser-beta_0.66.18_amd64.deb ...
Unpacking brave-browser-beta (0.66.18) ...
Setting up brave-browser-beta (0.66.18) ...
update-alternatives: using /usr/bin/brave-browser-beta to provide /usr/bin/x-www-browser (x-www-browser) in auto mode
update-alternatives: using /usr/bin/brave-browser-beta to provide /usr/bin/gnome-www-browser (gnome-www-browser) in auto mode
update-alternatives: using /usr/bin/brave-browser-beta to provide /usr/bin/brave-browser (brave-browser) in auto mode
Processing triggers for mime-support (3.60ubuntu1) ...

and hand-inspection showed the correct dependency metadata.

Copy link
Contributor

@mbacchi mbacchi left a comment

Choose a reason for hiding this comment

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

This is great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

brave-browser package should depend on brave-keyring
2 participants