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

Customize user dir on linux #89

Merged
merged 2 commits into from
Apr 13, 2018
Merged

Customize user dir on linux #89

merged 2 commits into from
Apr 13, 2018

Conversation

simonhong
Copy link
Member

Issue: brave/brave-browser#10

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.

Test Plan:

Set $CHROME_VERSION_EXTRA to "" or "beta" or "unstable" and check user dir in ~/.config/

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions

Brave support four different user dir depends on channel.
* Brave-Browser for stable release
* Brave-Browser-Beta for beta release
* Brave-Browser-Dev for dev release
* Brave-Development for unofficial build

Canary channel isn't supported on linux.

Channel suffixes are determined from $CHROME_VERSION_EXTRA, which is
passed by the launch wrapper script.
For more detail, see src/docs/user_data_dir.md#Linux
Functions in this header is only used in official build.
@simonhong simonhong requested a review from darkdh April 11, 2018 11:47
@simonhong simonhong self-assigned this Apr 11, 2018
std::string modifier;
std::string data_dir_suffix;

char* env = getenv("CHROME_VERSION_EXTRA");
Copy link
Member

Choose a reason for hiding this comment

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

do we have brave-browser PR that set this env?
It would be better if you link other depended PR in a PR. Just put depends [url] in commit or PR description.

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume that we reuse chromium's packaging target in //chrome/installer/linux.
With that, this env is set by wrapper( https://cs.chromium.org/chromium/src/chrome/installer/linux/common/wrapper)

Copy link
Member

@darkdh darkdh Apr 12, 2018

Choose a reason for hiding this comment

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

I wonder how we can trigger these targets
https://cs.chromium.org/chromium/src/chrome/installer/linux/BUILD.gn?l=517-525
so that we can have proper CHANNEL in wrapper?

Copy link
Member Author

@simonhong simonhong Apr 13, 2018

Choose a reason for hiding this comment

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

Sorry for late reply.
Afaik, we can do it by yarn create_dist.
With that, //chrome/installer/linux target is built.
That target should create each channe's installer with proper channel setting.

Copy link
Member

Choose a reason for hiding this comment

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

I mean how to control the channel setting, or is it build all available channels at once?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, create_dist command doesn't have channel option.
Current create_dist would make all three channels at once.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should create_dist support channel option?

Copy link
Member

Choose a reason for hiding this comment

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

I can accept current behavior and you can do channel option as follow-up

Copy link
Member Author

Choose a reason for hiding this comment

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

Always thanks for valuable review! @darkdh

@darkdh darkdh merged commit 1aeb024 into master Apr 13, 2018
@simonhong simonhong deleted the channel_support_linux branch April 13, 2018 02:19
NejcZdovc pushed a commit that referenced this pull request Dec 10, 2018
Corrected publisher state not getting loaded
bbondy pushed a commit that referenced this pull request Feb 18, 2019
Use new size graphic for browser action icon graphic
fmarier pushed a commit that referenced this pull request Oct 29, 2019
pass real cc_wrapper in env var
petemill pushed a commit that referenced this pull request Jul 27, 2020
pass real cc_wrapper in env var
petemill pushed a commit that referenced this pull request Jul 28, 2020
pass real cc_wrapper in env var
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.

3 participants