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 nightly release support system level install on Windows #3728

Closed
simonhong opened this issue Mar 15, 2019 · 4 comments · Fixed by brave/brave-core#2005
Closed

Make nightly release support system level install on Windows #3728

simonhong opened this issue Mar 15, 2019 · 4 comments · Fixed by brave/brave-core#2005

Comments

@simonhong
Copy link
Member

simonhong commented Mar 15, 2019

Chrome canary doesn't support system level install on Windows.
And we also set same value.

Should we have same policy for that or enable system level install on Windows?

In setup_main.cc,

  // Make sure system_level is supported if requested. For historical reasons,
  // system-level installs have never been supported for Chrome canary (SxS).
  // This is a brand-specific policy for this particular mode. In general,
  // system-level installation of secondary install modes is fully supported.
  if (!install_static::InstallDetails::Get().supports_system_level() &&
      (system_install ||
       cmd_line.HasSwitch(installer::switches::kSelfDestruct) ||
       cmd_line.HasSwitch(installer::switches::kRemoveChromeRegistration))) {
    return installer::SXS_OPTION_NOT_SUPPORTED;
  }

chromium_install_modes.cc has that config.
CC @bbondy

@simonhong simonhong self-assigned this Mar 15, 2019
@simonhong
Copy link
Member Author

simonhong commented Mar 15, 2019

@mihaiplesa Because of this setting, our windows nightly stub/standalone installer doesn't work with admin. When we run brave_installer.exe with nightly and system level, it just early returns.
When I run it w/o admin, it works well.
Also, you can build non-admin mode installer by setting TAG_ADMIN env to False.

build_omaha.py handles it.

  tag_admin = os.environ.get('TAG_ADMIN', 'prefers')
  tag = 'appguid=APP_GUID&appname=TAG_APP_NAME&needsadmin=TAG_ADMIN&lang=en&ap=TAG_AP'
  tag = tag.replace("TAG_ADMIN", tag_admin)
  tag = tag.replace("APP_GUID", args.guid[0])
  tag = tag.replace("TAG_APP_NAME", args.tag_app_name[0])
  tag = tag.replace("TAG_AP", args.tag_ap[0])

@simonhong simonhong added the needs-discussion Although the issue is clear, we haven't yet reached a decision about the right solution. label Mar 15, 2019
@bbondy
Copy link
Member

bbondy commented Mar 15, 2019

I think all code that lands even on master is reviewed so I think it's ok. Is there some reason to disable or treat it different from other channels?
cc @diracdeltas @fmarier

@simonhong
Copy link
Member Author

I can't find the exact reason from the source.
For historical reasons and brand specific policy from the comments are the only reason I can find. :)
I also think supporting admin on nightly would be fine.

@mihaiplesa
Copy link
Contributor

I'd suggest going forward with unblocking this (we can revert if needed).

@simonhong simonhong added this to the 0.64.x - Nightly milestone Mar 18, 2019
@simonhong simonhong added OS/Windows setup/installer QA/No and removed needs-discussion Although the issue is clear, we haven't yet reached a decision about the right solution. labels Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment